[dpdk-dev] [PATCH 0/2] Using virtio ethdev ports as non-root

[dpdk-dev] [PATCH 0/2] Using virtio ethdev ports as non-root

[David Marchand]

Here are two little changes to be able to run testpmd as non-root with
virtio ports in a guest.
This requires a functional vIOMMU (the main pain parts being writing a
Q35 machine configuration in qemu for x86 guests).

No change since the RFC, I just did not find the time to describe the
full setup (Q35 x86 machine config + permissions on /dev/hugepages
and /dev/vfio in the guest).

Comments?

-- 
David Marchand

David Marchand (2):
  bus/pci: check IO permissions for UIO only
  net/virtio: do not require IO permissions

 drivers/bus/pci/bsd/pci.c          |  5 +++++
 drivers/bus/pci/linux/pci.c        | 10 ++++++++++
 drivers/net/virtio/virtio_ethdev.c |  8 ++------
 3 files changed, 17 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/2] bus/pci: check IO permissions for UIO only

[David Marchand]

On x86, calling inb/outb special instructions (used in uio ioport
read/write parts) is only possible if the right IO permissions has been
granted.

The only user of this API (the net/virtio pmd) checks this
unconditionnaly but this should be hidden by the rte_pci_ioport API
itself and only checked when the device is bound to a UIO driver.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/pci/bsd/pci.c   |  5 +++++
 drivers/bus/pci/linux/pci.c | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 7777179..0575adc 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -539,6 +539,11 @@ rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 	switch (dev->kdrv) {
 #if defined(RTE_ARCH_X86)
 	case RTE_KDRV_NIC_UIO:
+		if (rte_eal_iopl_init() != 0) {
+			RTE_LOG(DEBUG, EAL, "%s(): cannot gain io permissions\n",
+				__func__);
+			return -1;
+		}
 		if ((uintptr_t) dev->mem_resource[bar].addr <= UINT16_MAX) {
 			p->base = (uintptr_t)dev->mem_resource[bar].addr;
 			ret = 0;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 318db19..cd4b996 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -661,6 +661,12 @@ pci_ioport_map(struct rte_pci_device *dev, int bar __rte_unused,
 		 dev->addr.domain, dev->addr.bus,
 		 dev->addr.devid, dev->addr.function);
 
+	if (rte_eal_iopl_init() != 0) {
+		RTE_LOG(DEBUG, EAL, "%s(): cannot gain io permissions\n",
+			__func__);
+		return -1;
+	}
+
 	fp = fopen("/proc/ioports", "r");
 	if (fp == NULL) {
 		RTE_LOG(ERR, EAL, "%s(): can't open ioports\n", __func__);
@@ -718,7 +724,11 @@ rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 		break;
 #endif
 	case RTE_KDRV_IGB_UIO:
+#if defined(RTE_ARCH_X86)
+		ret = pci_ioport_map(dev, bar, p);
+#else
 		ret = pci_uio_ioport_map(dev, bar, p);
+#endif
 		break;
 	case RTE_KDRV_UIO_GENERIC:
 #if defined(RTE_ARCH_X86)
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/2] net/virtio: do not require IO permissions

[David Marchand]

On x86, iopl permissions are only needed when the virtio devices are
bound to a uio kernel module.

When running a dpdk application as non root, the virtio driver was
refusing to register because of this check while it could work when
binding the device to vfio (requires to have a vIOMMU configured).

We still need to call rte_eal_iopl_init() in the constructor so that
the interrupt thread would inherit this permission in the case it could
be used with UIO later.
Log a warning message for the user to understand what is wrong.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 7261109..3506ca0 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1995,11 +1995,6 @@ exit:
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return 1;
-	}
-
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
 	if (vdpa_mode_selected(pci_dev->device.devargs))
 		return 1;
@@ -2031,7 +2026,8 @@ static struct rte_pci_driver rte_virtio_pmd = {
 
 RTE_INIT(rte_virtio_pmd_init)
 {
-	rte_eal_iopl_init();
+	if (rte_eal_iopl_init() != 0)
+		PMD_INIT_LOG(WARNING, "IOPL call failed - virtio devices won't be functional if bound to UIO drivers");
 	rte_pci_register(&rte_virtio_pmd);
 }
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 2/2] net/virtio: do not require IO permissions

[Tiwei Bie]

On Thu, Oct 17, 2019 at 01:56:28PM +0200, David Marchand wrote:
> On x86, iopl permissions are only needed when the virtio devices are
> bound to a uio kernel module.
> 
> When running a dpdk application as non root, the virtio driver was
> refusing to register because of this check while it could work when
> binding the device to vfio (requires to have a vIOMMU configured).
> 
> We still need to call rte_eal_iopl_init() in the constructor so that
> the interrupt thread would inherit this permission in the case it could
> be used with UIO later.
> Log a warning message for the user to understand what is wrong.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 7261109..3506ca0 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1995,11 +1995,6 @@ exit:
>  static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>  	struct rte_pci_device *pci_dev)
>  {
> -	if (rte_eal_iopl_init() != 0) {
> -		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> -		return 1;
> -	}
> -
>  	/* virtio pmd skips probe if device needs to work in vdpa mode */
>  	if (vdpa_mode_selected(pci_dev->device.devargs))
>  		return 1;
> @@ -2031,7 +2026,8 @@ static struct rte_pci_driver rte_virtio_pmd = {
>  
>  RTE_INIT(rte_virtio_pmd_init)
>  {
> -	rte_eal_iopl_init();
> +	if (rte_eal_iopl_init() != 0)
> +		PMD_INIT_LOG(WARNING, "IOPL call failed - virtio devices won't be functional if bound to UIO drivers");

Basically this will undo what below commit did, i.e.
annoying log will show again.

https://github.com/DPDK/dpdk/commit/705dced4a72a1053368c84c4b68f04f028a78b30

Maybe it's better to print this error when we really
need the port IO (legacy device):

https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_pci.c#L679-L680
https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_ethdev.c#L1834-L1835

Thanks,
Tiwei

>  	rte_pci_register(&rte_virtio_pmd);
>  }
>  
> -- 
> 1.8.3.1
> 

Re: [dpdk-dev] [PATCH 2/2] net/virtio: do not require IO permissions

[David Marchand]

On Fri, Oct 18, 2019 at 10:19 AM Tiwei Bie <tiwei.bie@intel.com> wrote:
>
> On Thu, Oct 17, 2019 at 01:56:28PM +0200, David Marchand wrote:
> > On x86, iopl permissions are only needed when the virtio devices are
> > bound to a uio kernel module.
> >
> > When running a dpdk application as non root, the virtio driver was
> > refusing to register because of this check while it could work when
> > binding the device to vfio (requires to have a vIOMMU configured).
> >
> > We still need to call rte_eal_iopl_init() in the constructor so that
> > the interrupt thread would inherit this permission in the case it could
> > be used with UIO later.
> > Log a warning message for the user to understand what is wrong.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  drivers/net/virtio/virtio_ethdev.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > index 7261109..3506ca0 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -1995,11 +1995,6 @@ exit:
> >  static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> >       struct rte_pci_device *pci_dev)
> >  {
> > -     if (rte_eal_iopl_init() != 0) {
> > -             PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> > -             return 1;
> > -     }
> > -
> >       /* virtio pmd skips probe if device needs to work in vdpa mode */
> >       if (vdpa_mode_selected(pci_dev->device.devargs))
> >               return 1;
> > @@ -2031,7 +2026,8 @@ static struct rte_pci_driver rte_virtio_pmd = {
> >
> >  RTE_INIT(rte_virtio_pmd_init)
> >  {
> > -     rte_eal_iopl_init();
> > +     if (rte_eal_iopl_init() != 0)
> > +             PMD_INIT_LOG(WARNING, "IOPL call failed - virtio devices won't be functional if bound to UIO drivers");
>
> Basically this will undo what below commit did, i.e.
> annoying log will show again.
>
> https://github.com/DPDK/dpdk/commit/705dced4a72a1053368c84c4b68f04f028a78b30

Yes.. true.
I wanted to keep a trace for debug, so maybe lower this to debug
level, or just drop this message.


>
> Maybe it's better to print this error when we really
> need the port IO (legacy device):

virtio calls rte_pci_ioport_map, so I suppose we can change the log
level to ERR in the first patch of this series:
http://patchwork.dpdk.org/patch/61370/

And I suppose I could add some context in this log, like the device name.

>
> https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_pci.c#L679-L680
> https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_ethdev.c#L1834-L1835


-- 
David Marchand

Re: [dpdk-dev] [PATCH 2/2] net/virtio: do not require IO permissions

[Tiwei Bie]

On Fri, Oct 18, 2019 at 10:33:47AM +0200, David Marchand wrote:
> On Fri, Oct 18, 2019 at 10:19 AM Tiwei Bie <tiwei.bie@intel.com> wrote:
> > On Thu, Oct 17, 2019 at 01:56:28PM +0200, David Marchand wrote:
> > > On x86, iopl permissions are only needed when the virtio devices are
> > > bound to a uio kernel module.
> > >
> > > When running a dpdk application as non root, the virtio driver was
> > > refusing to register because of this check while it could work when
> > > binding the device to vfio (requires to have a vIOMMU configured).
> > >
> > > We still need to call rte_eal_iopl_init() in the constructor so that
> > > the interrupt thread would inherit this permission in the case it could
> > > be used with UIO later.
> > > Log a warning message for the user to understand what is wrong.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  drivers/net/virtio/virtio_ethdev.c | 8 ++------
> > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > > index 7261109..3506ca0 100644
> > > --- a/drivers/net/virtio/virtio_ethdev.c
> > > +++ b/drivers/net/virtio/virtio_ethdev.c
> > > @@ -1995,11 +1995,6 @@ exit:
> > >  static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> > >       struct rte_pci_device *pci_dev)
> > >  {
> > > -     if (rte_eal_iopl_init() != 0) {
> > > -             PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> > > -             return 1;
> > > -     }
> > > -
> > >       /* virtio pmd skips probe if device needs to work in vdpa mode */
> > >       if (vdpa_mode_selected(pci_dev->device.devargs))
> > >               return 1;
> > > @@ -2031,7 +2026,8 @@ static struct rte_pci_driver rte_virtio_pmd = {
> > >
> > >  RTE_INIT(rte_virtio_pmd_init)
> > >  {
> > > -     rte_eal_iopl_init();
> > > +     if (rte_eal_iopl_init() != 0)
> > > +             PMD_INIT_LOG(WARNING, "IOPL call failed - virtio devices won't be functional if bound to UIO drivers");
> >
> > Basically this will undo what below commit did, i.e.
> > annoying log will show again.
> >
> > https://github.com/DPDK/dpdk/commit/705dced4a72a1053368c84c4b68f04f028a78b30
> 
> Yes.. true.
> I wanted to keep a trace for debug, so maybe lower this to debug
> level, or just drop this message.
> 
> 
> >
> > Maybe it's better to print this error when we really
> > need the port IO (legacy device):
> 
> virtio calls rte_pci_ioport_map, so I suppose we can change the log
> level to ERR in the first patch of this series:
> http://patchwork.dpdk.org/patch/61370/
> 
> And I suppose I could add some context in this log, like the device name.

Sounds like a good idea to me.

Thanks,
Tiwei


> 
> >
> > https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_pci.c#L679-L680
> > https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_ethdev.c#L1834-L1835
> 
> 
> -- 
> David Marchand

[dpdk-dev] [PATCH v2 0/2] Using virtio ethdev ports as non-root

[David Marchand]

Here are two little changes to be able to run testpmd as non-root with
virtio ports in a guest.
This requires a functional vIOMMU (the main pain parts being writing a
Q35 machine configuration in qemu for x86 guests).

No major change since the RFC, I just did not find the time to describe
the full setup (Q35 x86 machine config + permissions on /dev/hugepages
and /dev/vfio in the guest + ulimit -l unlimited).

-- 
David Marchand

David Marchand (2):
  bus/pci: check IO permissions for UIO only
  net/virtio: do not require IO permissions

 drivers/bus/pci/bsd/pci.c          |  5 +++++
 drivers/bus/pci/linux/pci.c        | 10 ++++++++++
 drivers/net/virtio/virtio_ethdev.c |  5 -----
 3 files changed, 15 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/2] bus/pci: check IO permissions for UIO only

[David Marchand]

On x86, calling inb/outb special instructions (used in UIO ioport
read/write parts) is only possible if the right IO permissions has been
granted.

The only user of this API (the net/virtio pmd) checks this
unconditionnaly but this should be hidden by the rte_pci_ioport API
itself and only checked when the device is bound to a UIO driver.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- change log message level from DEBUG to ERR,
- add device name in log message,

---
 drivers/bus/pci/bsd/pci.c   |  5 +++++
 drivers/bus/pci/linux/pci.c | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 7777179..ebbfeb1 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -539,6 +539,11 @@ rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 	switch (dev->kdrv) {
 #if defined(RTE_ARCH_X86)
 	case RTE_KDRV_NIC_UIO:
+		if (rte_eal_iopl_init() != 0) {
+			RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+				__func__, dev->name);
+			return -1;
+		}
 		if ((uintptr_t) dev->mem_resource[bar].addr <= UINT16_MAX) {
 			p->base = (uintptr_t)dev->mem_resource[bar].addr;
 			ret = 0;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 318db19..7b46fe1 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -661,6 +661,12 @@ pci_ioport_map(struct rte_pci_device *dev, int bar __rte_unused,
 		 dev->addr.domain, dev->addr.bus,
 		 dev->addr.devid, dev->addr.function);
 
+	if (rte_eal_iopl_init() != 0) {
+		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+			__func__, dev->name);
+		return -1;
+	}
+
 	fp = fopen("/proc/ioports", "r");
 	if (fp == NULL) {
 		RTE_LOG(ERR, EAL, "%s(): can't open ioports\n", __func__);
@@ -718,7 +724,11 @@ rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 		break;
 #endif
 	case RTE_KDRV_IGB_UIO:
+#if defined(RTE_ARCH_X86)
+		ret = pci_ioport_map(dev, bar, p);
+#else
 		ret = pci_uio_ioport_map(dev, bar, p);
+#endif
 		break;
 	case RTE_KDRV_UIO_GENERIC:
 #if defined(RTE_ARCH_X86)
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/2] net/virtio: do not require IO permissions

[David Marchand]

On x86, iopl permissions are only available to root user (or users that
have the CAP_SYS_RAWIO capability).
But those permissions are only needed when the virtio devices accesses
are done with inb/outb instructions, which is when the device is bound
to a UIO kernel module.

So far, the virtio driver was refusing to register based on the check
on IO permissions.
This check does not make sense when binding the device to vfio.

Now that the check on IO permissions has been abstracted in the ioport
API, we can remove it on virtio side.

We still need to call rte_eal_iopl_init() in the virtio constructor so
that the interrupt thread inherits this permission in the case it could
be used with UIO later.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- remove log message in constructor (thanks Tiwei),
- reword commit log,

---
 drivers/net/virtio/virtio_ethdev.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 7261109..0a2ed2e 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1995,11 +1995,6 @@ exit:
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return 1;
-	}
-
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
 	if (vdpa_mode_selected(pci_dev->device.devargs))
 		return 1;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 0/2] Using virtio ethdev ports as non-root

[Maxime Coquelin]

On 10/20/19 2:29 PM, David Marchand wrote:
> Here are two little changes to be able to run testpmd as non-root with
> virtio ports in a guest.
> This requires a functional vIOMMU (the main pain parts being writing a
> Q35 machine configuration in qemu for x86 guests).
> 
> No major change since the RFC, I just did not find the time to describe
> the full setup (Q35 x86 machine config + permissions on /dev/hugepages
> and /dev/vfio in the guest + ulimit -l unlimited).
> 

For the series:

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[dpdk-dev] [PATCH v3 0/2] Using virtio ethdev ports as non-root

[David Marchand]

Here are two little changes to be able to run testpmd as non-root with
virtio ports in a guest.
This requires a functional vIOMMU (the main pain parts being writing a
Q35 machine configuration in qemu for x86 guests).

No major change since the RFC, I just did not find the time to describe
the full setup (Q35 x86 machine config + permissions on /dev/hugepages
and /dev/vfio in the guest + ulimit -l unlimited).

-- 
David Marchand

David Marchand (2):
  bus/pci: check IO permissions for UIO only
  net/virtio: do not require IO permissions

 drivers/bus/pci/bsd/pci.c          | 5 +++++
 drivers/bus/pci/linux/pci.c        | 6 ++++++
 drivers/bus/pci/linux/pci_uio.c    | 6 ++++++
 drivers/net/virtio/virtio_ethdev.c | 5 -----
 4 files changed, 17 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v3 1/2] bus/pci: check IO permissions for UIO only

[David Marchand]

On x86, calling inb/outb special instructions (used in UIO ioport
read/write parts) is only possible if the right IO permissions has been
granted.

The only user of this API (the net/virtio pmd) checks this
unconditionnaly but this should be hidden by the rte_pci_ioport API
itself and only checked when the device is bound to a UIO driver.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v2:
- do not switch to pci_ioport_map in igb_uio case, add a check on iopl
  there too,

Changelog since v1:
- change log message level from DEBUG to ERR,
- add device name in log message,

---
 drivers/bus/pci/bsd/pci.c       | 5 +++++
 drivers/bus/pci/linux/pci.c     | 6 ++++++
 drivers/bus/pci/linux/pci_uio.c | 6 ++++++
 3 files changed, 17 insertions(+)

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 7777179..ebbfeb1 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -539,6 +539,11 @@ rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 	switch (dev->kdrv) {
 #if defined(RTE_ARCH_X86)
 	case RTE_KDRV_NIC_UIO:
+		if (rte_eal_iopl_init() != 0) {
+			RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+				__func__, dev->name);
+			return -1;
+		}
 		if ((uintptr_t) dev->mem_resource[bar].addr <= UINT16_MAX) {
 			p->base = (uintptr_t)dev->mem_resource[bar].addr;
 			ret = 0;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 318db19..740a2cd 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -657,6 +657,12 @@ pci_ioport_map(struct rte_pci_device *dev, int bar __rte_unused,
 	int found = 0;
 	size_t linesz;
 
+	if (rte_eal_iopl_init() != 0) {
+		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+			__func__, dev->name);
+		return -1;
+	}
+
 	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
 		 dev->addr.domain, dev->addr.bus,
 		 dev->addr.devid, dev->addr.function);
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index e031361..6dca05a 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -375,6 +375,12 @@ pci_uio_ioport_map(struct rte_pci_device *dev, int bar,
 	int uio_num;
 	unsigned long start;
 
+	if (rte_eal_iopl_init() != 0) {
+		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+			__func__, dev->name);
+		return -1;
+	}
+
 	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname), 0);
 	if (uio_num < 0)
 		return -1;
