Re: [PATCH v2 1/3] usb: core: add dma-noncoherent buffer alloc and free API

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Alan,

On Mon, Jun 30, 2025 at 10:16:30AM -0400, Alan Stern wrote:
> On Mon, Jun 30, 2025 at 04:18:51PM +0800, Xu Yang wrote:
> > > Also, the material that this routine replaces in the uvc and stk1160 
> > > drivers do not call flush_kernel_vmap_range().  Why did you add that 
> > > here?  Was this omission a bug in those drivers?
> > 
> > According to dma-api.rst:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/tree/Documentation/core-api/dma-api.rst?h=linux-6.15.y#n664
> > 
> > "Once a non-contiguous allocation is mapped using this function, the
> > flush_kernel_vmap_range() and invalidate_kernel_vmap_range() APIs must
> > be used to manage the coherency between the kernel mapping, the device
> > and user space mappings (if any)."
> > 
> > Possibly the uvc and stk1160 missed calling it, but since they won't be
> > the only user of the USB core, so we'd better call these APIs.
> 
> Documentation/core-api/cachetbl.rst says:
> 
>   ``void flush_kernel_vmap_range(void *vaddr, int size)``
> 
>        flushes the kernel cache for a given virtual address range in
>        the vmap area.  This is to make sure that any data the kernel
>        modified in the vmap range is made visible to the physical
>        page.  The design is to make this area safe to perform I/O on.
>        Note that this API does *not* also flush the offset map alias
>        of the area.
> 
>   ``void invalidate_kernel_vmap_range(void *vaddr, int size) invalidates``
> 
>        the cache for a given virtual address range in the vmap area
>        which prevents the processor from making the cache stale by
>        speculatively reading data while the I/O was occurring to the
>        physical pages.  This is only necessary for data reads into the
>        vmap area.
> 
> So invalidate_kernel_vmap_range() is not needed for data writes, that 
> is, for OUT transfers.  And ironically, flush_kernel_vmap_range() _is_ 
> needed (but only for OUT transfers).

flush_kernel_vmap_range() for OUT transfers and
invalidate_kernel_vmap_range() for IN transfers make sense to me.

> On the other hand, Christoph may think these call should be included 
> regardless.  Let's see what he recommends.  Christoph?
> 
> (Actually, I would expect dma_sync_sgtable_for_cpu() and 
> dma_sync_sgtable_for_device() to handle all of this for us 
> automatically, but never mind...)

-- 
Regards,

Laurent Pinchart