[v3,1/2] bus/pci: check IO permissions for UIO only

Message ID 1571732503-30424-2-git-send-email-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series Using virtio ethdev ports as non-root |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

David Marchand Oct. 22, 2019, 8:21 a.m. UTC
  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(+)
  

Comments

Maxime Coquelin Oct. 24, 2019, 9:55 a.m. UTC | #1
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
  

Patch

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;