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

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.

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

[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

[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

[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-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

[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-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

[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