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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <wddmyx4lccthln7mbngkd4zbh6y7mwetdba5e7ob6u4xevecmj@zopp65eqbeuu>
Date: Mon, 30 Jun 2025 16:18:51 +0800
From: Xu Yang <xu.yang_2@....com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: ezequiel@...guardiasur.com.ar, mchehab@...nel.org, 
	laurent.pinchart@...asonboard.com, hdegoede@...hat.com, gregkh@...uxfoundation.org, 
	mingo@...nel.org, tglx@...utronix.de, andriy.shevchenko@...ux.intel.com, 
	viro@...iv.linux.org.uk, thomas.weissschuh@...utronix.de, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org, imx@...ts.linux.dev, jun.li@....com
Subject: Re: [PATCH v2 1/3] usb: core: add dma-noncoherent buffer alloc and
 free API

Hi Alan,

On Fri, Jun 27, 2025 at 10:23:36AM -0400, Alan Stern wrote:
> On Fri, Jun 27, 2025 at 06:19:37PM +0800, Xu Yang wrote:
> > This will add usb_alloc_noncoherent() and usb_free_noncoherent()
> > functions to support alloc and free buffer in a dma-noncoherent way.
> > 
> > To explicit manage the memory ownership for the kernel and device,
> > this will also add usb_dma_noncoherent_sync_for_cpu/device() functions
> > and call it at proper time.  The management requires the user save
> > sg_table returned by usb_alloc_noncoherent() to urb->sgt.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@....com>
> > ---
> >  drivers/usb/core/hcd.c | 30 ++++++++++++++++
> >  drivers/usb/core/usb.c | 80 ++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/usb.h    |  9 +++++
> >  3 files changed, 119 insertions(+)
> > 
> > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > index c22de97432a0..5fa00d32afb8 100644
> > --- a/drivers/usb/core/hcd.c
> > +++ b/drivers/usb/core/hcd.c
> > @@ -1496,6 +1496,34 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> >  }
> >  EXPORT_SYMBOL_GPL(usb_hcd_map_urb_for_dma);
> >  
> > +static void usb_dma_noncoherent_sync_for_cpu(struct usb_hcd *hcd,
> > +					     struct urb *urb)
> > +{
> > +	enum dma_data_direction dir;
> > +
> > +	if (!urb->sgt)
> > +		return;
> > +
> > +	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> 
> Are the following operations really necessary if the direction is OUT?  
> There are no bidirectional URBs, and an OUT transfer never modifies the 
> contents of the transfer buffer so the buffer contents will be the same 
> after the URB completes as they were when the URB was submitted.

According to Laurent's reply email, it may be needed for some archs.

> 
> > +	invalidate_kernel_vmap_range(urb->transfer_buffer,
> > +				     urb->transfer_buffer_length);
> > +	dma_sync_sgtable_for_cpu(hcd->self.sysdev, urb->sgt, dir);
> > +}
> 
> This entire routine should be inserted at the appropriate place in 
> usb_hcd_unmap_urb_for_dma() instead of being standalone.

Okay.

> 
> > +static void usb_dma_noncoherent_sync_for_device(struct usb_hcd *hcd,
> > +						struct urb *urb)
> > +{
> > +	enum dma_data_direction dir;
> > +
> > +	if (!urb->sgt)
> > +		return;
> > +
> > +	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> > +	flush_kernel_vmap_range(urb->transfer_buffer,
> > +				urb->transfer_buffer_length);
> > +	dma_sync_sgtable_for_device(hcd->self.sysdev, urb->sgt, dir);
> > +}
> 
> Likewise, this code belongs inside usb_hcd_map_urb_for_dma().

Okay.

> 
> 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.

Thanks,
Xu Yang

> 
> Alan Stern