[PATCH v12 0/2] UIO support for dfl devices

[PATCH v12 0/2] UIO support for dfl devices

[Xu Yilun]

This patchset supports some dfl device drivers written in userspace.

There are some Q&A about why UIO driver is needed in v11:

From Greg:
  Why are you saying that an ethernet driver should be using the UIO
  interface?

  And why can't you use the existing UIO drivers that bind to memory
  regions specified by firmware?  Without an interrupt being used, why is
  UIO needed at all?

From Moritz:
  Essentially I see two options:
  - Have a DFL bus driver instantiate a platform driver (uio_pdrv_genirq)
    which I *think* you described above?
  - What this patch implements -- a UIO driver on the DFL bus

  These FPGA devices can on the fly change their contents and -- even if
  just for test -- being able to expose a bunch of registers via UIO can
  be extremely useful.

  Whether a device should expose registers or not should be up to the
  implemeneter of the FPGA design I think (policy). This patch (or the
  previous version) provides a mechanism to do so via DFL.

  This is similar in nature to uio_pdrv_genirq on a DT based platform, to
  expose the registers you instantiate the DT node.

  Re-implementing a new driver for each of these instances doesn't seem
  desirable and tying DFL as enumeration mechanism to UIO seems like a
  good compromise for enabling this kind of functionality.

  Note this is *not* an attempt to bypass the network stack or other
  existing subsystems.

See the original message in:
  https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m66ba2c96848e3dea38d1a4f16dfea3cb291f7975


From Yilun:
  The ETH GROUP IP is not designed as the full functional ethernet
  controller. It is specially developed for the Intel N3000 NIC. Since it
  is an FPGA based card, it is designed for the users to runtime reload
  part of the MAC layer logic developed by themselves, while the ETH GROUP
  is another part of the MAC which is not expected to be reloaded by
  customers, but it provides some configurations for software to work with
  the user logic.

  So I category the feature as the devices that "designed for specific
  purposes and does not fit into one of the standard kernel subsystems".
  Some related description could be found in Patch #2, to illustrate why
  using UIO for some DFL devices.

  There are now UIO drivers for PCI or platform devices, but in this case
  we are going to export a DFL(Device Feature List) bus device to
  userspace, a DFL driver for UIO is needed to bind to it.

See the original message in:
  https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m91b303fd61485644353fad1e1e9c11d528844684


Xu Yilun (2):
  uio: uio_dfl: add userspace i/o driver for DFL bus
  Documentation: fpga: dfl: Add description for DFL UIO support

 Documentation/fpga/dfl.rst | 26 ++++++++++++++++++
 MAINTAINERS                |  1 +
 drivers/uio/Kconfig        | 17 ++++++++++++
 drivers/uio/Makefile       |  1 +
 drivers/uio/uio_dfl.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 111 insertions(+)
 create mode 100644 drivers/uio/uio_dfl.c

-- 
2.7.4

[PATCH v12 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Xu Yilun]

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@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
---
v9: switch to add a uio driver in drivers/uio
v10: add the source file in MAINTAINERS
     more descriptive Kconfig header
     add detailed path for opae uio example
v11: no change
v12: rebase to 5.12-rc2, no change
---
 MAINTAINERS           |  1 +
 drivers/uio/Kconfig   | 17 +++++++++++++
 drivers/uio/Makefile  |  1 +
 drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+)
 create mode 100644 drivers/uio/uio_dfl.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85c..b58a469 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6992,6 +6992,7 @@ S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-dfl*
 F:	Documentation/fpga/dfl.rst
 F:	drivers/fpga/dfl*
+F:	drivers/uio/uio_dfl.c
 F:	include/linux/dfl.h
 F:	include/uapi/linux/fpga-dfl.h
 
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 202ee81..5531f3a 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -165,4 +165,21 @@ 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 (Device Feature List) bus"
+	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
+
+	  It could be found at:
+
+	    opae-sdk/tools/libopaeuio/
+
+	  If you compile this as a module, it will be called uio_dfl.
 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");
-- 
2.7.4

[PATCH v12 2/2] Documentation: fpga: dfl: Add description for DFL UIO support

[Xu Yilun]

This patch adds description for UIO support for dfl devices on DFL
bus.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
Reviewed-by: Wu Hao <hao.wu@intel.com>
---
v2: no doc in v1, add it for v2.
v3: some documentation fixes.
v4: documentation change since the driver matching is changed.
v5: no change.
v6: improve the title of the userspace driver support section.
    some word improvement.
