| Message ID | 20190110134433.15672-2-joro@8bytes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix virtio-blk issue with SWIOTLB | expand |
On Thu, Jan 10, 2019 at 02:44:31PM +0100, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > The SWIOTLB implementation has a maximum size it can > allocate dma-handles for. This needs to be exported so that > device drivers don't try to allocate larger chunks. > > This is especially important for block device drivers like > virtio-blk, that might do DMA through SWIOTLB. Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they need to limit the size of pages. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> > --- > include/linux/swiotlb.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h > index 7c007ed7505f..0bcc80a97036 100644 > --- a/include/linux/swiotlb.h > +++ b/include/linux/swiotlb.h > @@ -72,6 +72,11 @@ static inline bool is_swiotlb_buffer(phys_addr_t paddr) > return paddr >= io_tlb_start && paddr < io_tlb_end; > } > > +static inline size_t swiotlb_max_alloc_size(void) > +{ > + return ((1UL << IO_TLB_SHIFT) * IO_TLB_SEGSIZE); > +} > + > bool swiotlb_map(struct device *dev, phys_addr_t *phys, dma_addr_t *dma_addr, > size_t size, enum dma_data_direction dir, unsigned long attrs); > void __init swiotlb_exit(void); > @@ -95,6 +100,13 @@ static inline unsigned int swiotlb_max_segment(void) > { > return 0; > } > + > +static inline size_t swiotlb_max_alloc_size(void) > +{ > + /* There is no limit when SWIOTLB isn't used */ > + return ~0UL; > +} > + > #endif /* CONFIG_SWIOTLB */ > > extern void swiotlb_print_info(void); > -- > 2.17.1 >
On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote: > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they > need to limit the size of pages. That function just exports the overall size of the swiotlb aperture, no? What I need here is the maximum size for a single mapping. Regards, Joerg
On Fri, Jan 11, 2019 at 10:12:31AM +0100, Joerg Roedel wrote: > On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote: > > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they > > need to limit the size of pages. > > That function just exports the overall size of the swiotlb aperture, no? > What I need here is the maximum size for a single mapping. Yes. The other drivers just assumed that if there is SWIOTLB they would use the smaller size by default (as in they knew the limitation). But I agree it would be better to have something smarter - and also convert the DRM drivers to piggy back on this. Or alternatively we could make SWIOTLB handle bigger sizes.. > > Regards, > > Joerg
On Mon, Jan 14, 2019 at 03:49:07PM -0500, Konrad Rzeszutek Wilk wrote: > On Fri, Jan 11, 2019 at 10:12:31AM +0100, Joerg Roedel wrote: > > On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote: > > > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they > > > need to limit the size of pages. > > > > That function just exports the overall size of the swiotlb aperture, no? > > What I need here is the maximum size for a single mapping. > > Yes. The other drivers just assumed that if there is SWIOTLB they would use > the smaller size by default (as in they knew the limitation). > > But I agree it would be better to have something smarter - and also convert the > DRM drivers to piggy back on this. > > Or alternatively we could make SWIOTLB handle bigger sizes.. Just a thought: is it a good idea to teach blk_queue_max_segment_size to get the dma size? This will help us find other devices possibly missing this check. > > > > Regards, > > > > Joerg
On Mon, Jan 14, 2019 at 04:59:27PM -0500, Michael S. Tsirkin wrote: > On Mon, Jan 14, 2019 at 03:49:07PM -0500, Konrad Rzeszutek Wilk wrote: > > On Fri, Jan 11, 2019 at 10:12:31AM +0100, Joerg Roedel wrote: > > > On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote: > > > > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they > > > > need to limit the size of pages. > > > > > > That function just exports the overall size of the swiotlb aperture, no? > > > What I need here is the maximum size for a single mapping. > > > > Yes. The other drivers just assumed that if there is SWIOTLB they would use > > the smaller size by default (as in they knew the limitation). > > > > But I agree it would be better to have something smarter - and also convert the > > DRM drivers to piggy back on this. > > > > Or alternatively we could make SWIOTLB handle bigger sizes.. > > > Just a thought: is it a good idea to teach blk_queue_max_segment_size > to get the dma size? This will help us find other devices > possibly missing this check. Yes, we should. Both the existing DMA size communicated through dma_params which is set by the driver, and this new DMA-ops exposed one which needs to be added. I'm working on some preliminary patches for the first part, as I think I introduced a bug related to that in the SCSI layer in 5.0..
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 7c007ed7505f..0bcc80a97036 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -72,6 +72,11 @@ static inline bool is_swiotlb_buffer(phys_addr_t paddr) return paddr >= io_tlb_start && paddr < io_tlb_end; } +static inline size_t swiotlb_max_alloc_size(void) +{ + return ((1UL << IO_TLB_SHIFT) * IO_TLB_SEGSIZE); +} + bool swiotlb_map(struct device *dev, phys_addr_t *phys, dma_addr_t *dma_addr, size_t size, enum dma_data_direction dir, unsigned long attrs); void __init swiotlb_exit(void); @@ -95,6 +100,13 @@ static inline unsigned int swiotlb_max_segment(void) { return 0; } + +static inline size_t swiotlb_max_alloc_size(void) +{ + /* There is no limit when SWIOTLB isn't used */ + return ~0UL; +} + #endif /* CONFIG_SWIOTLB */ extern void swiotlb_print_info(void);