[dpdk-dev] [PATCH] bus/pci: support iova=va on PowerNV systems

[dpdk-dev] [PATCH] bus/pci: support iova=va on PowerNV systems

[David Christensen]

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(+)

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

Re: [dpdk-dev] [PATCH] bus/pci: support iova=va on PowerNV systems

[David Marchand]

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 Marchand

Re: [dpdk-dev] [PATCH] bus/pci: support iova=va on PowerNV systems

[David Christensen]

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

[dpdk-dev] [PATCH v2] bus/pci: support iova=va on PowerNV systems

[David Christensen]

All recent POWER systems, Power 8 and 9 specifically, support an IOMMU
(it can't be disabled). The functionality of the IOMMU is different
depending on whether it's running on a bare metal PowerNV system or in
a virtual environment (PowerVM LPAR or KVM/QEMU).  DPDK currently
supports the IOMMU found on PowerNV platforms, sPAPRv2, so IOVA=VA
mode can be enabled when the correct platform is detected.

The POWER IOMMU type can't be detected through mechansims such as
parsing files in the /sys heirarchy like x86_64 systems so the
/proc/cpuinfo file is parsed to determine whether Linux is running
on bare metal (i.e. PowerNV) or in a virtual environment (KVM/QEMU).

Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
---
v2:
* Fixed memory leak from use of getline
* Fixed nits for NULL memory address and indent levels
* Modified commit message description on logic for detecting IOMMU
  on a POWER system
---
 drivers/bus/pci/linux/pci.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 740a2cdad..a1e30b721 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -549,7 +549,40 @@ 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)
 {
-	return false;
+	/*
+	 * 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 = NULL;
+	size_t len = 0;
+	char filename[PATH_MAX] = "/proc/cpuinfo";
+	FILE *fp = fopen(filename, "r");
+	bool ret = false;
+
+	if (fp == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
+			__func__, filename, strerror(errno));
+		return ret;
+	}
+
+	/* Check for a PowerNV platform */
+	while (getline(&line, &len, fp) != -1) {
+		if (strstr(line, "platform") != NULL)
+			continue;
+
+		if (strstr(line, "PowerNV") != NULL) {
+			RTE_LOG(DEBUG, EAL, "Running on a PowerNV system\n");
+			ret = true;
+			break;
+		}
+	}
+
+	free(line);
+	fclose(fp);
+	return ret;
 }
 #else
 bool
-- 
2.18.1

Re: [dpdk-dev] [PATCH v2] bus/pci: support iova=va on PowerNV systems

[David Marchand]

On Mon, Mar 16, 2020 at 9:38 PM David Christensen
<drc@linux.vnet.ibm.com> wrote:
>
> All recent POWER systems, Power 8 and 9 specifically, support an IOMMU
> (it can't be disabled). The functionality of the IOMMU is different
> depending on whether it's running on a bare metal PowerNV system or in
> a virtual environment (PowerVM LPAR or KVM/QEMU).  DPDK currently
> supports the IOMMU found on PowerNV platforms, sPAPRv2, so IOVA=VA
> mode can be enabled when the correct platform is detected.
>
> The POWER IOMMU type can't be detected through mechansims such as

mechanisms

> parsing files in the /sys heirarchy like x86_64 systems so the

hierarchy


> /proc/cpuinfo file is parsed to determine whether Linux is running
> on bare metal (i.e. PowerNV) or in a virtual environment (KVM/QEMU).
>
> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>

Applied, thanks.


-- 
David Marchand