v7: rebased to next-20210119
v8: some doc fixes.
v9: some doc change since we switch to the driver in drivers/uio.
v10: no change.
v11: add description that interrupt support is not implemented yet.
v12: rebase to 5.12-rc2, no change
---
 Documentation/fpga/dfl.rst | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index c41ac76..f3a1223 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -7,6 +7,7 @@ Authors:
 - Enno Luebbers <enno.luebbers@intel.com>
 - Xiao Guangrong <guangrong.xiao@linux.intel.com>
 - Wu Hao <hao.wu@intel.com>
+- Xu Yilun <yilun.xu@intel.com>
 
 The Device Feature List (DFL) FPGA framework (and drivers according to
 this framework) hides the very details of low layer hardwares and provides
@@ -530,6 +531,31 @@ Being able to specify more than one DFL per BAR has been considered, but it
 was determined the use case did not provide value.  Specifying a single DFL
 per BAR simplifies the implementation and allows for extra error checking.
 
+
+Userspace driver support for DFL devices
+========================================
+The purpose of an FPGA is to be reprogrammed with newly developed hardware
+components. New hardware can instantiate a new private feature in the DFL, and
+then present a DFL device in the system. In some cases users may need a
+userspace driver for the DFL device:
+
+* Users may need to run some diagnostic test for their hardware.
+* Users may prototype the kernel driver in user space.
+* Some hardware is designed for specific purposes and does not fit into one of
+  the standard kernel subsystems.
+
+This requires direct access to MMIO space and interrupt handling from
+userspace. The uio_dfl module exposes the UIO device interfaces for this
+purpose.
+
+Currently the uio_dfl driver only supports the Ether Group sub feature, which
+has no irq in hardware. So the interrupt handling is not added in this driver.
+
+UIO_DFL should be selected to enable the uio_dfl module driver. To support a
+new DFL feature via UIO direct access, its feature id should be added to the
+driver's id_table.
+
+
 Open discussion
 ===============
 FME driver exports one ioctl (DFL_FPGA_FME_PORT_PR) for partial reconfiguration
-- 
2.7.4

Re: [PATCH v12 0/2] UIO support for dfl devices

[Xu Yilun]

Hi Greg:

I listed below some answers from Moritz and Yilun from previous mails for
your question.

Do you have more comments?

Thanks in advance,
Yilun

On Mon, Mar 08, 2021 at 09:59:34AM +0800, Xu Yilun wrote:
> This patchset supports some dfl device drivers written in userspace.
> 
> There are some Q&A about why UIO driver is needed in v11:
> 
> >From Greg:
>   Why are you saying that an ethernet driver should be using the UIO
>   interface?
> 
>   And why can't you use the existing UIO drivers that bind to memory
>   regions specified by firmware?  Without an interrupt being used, why is
>   UIO needed at all?
> 
> >From Moritz:
>   Essentially I see two options:
>   - Have a DFL bus driver instantiate a platform driver (uio_pdrv_genirq)
>     which I *think* you described above?
>   - What this patch implements -- a UIO driver on the DFL bus
> 
>   These FPGA devices can on the fly change their contents and -- even if
>   just for test -- being able to expose a bunch of registers via UIO can
>   be extremely useful.
> 
>   Whether a device should expose registers or not should be up to the
>   implemeneter of the FPGA design I think (policy). This patch (or the
>   previous version) provides a mechanism to do so via DFL.
> 
>   This is similar in nature to uio_pdrv_genirq on a DT based platform, to
>   expose the registers you instantiate the DT node.
> 
>   Re-implementing a new driver for each of these instances doesn't seem
>   desirable and tying DFL as enumeration mechanism to UIO seems like a
>   good compromise for enabling this kind of functionality.
> 
>   Note this is *not* an attempt to bypass the network stack or other
>   existing subsystems.
> 
> See the original message in:
>   https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m66ba2c96848e3dea38d1a4f16dfea3cb291f7975
> 
> 
> >From Yilun:
>   The ETH GROUP IP is not designed as the full functional ethernet
>   controller. It is specially developed for the Intel N3000 NIC. Since it
>   is an FPGA based card, it is designed for the users to runtime reload
>   part of the MAC layer logic developed by themselves, while the ETH GROUP
>   is another part of the MAC which is not expected to be reloaded by
>   customers, but it provides some configurations for software to work with
>   the user logic.
> 
>   So I category the feature as the devices that "designed for specific
>   purposes and does not fit into one of the standard kernel subsystems".
>   Some related description could be found in Patch #2, to illustrate why
>   using UIO for some DFL devices.
> 
>   There are now UIO drivers for PCI or platform devices, but in this case
>   we are going to export a DFL(Device Feature List) bus device to
>   userspace, a DFL driver for UIO is needed to bind to it.
> 
> See the original message in:
>   https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m91b303fd61485644353fad1e1e9c11d528844684
> 
> 
> Xu Yilun (2):
>   uio: uio_dfl: add userspace i/o driver for DFL bus
>   Documentation: fpga: dfl: Add description for DFL UIO support
> 
>  Documentation/fpga/dfl.rst | 26 ++++++++++++++++++
>  MAINTAINERS                |  1 +
>  drivers/uio/Kconfig        | 17 ++++++++++++
>  drivers/uio/Makefile       |  1 +
>  drivers/uio/uio_dfl.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 111 insertions(+)
>  create mode 100644 drivers/uio/uio_dfl.c
> 
> -- 
> 2.7.4

