Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Xu, HuilongX]

Note that since igb_uio no longer has a PCI ID list, it can now be
bound to any device, not just those explicitly supported by DPDK. In
other words, it now behaves similar to PCI stub, VFIO and other generic
PCI drivers.

Therefore to bind a new device to igb_uio, the user will now have to
first write its PCI ID to "new_id" file inside the igb_uio driver
directory, and only then write the PCI ID to "bind". This will be
reflected in later changes to PCI binding script as well.

Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>

Test-by: HuilongX Xu <huilongx.xu at int.com>



Compile pass

     >>Compile OS: FC20 x86_64

     >>Kernel version: 3.13.6-200

     >>GCC version: 4.8.2

     >>Server: Crownpass
---
lib/librte_eal/linuxapp/igb_uio/igb_uio.c |   21 +--------------------
1 files changed, 1 insertions(+), 20 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 043c0f6..d30c94a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -65,25 +65,6 @@ struct rte_uio_pci_dev {
static char *intr_mode = NULL;
static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
-/* PCI device id table */
-static struct pci_device_id igbuio_pci_ids[] = {
-#define RTE_PCI_DEV_ID_DECL_EM(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IGBVF(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IXGBE(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IXGBEVF(vend, dev) {PCI_DEVICE(vend, dev)},
-#ifdef RTE_LIBRTE_VIRTIO_PMD
-#define RTE_PCI_DEV_ID_DECL_VIRTIO(vend, dev) {PCI_DEVICE(vend, dev)},
-#endif
-#ifdef RTE_LIBRTE_VMXNET3_PMD
-#define RTE_PCI_DEV_ID_DECL_VMXNET3(vend, dev) {PCI_DEVICE(vend, dev)},
-#endif
-#include <rte_pci_dev_ids.h>
-{ 0, },
-};
-
-MODULE_DEVICE_TABLE(pci, igbuio_pci_ids);
-
static inline struct rte_uio_pci_dev *
igbuio_get_uio_pci_dev(struct uio_info *info)
{
@@ -620,7 +601,7 @@ igbuio_config_intr_mode(char *intr_str)
 static struct pci_driver igbuio_pci_driver = {
               .name = "igb_uio",
-              .id_table = igbuio_pci_ids,
+             .id_table = NULL,
               .probe = igbuio_pci_probe,
               .remove = igbuio_pci_remove,
};
--
1.7.0.7

[dpdk-dev] [PATCH 00/16] [RFC] [VFIO] Add VFIO support to DPDK

[Burakov, Anatoly]

This patchset adds support for using VFIO instead of IGB_UIO to
map the device BARs.

VFIO is a kernel 3.6+ driver allowing secure DMA from userspace
by means of using IOMMU instead of working directly with physical
memory like igb_uio does.

Short summary:
* Adding support for VFIO in EAL PCI code
* Adding new command-line parameter for VFIO interrupt type
* Adding support for VFIO in setup.sh
* Renaming igb_uio_bind to dpdk_nic_bind and adding support for
  VFIO there
* Removing PCI ID list from igb_uio, effectively making it another
  generic PCI driver similar to pci_stub, vfio-pci et al
* Adding autotest for VFIO interrupt types
* Making igb_uio and VFIO compilation optional

I'm submitting this as an RFC because this patch is based off
current dpdk.org branch with David Marchand's RTE_EAL_UNBIND_PORTS
patchset. IOW, this will *not* apply to the dpdk.org tree *unless* you
also apply David's patches beforehand.

Signed-off by: Anatoly Burakov <anatoly.burakov@intel.com>

Anatoly Burakov (16):
  Separate igb_uio mapping into a separate file
  Distinguish between legitimate failures and non-fatal errors
  Rename RTE_PCI_DRV_NEED_IGB_UIO to RTE_PCI_DRV_NEED_MAPPING
  Make igb_uio compilation optional
  Moved interrupt type out of igb_uio
  Add support for VFIO in Linuxapp targets
  Add support for VFIO interrupts, add VFIO header
  Add support for mapping devices through VFIO.
  Enable VFIO device binding
  Added support for selecting VFIO interrupt type from EAL command-line
  Make --no-huge use mmap instead of malloc.
  Adding unit tests for VFIO EAL command-line parameter
  Removed PCI ID table from igb_uio
  Renamed igb_uio_bind to dpdk_nic_bind
  Added support for VFIO drivers in dpdk_nic_bind.py
  Adding support for VFIO to setup.sh

 app/test/test_eal_flags.c                          |  24 +
 app/test/test_pci.c                                |   4 +-
 config/defconfig_i686-default-linuxapp-gcc         |   2 +
 config/defconfig_i686-default-linuxapp-icc         |   2 +
 config/defconfig_x86_64-default-linuxapp-gcc       |   2 +
 config/defconfig_x86_64-default-linuxapp-icc       |   2 +
 lib/librte_eal/bsdapp/eal/eal_pci.c                |   2 +-
 lib/librte_eal/common/Makefile                     |   1 +
 lib/librte_eal/common/eal_common_pci.c             |  17 +-
 lib/librte_eal/common/include/rte_pci.h            |   7 +-
 .../common/include/rte_pci_dev_feature_defs.h      |  46 ++
 .../common/include/rte_pci_dev_features.h          |  42 ++
 lib/librte_eal/linuxapp/Makefile                   |   2 +
 lib/librte_eal/linuxapp/eal/Makefile               |   6 +-
 lib/librte_eal/linuxapp/eal/eal.c                  |  35 +
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 203 +++++-
 lib/librte_eal/linuxapp/eal/eal_memory.c           |   8 +-
 lib/librte_eal/linuxapp/eal/eal_pci.c              | 480 ++------------
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 416 ++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c         | 709 +++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_socket.c  | 367 +++++++++++
 .../linuxapp/eal/include/eal_internal_cfg.h        |   3 +
 lib/librte_eal/linuxapp/eal/include/eal_pci_init.h | 120 ++++
 lib/librte_eal/linuxapp/eal/include/eal_vfio.h     |  55 ++
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |   7 +-
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |  70 +-
 lib/librte_pmd_e1000/em_ethdev.c                   |   2 +-
 lib/librte_pmd_e1000/igb_ethdev.c                  |   4 +-
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c                |   4 +-
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c            |   2 +-
 tools/dpdk_nic_bind.py                             | 500 +++++++++++++++
 tools/igb_uio_bind.py                              | 485 --------------
 tools/setup.sh                                     | 168 ++++-
 33 files changed, 2797 insertions(+), 1000 deletions(-)
 create mode 100644 lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
 create mode 100644 lib/librte_eal/common/include/rte_pci_dev_features.h
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_uio.c
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio_socket.c
 create mode 100644 lib/librte_eal/linuxapp/eal/include/eal_pci_init.h
 create mode 100644 lib/librte_eal/linuxapp/eal/include/eal_vfio.h
 create mode 100755 tools/dpdk_nic_bind.py
 delete mode 100755 tools/igb_uio_bind.py

-- 
1.8.1.4

[dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Anatoly Burakov]

Note that since igb_uio no longer has a PCI ID list, it can now be
bound to any device, not just those explicitly supported by DPDK. In
other words, it now behaves similar to PCI stub, VFIO and other generic
PCI drivers.

Therefore to bind a new device to igb_uio, the user will now have to
first write its PCI ID to "new_id" file inside the igb_uio driver
directory, and only then write the PCI ID to "bind". This will be
reflected in later changes to PCI binding script as well.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 043c0f6..d30c94a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -65,25 +65,6 @@ struct rte_uio_pci_dev {
 static char *intr_mode = NULL;
 static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 
-/* PCI device id table */
-static struct pci_device_id igbuio_pci_ids[] = {
-#define RTE_PCI_DEV_ID_DECL_EM(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IGBVF(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IXGBE(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IXGBEVF(vend, dev) {PCI_DEVICE(vend, dev)},
-#ifdef RTE_LIBRTE_VIRTIO_PMD
-#define RTE_PCI_DEV_ID_DECL_VIRTIO(vend, dev) {PCI_DEVICE(vend, dev)},
-#endif
-#ifdef RTE_LIBRTE_VMXNET3_PMD
-#define RTE_PCI_DEV_ID_DECL_VMXNET3(vend, dev) {PCI_DEVICE(vend, dev)},
-#endif
-#include <rte_pci_dev_ids.h>
-{ 0, },
-};
-
-MODULE_DEVICE_TABLE(pci, igbuio_pci_ids);
-
 static inline struct rte_uio_pci_dev *
 igbuio_get_uio_pci_dev(struct uio_info *info)
 {
@@ -620,7 +601,7 @@ igbuio_config_intr_mode(char *intr_str)
 
 static struct pci_driver igbuio_pci_driver = {
 	.name = "igb_uio",
-	.id_table = igbuio_pci_ids,
+	.id_table = NULL,
 	.probe = igbuio_pci_probe,
 	.remove = igbuio_pci_remove,
 };
-- 
1.7.0.7

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Thomas Monjalon]

2014-05-19 16:51, Anatoly Burakov:
> Note that since igb_uio no longer has a PCI ID list, it can now be
> bound to any device, not just those explicitly supported by DPDK. In
> other words, it now behaves similar to PCI stub, VFIO and other generic
> PCI drivers.

I wonder if we could replace igb_uio by uio_pci_generic?

> Therefore to bind a new device to igb_uio, the user will now have to
> first write its PCI ID to "new_id" file inside the igb_uio driver
> directory, and only then write the PCI ID to "bind". This will be
> reflected in later changes to PCI binding script as well.

Please explain in the commit log why you are removing PCI ids from igb_uio.

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Burakov, Anatoly]

Hi Thomas,

> I wonder if we could replace igb_uio by uio_pci_generic?

Can it do DMA or IOMMU support? Not even VFIO does everything we need, you may have noticed that I have to go to PCI config space to enable bus mastering. I don't think that driver can do either of those things. Unless, of course, you meant simply not tying the binding script to specific driver, which in effect is already in place (although it does check whether vfio-pci or igb_uio are loaded).

Best regards,
Anatoly Burakov
DPDK SW Engineer

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Thomas Monjalon]

2014-05-22 13:24, Burakov, Anatoly:
> Hi Thomas,
> 
> > I wonder if we could replace igb_uio by uio_pci_generic?
> 
> Can it do DMA or IOMMU support? Not even VFIO does everything we need, you
> may have noticed that I have to go to PCI config space to enable bus
> mastering. I don't think that driver can do either of those things. Unless,
> of course, you meant simply not tying the binding script to specific
> driver, which in effect is already in place (although it does check whether
> vfio-pci or igb_uio are loaded).

Actually my question was: is there something in igb_uio which is not already 
handled by uio_pci_generic?

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Stephen Hemminger]

