[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen/swiotlb: add alignment check for dma buffers
On Fri, 13 Sep 2024, Jan Beulich wrote: > On 13.09.2024 16:56, Juergen Gross wrote: > > --- a/drivers/xen/swiotlb-xen.c > > +++ b/drivers/xen/swiotlb-xen.c > > @@ -78,9 +78,15 @@ static inline int > > range_straddles_page_boundary(phys_addr_t p, size_t size) > > { > > unsigned long next_bfn, xen_pfn = XEN_PFN_DOWN(p); > > unsigned int i, nr_pages = XEN_PFN_UP(xen_offset_in_page(p) + size); > > + unsigned int order = get_order(size); > > > > next_bfn = pfn_to_bfn(xen_pfn); > > > > + /* If buffer is physically aligned, ensure DMA alignment. */ > > + if (IS_ALIGNED(p, 1UL << (order + PAGE_SHIFT)) && > > Why this check? xen_swiotlb_alloc_coherent() guarantees it, while > xen_swiotlb_free_coherent() only checks properties of the original > allocation. And for xen_swiotlb_map_page() this looks actively > wrong to me, in case that function was called with offset non-zero. I understand xen_swiotlb_alloc_coherent and xen_swiotlb_free_coherent not needing the check, but I think we might need the check for xen_swiotlb_map_page. At that point, I would keep the check for all callers. Unless there is another way to detect whether the mapping needs alignment specifically for map_page? For the offset, in theory if the device needs alignment, the offset should be zero? If the offset is not zero, then there should be no alignment requirement. The way Juergen wrote the check, we would take the fast path if offset != zero, which makes sense to me.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |