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: <jrsw27uoc3wo7b7ry4mgkak2feyrvznad2b2pbmr3coabpulgy@lyfkfj3oik5l>
Date: Mon, 30 Jun 2025 15:26:26 +0800
From: Xu Yang <xu.yang_2@....com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: ezequiel@...guardiasur.com.ar, mchehab@...nel.org, 
	laurent.pinchart@...asonboard.com, hdegoede@...hat.com, gregkh@...uxfoundation.org, 
	mingo@...nel.org, tglx@...utronix.de, 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 Andy,

Thanks for your comments!

On Fri, Jun 27, 2025 at 02:22:59PM +0300, Andy Shevchenko 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.
> 
> ...
> 
> > +/**
> > + * usb_alloc_noncoherent - allocate dma-noncoherent buffer for URB_NO_xxx_DMA_MAP
> > + * @dev: device the buffer will be used with
> > + * @size: requested buffer size
> > + * @mem_flags: affect whether allocation may block
> > + * @dma: used to return DMA address of buffer
> > + * @dir: dma transfer direction
> > + * @table: used to return sg_table of allocated memory
> > + *
> > + * Return: Either null (indicating no buffer could be allocated), or the
> > + * cpu-space pointer to a buffer that may be used to perform DMA to the
> > + * specified device.  Such cpu-space buffers are returned along with the DMA
> > + * address (through the pointer provided).
> 
> Return section should be last in the kernel-doc (this requirement is
> documented).

Okay. I'll improve it.

> 
> > + * To explicit manage the memory ownership for the kernel vs the device by
> > + * usb core, the user needs save sg_table to urb->sgt. Then usb core will
> > + * do dma sync for cpu and device properly.
> > + *
> > + * When the buffer is no longer used, free it with usb_free_noncoherent().
> 
> Here and everywhere else in your series, pay respect to acronyms by using them
> as acronyms:
> 
> dma --> DMA
> cpu --> CPU
> usb --> USB
> 
> and so on...

Okay.

> 
> 
> > + */
> > +void *usb_alloc_noncoherent(struct usb_device *dev, size_t size,
> > +			    gfp_t mem_flags, dma_addr_t *dma,
> > +			    enum dma_data_direction dir,
> > +			    struct sg_table **table)
> > +{
> > +	struct device *dmadev;
> > +	struct sg_table *sgt;
> > +	void *buffer;
> > +
> > +	if (!dev || !dev->bus)
> 
> When !dev case is possible?

Not exactly sure, but it depends on the user. This sanity test is also
carried over from usb_alloc_coherent() to avoid any surprise.

> 
> > +		return NULL;
> > +
> > +	dmadev = bus_to_hcd(dev->bus)->self.sysdev;
> > +
> > +	sgt = dma_alloc_noncontiguous(dmadev, size, dir, mem_flags, 0);
> > +	if (!sgt)
> > +		return NULL;
> > +
> > +	buffer = dma_vmap_noncontiguous(dmadev, size, sgt);
> > +	if (!buffer) {
> > +		dma_free_noncontiguous(dmadev, size, sgt, dir);
> > +		return NULL;
> > +	}
> > +
> > +	*table = sgt;
> > +	*dma = sg_dma_address(sgt->sgl);
> > +
> > +	return buffer;
> > +}
> 
> ...
> 
> > +void usb_free_noncoherent(struct usb_device *dev, size_t size,
> > +			  void *addr, enum dma_data_direction dir,
> > +			  struct sg_table *table)
> > +{
> > +	struct device *dmadev;
> > +
> > +	if (!dev || !dev->bus)
> 
> Ditto.
> 
> > +		return;
> > +	if (!addr)
> > +		return;
> > +
> > +	dmadev = bus_to_hcd(dev->bus)->self.sysdev;
> > +	dma_vunmap_noncontiguous(dmadev, addr);
> > +	dma_free_noncontiguous(dmadev, size, table, dir);
> > +}

Thanks,
Xu Yang

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