On Thu, 22 May 2014 15:13:49 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2014-05-19 16:51, Anatoly Burakov:
> > Note that since igb_uio no longer has a PCI ID list, it can now be
> > bound to any device, not just those explicitly supported by DPDK. In
> > other words, it now behaves similar to PCI stub, VFIO and other generic
> > PCI drivers.  
> 
> I wonder if we could replace igb_uio by uio_pci_generic?

Not as is. I am strarting a new driver for upstream kernel based of
pci_generic plus igb_uio. After discussion with Greg KH, doing a new
driver seems like best idea. 

PCI generic driver as is does not do interrupts, and does not
claim PCI resources from kernel.

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Thomas Monjalon]

2014-05-23 08:11, Stephen Hemminger:
> On Thu, 22 May 2014 15:13:49 +0200
> 
> Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > 2014-05-19 16:51, Anatoly Burakov:
> > > Note that since igb_uio no longer has a PCI ID list, it can now be
> > > bound to any device, not just those explicitly supported by DPDK. In
> > > other words, it now behaves similar to PCI stub, VFIO and other generic
> > > PCI drivers.
> > 
> > I wonder if we could replace igb_uio by uio_pci_generic?
> 
> Not as is. I am strarting a new driver for upstream kernel based of
> pci_generic plus igb_uio. After discussion with Greg KH, doing a new
> driver seems like best idea.
> 
> PCI generic driver as is does not do interrupts, and does not
> claim PCI resources from kernel.