Re: [PATCH v12 0/2] UIO support for dfl devices

[Greg KH]

On Tue, Mar 16, 2021 at 01:10:05PM +0800, Xu Yilun wrote:
> Hi Greg:
> 
> I listed below some answers from Moritz and Yilun from previous mails for
> your question.
> 
> Do you have more comments?

Nah, it's fine, now queued up, thanks.

greg k-h

Re: [PATCH v12 0/2] UIO support for dfl devices

[Xu Yilun]

Hi Moritz:

Sorry I need to get back to you again, seems no more comments from Greg.

The patchset is stuck here for more than 1 month. Do you have some
more suggestion that could make it move forward? Do you have some more
comments? Or give an acked-by? Or could you apply it to your fpga branch
and go with next pull request?

Thanks,
Yilun

On Mon, Mar 08, 2021 at 09:59:34AM +0800, Xu Yilun wrote:
> This patchset supports some dfl device drivers written in userspace.
> 
> There are some Q&A about why UIO driver is needed in v11:
> 
> >From Greg:
>   Why are you saying that an ethernet driver should be using the UIO
>   interface?
> 
>   And why can't you use the existing UIO drivers that bind to memory
>   regions specified by firmware?  Without an interrupt being used, why is
>   UIO needed at all?
> 
> >From Moritz:
>   Essentially I see two options:
>   - Have a DFL bus driver instantiate a platform driver (uio_pdrv_genirq)
>     which I *think* you described above?
>   - What this patch implements -- a UIO driver on the DFL bus
> 
>   These FPGA devices can on the fly change their contents and -- even if
>   just for test -- being able to expose a bunch of registers via UIO can
>   be extremely useful.
> 
>   Whether a device should expose registers or not should be up to the
>   implemeneter of the FPGA design I think (policy). This patch (or the
>   previous version) provides a mechanism to do so via DFL.
> 
>   This is similar in nature to uio_pdrv_genirq on a DT based platform, to
>   expose the registers you instantiate the DT node.
> 
>   Re-implementing a new driver for each of these instances doesn't seem
>   desirable and tying DFL as enumeration mechanism to UIO seems like a
>   good compromise for enabling this kind of functionality.
> 
>   Note this is *not* an attempt to bypass the network stack or other
>   existing subsystems.
> 
> See the original message in:
>   https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m66ba2c96848e3dea38d1a4f16dfea3cb291f7975
> 
> 
> >From Yilun:
>   The ETH GROUP IP is not designed as the full functional ethernet
>   controller. It is specially developed for the Intel N3000 NIC. Since it
>   is an FPGA based card, it is designed for the users to runtime reload
>   part of the MAC layer logic developed by themselves, while the ETH GROUP
>   is another part of the MAC which is not expected to be reloaded by
>   customers, but it provides some configurations for software to work with
>   the user logic.
> 
>   So I category the feature as the devices that "designed for specific
>   purposes and does not fit into one of the standard kernel subsystems".
>   Some related description could be found in Patch #2, to illustrate why
>   using UIO for some DFL devices.
> 
>   There are now UIO drivers for PCI or platform devices, but in this case
>   we are going to export a DFL(Device Feature List) bus device to
>   userspace, a DFL driver for UIO is needed to bind to it.
> 
> See the original message in:
>   https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m91b303fd61485644353fad1e1e9c11d528844684
> 
> 
> Xu Yilun (2):
>   uio: uio_dfl: add userspace i/o driver for DFL bus
>   Documentation: fpga: dfl: Add description for DFL UIO support
> 
>  Documentation/fpga/dfl.rst | 26 ++++++++++++++++++
>  MAINTAINERS                |  1 +
>  drivers/uio/Kconfig        | 17 ++++++++++++
>  drivers/uio/Makefile       |  1 +
>  drivers/uio/uio_dfl.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 111 insertions(+)
>  create mode 100644 drivers/uio/uio_dfl.c
> 
> -- 
> 2.7.4