-- 
1.8.3.1

[dpdk-dev] [PATCH v3 2/2] net/virtio: do not require IO permissions

[David Marchand]

On x86, iopl permissions are only available to root user (or users that
have the CAP_SYS_RAWIO capability).
But those permissions are only needed when the virtio devices accesses
are done with inb/outb instructions, which is when the device is bound
to a UIO kernel module.

So far, the virtio driver was refusing to register based on the check
on IO permissions.
This check does not make sense when binding the device to vfio.

Now that the check on IO permissions has been abstracted in the ioport
API, we can remove it on virtio side.

We still need to call rte_eal_iopl_init() in the virtio constructor so
that the interrupt thread inherits this permission in the case it could
be used with UIO later.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
Changelog since v1:
- remove log message in constructor (thanks Tiwei),
- reword commit log,

---
 drivers/net/virtio/virtio_ethdev.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 7261109..0a2ed2e 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1995,11 +1995,6 @@ exit:
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return 1;
-	}
-
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
 	if (vdpa_mode_selected(pci_dev->device.devargs))
 		return 1;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: do not require IO permissions

[Tiwei Bie]

On Tue, Oct 22, 2019 at 10:21:43AM +0200, David Marchand wrote:
> On x86, iopl permissions are only available to root user (or users that
> have the CAP_SYS_RAWIO capability).
> But those permissions are only needed when the virtio devices accesses
> are done with inb/outb instructions, which is when the device is bound
> to a UIO kernel module.
> 
> So far, the virtio driver was refusing to register based on the check
> on IO permissions.
> This check does not make sense when binding the device to vfio.
> 
> Now that the check on IO permissions has been abstracted in the ioport
> API, we can remove it on virtio side.
> 
> We still need to call rte_eal_iopl_init() in the virtio constructor so
> that the interrupt thread inherits this permission in the case it could
> be used with UIO later.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---

