linux-kernel - Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

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: <YA98/8r+yOCurHAJ@epycbox.lan>
Date:   Mon, 25 Jan 2021 18:22:55 -0800
From:   Moritz Fischer <mdf@...nel.org>
To:     Tom Rix <trix@...hat.com>
Cc:     Xu Yilun <yilun.xu@...el.com>, gregkh@...uxfoundation.org,
        mdf@...nel.org, linux-fpga@...r.kernel.org,
        linux-kernel@...r.kernel.org, lgoncalv@...hat.com, hao.wu@...el.com
Subject: Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
> 
> On 1/25/21 12:49 AM, Xu Yilun wrote:
> > This patch supports the DFL drivers be written in userspace. This is
> > realized by exposing the userspace I/O device interfaces.
> >
> > The driver now only binds the ether group feature, which has no irq. So
> > the irq support is not implemented yet.
> >
> > Signed-off-by: Xu Yilun <yilun.xu@...el.com>
> > ---
> > v9: switch to add a uio driver in drivers/uio
> > ---
> >  drivers/uio/Kconfig   | 13 ++++++++++
> >  drivers/uio/Makefile  |  1 +
> >  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> You should add this to the MAINTAINERS file.

This is covered by MAINTAINERS under drivers/uio.
> 
> >  3 files changed, 80 insertions(+)
> >  create mode 100644 drivers/uio/uio_dfl.c
> >
> > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > index 202ee81..44778f8 100644
> > --- a/drivers/uio/Kconfig
> > +++ b/drivers/uio/Kconfig
> > @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
> >  	  to network and storage devices from userspace.
> >  
> >  	  If you compile this as a module, it will be called uio_hv_generic.
> > +
> > +config UIO_DFL
> > +	tristate "Generic driver for DFL bus"
> 
> The term 'DFL' will be unknown to folks in drivers/uio
> 
> I think it would be better if DFL was always prefixed 'FPGA DFL'
> 
> > +	depends on FPGA_DFL
> > +	help
> > +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
> > +	  It is useful to provide direct access to DFL devices from userspace.
> > +	  A sample userspace application using this driver is available for
> > +	  download in a git repository:
> > +
> > +	    git clone https://github.com/OPAE/opae-sdk.git
> > +
> > +	  If you compile this as a module, it will be called uio_dfl.

I'm not sure KConfig is the right place for this.
> 
> opae-sdk is pretty large and uncovered in the Documentation/fpga/dfl.rst.
> 
> Where in opae-sdk is this example ?
> 
> If you can point me at the example, I will turn it into a selftest.
> 
> Tom
> 
> >  endif
> > diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> > index c285dd2..f2f416a1 100644
> > --- a/drivers/uio/Makefile
> > +++ b/drivers/uio/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
> >  obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
> >  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
> >  obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
> > +obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> > diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
> > new file mode 100644
> > index 0000000..89c0fc7
> > --- /dev/null
> > +++ b/drivers/uio/uio_dfl.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Generic DFL driver for Userspace I/O devicess
> > + *
> > + * Copyright (C) 2021 Intel Corporation, Inc.
> > + */
> > +#include <linux/dfl.h>
> > +#include <linux/errno.h>
> > +#include <linux/module.h>
> > +#include <linux/uio_driver.h>
> > +
> > +#define DRIVER_NAME "uio_dfl"
> > +
> > +static int uio_dfl_probe(struct dfl_device *ddev)
> > +{
> > +	struct resource *r = &ddev->mmio_res;
> > +	struct device *dev = &ddev->dev;
> > +	struct uio_info *uioinfo;
> > +	struct uio_mem *uiomem;
> > +	int ret;
> > +
> > +	uioinfo = devm_kzalloc(dev, sizeof(struct uio_info), GFP_KERNEL);
> > +	if (!uioinfo)
> > +		return -ENOMEM;
> > +
> > +	uioinfo->name = DRIVER_NAME;
> > +	uioinfo->version = "0";
> > +
> > +	uiomem = &uioinfo->mem[0];
> > +	uiomem->memtype = UIO_MEM_PHYS;
> > +	uiomem->addr = r->start & PAGE_MASK;
> > +	uiomem->offs = r->start & ~PAGE_MASK;
> > +	uiomem->size = (uiomem->offs + resource_size(r)
> > +			+ PAGE_SIZE - 1) & PAGE_MASK;
> > +	uiomem->name = r->name;
> > +
> > +	/* Irq is yet to be supported */
> > +	uioinfo->irq = UIO_IRQ_NONE;
> > +
> > +	ret = devm_uio_register_device(dev, uioinfo);
> > +	if (ret)
> > +		dev_err(dev, "unable to register uio device\n");
> > +
> > +	return ret;
> > +}
> > +
> > +#define FME_FEATURE_ID_ETH_GROUP	0x10
> > +
> > +static const struct dfl_device_id uio_dfl_ids[] = {
> > +	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
> > +
> > +static struct dfl_driver uio_dfl_driver = {
> > +	.drv = {
> > +		.name = DRIVER_NAME,
> > +	},
> > +	.id_table	= uio_dfl_ids,
> > +	.probe		= uio_dfl_probe,
> > +};
> > +module_dfl_driver(uio_dfl_driver);
> > +
> > +MODULE_DESCRIPTION("Generic DFL driver for Userspace I/O devices");
> > +MODULE_AUTHOR("Intel Corporation");
> > +MODULE_LICENSE("GPL v2");
>