Re: [PATCH v12 0/2] UIO support for dfl devices

[Moritz Fischer]

Hi Xu,

On Wed, Mar 24, 2021 at 04:22:17PM +0800, Xu Yilun wrote:
> Hi Moritz:
> 
> Sorry I need to get back to you again, seems no more comments from Greg.
> 
> The patchset is stuck here for more than 1 month. Do you have some
> more suggestion that could make it move forward? Do you have some more
> comments? Or give an acked-by? Or could you apply it to your fpga branch
> and go with next pull request?

In its current form it's a UIO driver and needs at least Greg's Acked-by
before I could apply it.

Greg, can you take another look?

> 
> Thanks,
> Yilun
> 
> On Mon, Mar 08, 2021 at 09:59:34AM +0800, Xu Yilun wrote:
> > This patchset supports some dfl device drivers written in userspace.
> > 
> > There are some Q&A about why UIO driver is needed in v11:
> > 
> > >From Greg:
> >   Why are you saying that an ethernet driver should be using the UIO
> >   interface?
> > 
> >   And why can't you use the existing UIO drivers that bind to memory
> >   regions specified by firmware?  Without an interrupt being used, why is
> >   UIO needed at all?
> > 
> > >From Moritz:
> >   Essentially I see two options:
> >   - Have a DFL bus driver instantiate a platform driver (uio_pdrv_genirq)
> >     which I *think* you described above?
> >   - What this patch implements -- a UIO driver on the DFL bus
> > 
> >   These FPGA devices can on the fly change their contents and -- even if
> >   just for test -- being able to expose a bunch of registers via UIO can
> >   be extremely useful.
> > 
> >   Whether a device should expose registers or not should be up to the
> >   implemeneter of the FPGA design I think (policy). This patch (or the
> >   previous version) provides a mechanism to do so via DFL.
> > 
> >   This is similar in nature to uio_pdrv_genirq on a DT based platform, to
> >   expose the registers you instantiate the DT node.
> > 
> >   Re-implementing a new driver for each of these instances doesn't seem
> >   desirable and tying DFL as enumeration mechanism to UIO seems like a
> >   good compromise for enabling this kind of functionality.
> > 
> >   Note this is *not* an attempt to bypass the network stack or other
> >   existing subsystems.
> > 
> > See the original message in:
> >   https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m66ba2c96848e3dea38d1a4f16dfea3cb291f7975
> > 
> > 
> > >From Yilun:
> >   The ETH GROUP IP is not designed as the full functional ethernet
> >   controller. It is specially developed for the Intel N3000 NIC. Since it
> >   is an FPGA based card, it is designed for the users to runtime reload
> >   part of the MAC layer logic developed by themselves, while the ETH GROUP
> >   is another part of the MAC which is not expected to be reloaded by
> >   customers, but it provides some configurations for software to work with
> >   the user logic.
> > 
> >   So I category the feature as the devices that "designed for specific
> >   purposes and does not fit into one of the standard kernel subsystems".
> >   Some related description could be found in Patch #2, to illustrate why
> >   using UIO for some DFL devices.
> > 
> >   There are now UIO drivers for PCI or platform devices, but in this case
> >   we are going to export a DFL(Device Feature List) bus device to
> >   userspace, a DFL driver for UIO is needed to bind to it.
> > 
> > See the original message in:
> >   https://lore.kernel.org/linux-fpga/YDvQ8aO8v3NhLKzx@epycbox.lan/T/#m91b303fd61485644353fad1e1e9c11d528844684
> > 
> > 
> > Xu Yilun (2):
> >   uio: uio_dfl: add userspace i/o driver for DFL bus
> >   Documentation: fpga: dfl: Add description for DFL UIO support
> > 
> >  Documentation/fpga/dfl.rst | 26 ++++++++++++++++++
> >  MAINTAINERS                |  1 +
> >  drivers/uio/Kconfig        | 17 ++++++++++++
> >  drivers/uio/Makefile       |  1 +
> >  drivers/uio/uio_dfl.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 111 insertions(+)
> >  create mode 100644 drivers/uio/uio_dfl.c
> > 
> > -- 
> > 2.7.4

Thanks,
Moritz