Acked-by: Tiwei Bie <tiwei.bie@intel.com>

Thanks,
Tiwei

Re: [dpdk-dev] [PATCH v3 1/2] bus/pci: check IO permissions for UIO only

[Maxime Coquelin]

On 10/22/19 10:21 AM, David Marchand wrote:
> On x86, calling inb/outb special instructions (used in UIO ioport
> read/write parts) is only possible if the right IO permissions has been
> granted.
> 
> The only user of this API (the net/virtio pmd) checks this
> unconditionnaly but this should be hidden by the rte_pci_ioport API
> itself and only checked when the device is bound to a UIO driver.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changelog since v2:
> - do not switch to pci_ioport_map in igb_uio case, add a check on iopl
>   there too,
> 
> Changelog since v1:
> - change log message level from DEBUG to ERR,
> - add device name in log message,
> 
> ---
>  drivers/bus/pci/bsd/pci.c       | 5 +++++
>  drivers/bus/pci/linux/pci.c     | 6 ++++++
>  drivers/bus/pci/linux/pci_uio.c | 6 ++++++
>  3 files changed, 17 insertions(+)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v3 0/2] Using virtio ethdev ports as non-root

[David Marchand]

On Tue, Oct 22, 2019 at 10:22 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> Here are two little changes to be able to run testpmd as non-root with
> virtio ports in a guest.
> This requires a functional vIOMMU (the main pain parts being writing a
> Q35 machine configuration in qemu for x86 guests).
>
> No major change since the RFC, I just did not find the time to describe
> the full setup (Q35 x86 machine config + permissions on /dev/hugepages
> and /dev/vfio in the guest + ulimit -l unlimited).
>
> --
> David Marchand
>
> David Marchand (2):
>   bus/pci: check IO permissions for UIO only
>   net/virtio: do not require IO permissions

Applied, thanks for the reviews Maxime, Tiwei.



--
David Marchand