OK, thanks. Don't hesitate to keep us informed about this work.
-- 
Thomas

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Antti Kantee]

On 22/05/14 13:13, Thomas Monjalon wrote:
> 2014-05-19 16:51, Anatoly Burakov:
>> Note that since igb_uio no longer has a PCI ID list, it can now be
>> bound to any device, not just those explicitly supported by DPDK. In
>> other words, it now behaves similar to PCI stub, VFIO and other generic
>> PCI drivers.
>
> I wonder if we could replace igb_uio by uio_pci_generic?

I've been running plenty of the NetBSD kernel PCI drivers in Linux 
userspace on top of uio_pci_generic, including NICs supported by DPDK. 
The only real annoyance is that mainline uio_pci_generic doesn't support 
MSI.  A pseudo-annoyance is that uio_pci_generic turns interrupts off 
from the PCI config space each time after you read an interrupt, so they 
have to be reenabled after each one (and NetBSD kernel drivers tend to 
like using interrupts for everything).

The annoyance of vfio is iommus.  Yes, I want to make the tradeoff of 
possibly scribbling memory vs. not being able to do anything on the 
wrong system.

I'd like to see a generic Linux kernel PCI driver blob without 
annoyances, though not yet annoyed enough to do anything myself ;)

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Thomas Monjalon]

