bus/pci: support iova=va on PowerNV systems
diff mbox series

Message ID 20200226195033.33877-1-drc@linux.vnet.ibm.com
State Superseded
Delegated to: David Marchand
Headers show
Series
  • bus/pci: support iova=va on PowerNV systems
Related show

Checks

Context Check Description
ci/travis-robot success Travis build: passed
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

David Christensen Feb. 26, 2020, 7:50 p.m. UTC
Bare metal PowerNV systems include a DPDK supported IOMMU that allows
IOVA=VA support. Test for the platform type and report virtual address
support if running on a PowerNV system.

Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
---
 drivers/bus/pci/linux/pci.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

David Marchand March 13, 2020, 9:09 a.m. UTC | #1
On Wed, Feb 26, 2020 at 8:50 PM David Christensen
<drc@linux.vnet.ibm.com> wrote:
>
> Bare metal PowerNV systems include a DPDK supported IOMMU that allows
> IOVA=VA support. Test for the platform type and report virtual address
> support if running on a PowerNV system.

I can't test it, so I'll trust you on this :-).
Comments below.


>
> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> ---
>  drivers/bus/pci/linux/pci.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 740a2cdad..696dc177e 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -549,6 +549,35 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
>  bool
>  pci_device_iommu_support_va(__rte_unused const struct rte_pci_device *dev)
>  {
> +       /*
> +        * IOMMU is always present on a PowerNV host (IOMMUv2).
> +        * IOMMU is also present in a KVM/QEMU VM (IOMMUv1) but is not
> +        * currently supported by DPDK. Test for our current environment
> +        * and report VA support as appropriate.
> +        */
> +
> +       char *line = 0;

Nit: NULL

> +       size_t len = 0;
> +       char filename[PATH_MAX] = "/proc/cpuinfo";
> +       FILE *fp = fopen(filename, "r");
> +
> +       if (fp == NULL) {
> +               RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
> +                       __func__, filename, strerror(errno));
> +               return false;
> +       }
> +
> +       /* Check for a PowerNV platform */
> +       while (getline(&line, &len, fp) != -1) {

From here, you leak line.

man getline

       ssize_t getline(char **lineptr, size_t *n, FILE *stream);

       If  *lineptr is NULL, then getline() will allocate a buffer for storing
       the line, which should be freed by the user program.   (In  this  case,
       the value in *n is ignored.)


> +               if (strstr(line, "platform") != NULL)

Nit, to avoid multiple level of indent, how about invert the check:

if (strstr(line, "platform") == NULL)
    continue;

> +                       if (strstr(line, "PowerNV") != NULL) {
> +                               RTE_LOG(DEBUG, EAL, "Running on a PowerNV system\n");

Not really helpful, it does not indicate that this system iommu supports VA.


> +                               fclose(fp);
> +                               return true;
> +                       }
> +       }
> +       fclose(fp);
> +
>         return false;
>  }
>  #else
> --
> 2.18.1
>
David Christensen March 16, 2020, 4:09 p.m. UTC | #2
>> Bare metal PowerNV systems include a DPDK supported IOMMU that allows
>> IOVA=VA support. Test for the platform type and report virtual address
>> support if running on a PowerNV system.
...
>>
>> +
>> +       char *line = 0;
> 
> Nit: NULL

Fixed.

>> +       /* Check for a PowerNV platform */
>> +       while (getline(&line, &len, fp) != -1) {
> 
>  From here, you leak line.
> 
Fixed.

> 
> Nit, to avoid multiple level of indent, how about invert the check:
> 
> if (strstr(line, "platform") == NULL)
>      continue;

Fixed.

>> +                       if (strstr(line, "PowerNV") != NULL) {
>> +                               RTE_LOG(DEBUG, EAL, "Running on a PowerNV system\n");
> 
> Not really helpful, it does not indicate that this system iommu supports VA.

This is the advice given by the kernel developer who maintains the IOMMU 
code for PPC.  There's nothing in the /sys filesystem that describes the 
IOMMU like there is in x86_64 platforms.  All POWER systems supported by 
DPDK have an IOMMU (you can't turn it off). This is true for both bare 
metal systems like PowerNV (Power Non-Virtualized) as well as 
virtualized systems like QEMU/KVM and PowerVM LPARs.  In the case of 
virtualized systems the IOMMU has different capabilities and is not 
currently supported in DPDK.  Thus, if I know the platform I'm running 
on then I know the IOMMU is present and enabled.

Dave

Patch
diff mbox series

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 740a2cdad..696dc177e 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -549,6 +549,35 @@  pci_device_iommu_support_va(const struct rte_pci_device *dev)
 bool
 pci_device_iommu_support_va(__rte_unused const struct rte_pci_device *dev)
 {
+	/*
+	 * IOMMU is always present on a PowerNV host (IOMMUv2).
+	 * IOMMU is also present in a KVM/QEMU VM (IOMMUv1) but is not
+	 * currently supported by DPDK. Test for our current environment
+	 * and report VA support as appropriate.
+	 */
+
+	char *line = 0;
+	size_t len = 0;
+	char filename[PATH_MAX] = "/proc/cpuinfo";
+	FILE *fp = fopen(filename, "r");
+
+	if (fp == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
+			__func__, filename, strerror(errno));
+		return false;
+	}
+
+	/* Check for a PowerNV platform */
+	while (getline(&line, &len, fp) != -1) {
+		if (strstr(line, "platform") != NULL)
+			if (strstr(line, "PowerNV") != NULL) {
+				RTE_LOG(DEBUG, EAL, "Running on a PowerNV system\n");
+				fclose(fp);
+				return true;
+			}
+	}
+	fclose(fp);
+
 	return false;
 }
 #else