Re: [PATCH v4] Add new uio device for PCI with dynamic memory allocation

[Hongren Zheng <i@zenithal.me>]

On Wed, Apr 29, 2020 at 01:53:02PM +0000, Stahl, Manuel wrote:
> On Mi, 2020-04-29 at 11:41 +0200, gregkh@linuxfoundation.org wrote:
> > On Wed, Apr 29, 2020 at 07:51:01AM +0000, Stahl, Manuel wrote:
> > > On Di, 2020-04-28 at 15:54 +0200, gregkh @ linuxfoundation . org wrote:
> > > > On Thu, Apr 16, 2020 at 06:38:30PM +0200, Manuel Stahl wrote:
> > > > > 
> > > > > + *
> > > > > + * Since the driver does not declare any device ids, you must allocate
> > > > > + * id and bind the device to the driver yourself.  For example:
> > > > > + *
> > > > > + * # echo "8086 10f5" > /sys/bus/pci/drivers/uio_pci_dmem_genirq/new_id
> > > > > + * # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/e1000e/unbind
> > > > > + * # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/uio_pci_dmem_genirq/bind
> > > > > + * # ls -l /sys/bus/pci/devices/0000:00:19.0/driver
> > > > > + * .../0000:00:19.0/driver -> ../../../bus/pci/drivers/uio_pci_dmem_genirq
> > > > > + *
> > > > > + * Or use a modprobe alias:
> > > > > + * # alias pci:v000010EEd00001000sv*sd*sc*i* uio_pci_dmem_genirq
> > > > > + *
> > > > > + * Driver won't bind to devices which do not support the Interrupt Disable Bit
> > > > > + * in the command register. All devices compliant to PCI 2.3 (circa 2002) and
> > > > > + * all compliant PCI Express devices should support this bit.
> > > > > + *
> > > > > + * The DMA mask bits and sizes of dynamic regions are derived from module
> > > > > + * parameters.
> > > > > + *
> > > > > + * The format for specifying dynamic region sizes in module parameters
> > > > > + * is as follows:
> > > > > + *
> > > > > + * uio_pci_dmem_genirq.dmem_sizes := <uio_dmem_sizes_def>[;<uio_dmem_sizes_def>]
> > > > > + * <uio_dmem_sizes_def>           := <pci_id>:<size>[,<size>]
> > > > > + * <pci_id>                       := <vendor>:<device>
> > > > > + * <size>                         := standard linux memsize
> > > > > + *
> > > > > + * Examples:
> > > > > + *
> > > > > + * 1) UIO dmem device with 3 dynamic regions:
> > > > > + * uio_pci_dmem_genirq.dmem_sizes=8086:10f5:4K,16K,4M
> > > > > + *
> > > > > + * 2) Two UIO dmem devices with different number of dynamic regions:
> > > > > + * uio_pci_dmem_genirq.dmem_sizes=8086:10f5:4K,16K,4M;1234:0001:8K
> > > > 
> > > > Module parameters are horrid, are you sure there is no other way?
> > > 
> > > You're right, seemed to be the simplest solution back when we started developing this driver. I will try to change it to sysfs, so that one can add regions while the module is already loaded.
> > 
> > /me hands you some \n characters...
> > 
> > Anyway, configfs is for configuring stuff, don't make a sysfs file that
> > you have to somehow "parse" please.
> 
> Looking back at this driver after some years I realized again the reason
> for using kernel parameters:
> 
> The current UIO API needs the information about available memory maps when
> registering a new UIO device with __uio_register_device(), which obviously
> needs to be called during probe() in uio_pci_dmem_genirq. Otherwise there
> is no device file in /dev to open for user space applications.
> 
> After that there is no function to update the uio_map info. So we can either
> keep the module parameters and allocate the DMA memory during probe() or
> allocate the DMA memory during mmap() and
>   a) replicate parts of uio_dev_add_attributes() in this driver to update sysfs
>   b) add a function in uio.c to allow updates to the uio_map
> 
> Which way would you go?
> 
> Best regards,
> Manuel Stahl

I have similar need for our FPGA project where DMA to
userspace is wanted, how could this be moved forward?

Cc'ed my collaborator here.

It seems that there is not enough maintainance bandwidth for the UIO driver.
I'm willing to be a reviewer, and I assume after handling some of the
patches I can manage to do it. I wonder what else should be done.