2014-05-23 00:10, Antti Kantee:
> On 22/05/14 13:13, Thomas Monjalon wrote:
> > 2014-05-19 16:51, Anatoly Burakov:
> >> Note that since igb_uio no longer has a PCI ID list, it can now be
> >> bound to any device, not just those explicitly supported by DPDK. In
> >> other words, it now behaves similar to PCI stub, VFIO and other generic
> >> PCI drivers.
> > 
> > I wonder if we could replace igb_uio by uio_pci_generic?
> 
> I've been running plenty of the NetBSD kernel PCI drivers in Linux
> userspace on top of uio_pci_generic, including NICs supported by DPDK.
> The only real annoyance is that mainline uio_pci_generic doesn't support
> MSI.  A pseudo-annoyance is that uio_pci_generic turns interrupts off
> from the PCI config space each time after you read an interrupt, so they
> have to be reenabled after each one (and NetBSD kernel drivers tend to
> like using interrupts for everything).
> 
> The annoyance of vfio is iommus.  Yes, I want to make the tradeoff of
> possibly scribbling memory vs. not being able to do anything on the
> wrong system.
> 
> I'd like to see a generic Linux kernel PCI driver blob without
> annoyances, though not yet annoyed enough to do anything myself ;)

So maybe it's possible to improve uio_pci_generic in order to replace igb_uio.
If someone wants to work on it, it's possible to stage uio_pci_generic in 
dpdk.org in order to make it ready for kernel.org.

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Antti Kantee]

On 28/05/14 13:45, Thomas Monjalon wrote:
> So maybe it's possible to improve uio_pci_generic in order to replace igb_uio.
> If someone wants to work on it, it's possible to stage uio_pci_generic in
> dpdk.org in order to make it ready for kernel.org.

Back when researching MSI + uio_pci_generic, I found this:
http://www.gossamer-threads.com/lists/linux/kernel/1738200

I'm not sure I completely follow the logic of the argument there, but 
seems like the maintainer's(?) mind of uio_pci_generic never supporting 
MSI is quite made up.

Re: [dpdk-dev] [PATCH v2 13/16] Removed PCI ID table from igb_uio

[Stephen Hemminger]

On Wed, 28 May 2014 15:45:02 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2014-05-23 00:10, Antti Kantee:
> > On 22/05/14 13:13, Thomas Monjalon wrote:
> > > 2014-05-19 16:51, Anatoly Burakov:
> > >> Note that since igb_uio no longer has a PCI ID list, it can now be
> > >> bound to any device, not just those explicitly supported by DPDK. In
> > >> other words, it now behaves similar to PCI stub, VFIO and other generic
> > >> PCI drivers.
> > > 
> > > I wonder if we could replace igb_uio by uio_pci_generic?
> > 
> > I've been running plenty of the NetBSD kernel PCI drivers in Linux
> > userspace on top of uio_pci_generic, including NICs supported by DPDK.
> > The only real annoyance is that mainline uio_pci_generic doesn't support
> > MSI.  A pseudo-annoyance is that uio_pci_generic turns interrupts off
> > from the PCI config space each time after you read an interrupt, so they
> > have to be reenabled after each one (and NetBSD kernel drivers tend to
> > like using interrupts for everything).
> > 
> > The annoyance of vfio is iommus.  Yes, I want to make the tradeoff of
> > possibly scribbling memory vs. not being able to do anything on the
> > wrong system.
> > 
> > I'd like to see a generic Linux kernel PCI driver blob without
> > annoyances, though not yet annoyed enough to do anything myself ;)
> 
> So maybe it's possible to improve uio_pci_generic in order to replace igb_uio.
> If someone wants to work on it, it's possible to stage uio_pci_generic in 
> dpdk.org in order to make it ready for kernel.org.
> 

I am doing a new version of uio_pci for upstream kernel and will submit
when ready.  It will be for 3.10 or later kernel, will not bother backporting
past that.