[PATCH 0/1] pci: read amd iommu virtual address width

[PATCH 0/1] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Michael Piszczek (1):
  pci: read amd iommu virtual address width

 drivers/bus/pci/linux/pci.c | 42 +++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

-- 
2.34.1

[PATCH 1/1] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 42 +++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index e521459870..df9da78e45 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -492,6 +492,38 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
+static uint64_t
+pci_device_amd_iommu_support_va(const struct rte_pci_addr *addr)
+{
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
+
+	char filename[PATH_MAX];
+	FILE *fp;
+	uint64_t amd_cap_reg = 0;
+
+	snprintf(filename, sizeof(filename),
+		"%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
+		rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
+		addr->function);
+
+	fp = fopen(filename, "r");
+	if (fp == NULL)
+		return 0;
+
+	/* We have an Amd IOMMU */
+	if (fscanf(fp, "%" PRIx64, &amd_cap_reg) != 1) {
+		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+		fclose(fp);
+		return 0;
+	}
+
+	fclose(fp);
+
+	return ((amd_cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+}
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
@@ -501,6 +533,16 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
 	char filename[PATH_MAX];
 	FILE *fp;
 	uint64_t mgaw, vtd_cap_reg = 0;
+	struct stat s;
+
+	if (stat("/sys/class/iommu/ivhd2/amd-iommu", &s) == 0) {
+		mgaw = pci_device_amd_iommu_support_va(addr);
+		if (mgaw > 0) {
+			rte_mem_set_dma_mask(mgaw);
+			return true;
+		}
+		return false;
+	}
 
 	snprintf(filename, sizeof(filename),
 		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
-- 
2.34.1

[PATCH v2] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index e521459870..0c6d79ca74 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -4,6 +4,7 @@
 
 #include <string.h>
 #include <dirent.h>
+#include <sys/stat.h>
 
 #include <rte_log.h>
 #include <rte_bus.h>
@@ -492,6 +493,38 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
+static uint64_t
+pci_device_amd_iommu_support_va(const struct rte_pci_addr *addr)
+{
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
+
+	char filename[PATH_MAX];
+	FILE *fp;
+	uint64_t amd_cap_reg = 0;
+
+	snprintf(filename, sizeof(filename),
+		"%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
+		rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
+		addr->function);
+
+	fp = fopen(filename, "r");
+	if (fp == NULL)
+		return 0;
+
+	/* We have an Amd IOMMU */
+	if (fscanf(fp, "%" PRIx64, &amd_cap_reg) != 1) {
+		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+		fclose(fp);
+		return 0;
+	}
+
+	fclose(fp);
+
+	return ((amd_cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+}
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
@@ -501,6 +534,16 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
 	char filename[PATH_MAX];
 	FILE *fp;
 	uint64_t mgaw, vtd_cap_reg = 0;
+	struct stat s;
+
+	if (stat("/sys/class/iommu/ivhd2/amd-iommu", &s) == 0) {
+		mgaw = pci_device_amd_iommu_support_va(addr);
+		if (mgaw > 0) {
+			rte_mem_set_dma_mask(mgaw);
+			return true;
+		}
+		return false;
+	}
 
 	snprintf(filename, sizeof(filename),
 		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
-- 
2.34.1

Re: [PATCH v2] pci: read amd iommu virtual address width

[David Marchand]

On Thu, Sep 15, 2022 at 8:40 AM Michael Piszczek <mpiszczek@ddn.com> wrote:
>
> Add code to read the virtual address width for AMD processors.
>
> Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
> ---
>  drivers/bus/pci/linux/pci.c | 43 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index e521459870..0c6d79ca74 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -4,6 +4,7 @@
>
>  #include <string.h>
>  #include <dirent.h>
> +#include <sys/stat.h>
>
>  #include <rte_log.h>
>  #include <rte_bus.h>
> @@ -492,6 +493,38 @@ rte_pci_scan(void)
>  }
>
>  #if defined(RTE_ARCH_X86)
> +
> +static uint64_t
> +pci_device_amd_iommu_support_va(const struct rte_pci_addr *addr)
> +{
> +#define RD_AMD_CAP_VASIZE_SHIFT 15
> +#define RD_AMD_CAP_VASIZE_MASK (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
> +
> +       char filename[PATH_MAX];
> +       FILE *fp;
> +       uint64_t amd_cap_reg = 0;
> +
> +       snprintf(filename, sizeof(filename),
> +               "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
> +               rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> +               addr->function);
> +
> +       fp = fopen(filename, "r");
> +       if (fp == NULL)
> +               return 0;
> +
> +       /* We have an Amd IOMMU */
> +       if (fscanf(fp, "%" PRIx64, &amd_cap_reg) != 1) {
> +               RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
> +               fclose(fp);
> +               return 0;
> +       }
> +
> +       fclose(fp);
> +
> +       return ((amd_cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
> +}
> +
>  bool
>  pci_device_iommu_support_va(const struct rte_pci_device *dev)
>  {
> @@ -501,6 +534,16 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
>         char filename[PATH_MAX];
>         FILE *fp;
>         uint64_t mgaw, vtd_cap_reg = 0;
> +       struct stat s;
> +
> +       if (stat("/sys/class/iommu/ivhd2/amd-iommu", &s) == 0) {
> +               mgaw = pci_device_amd_iommu_support_va(addr);
> +               if (mgaw > 0) {
> +                       rte_mem_set_dma_mask(mgaw);
> +                       return true;
> +               }
> +               return false;
> +       }
>
>         snprintf(filename, sizeof(filename),
>                  "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
> --
> 2.34.1
>

Sorry, I picked you guys with @amd.com mail addresses.
If there is someone with knowledge of this piece of AMD hw available,
can this patch be reviewed?
Thanks.


-- 
David Marchand

RE: [PATCH v2] pci: read amd iommu virtual address width

[Varghese, Vipin]

[Public]

Hi Michael,

Thank you for looking into the change, please find my comments shared below

<snipped>

> >
> > Add code to read the virtual address width for AMD processors.
> >
> > Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
> > ---
> >  drivers/bus/pci/linux/pci.c | 43
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > index e521459870..0c6d79ca74 100644
> > --- a/drivers/bus/pci/linux/pci.c
> > +++ b/drivers/bus/pci/linux/pci.c
> > @@ -4,6 +4,7 @@
> >
> >  #include <string.h>
> >  #include <dirent.h>
> > +#include <sys/stat.h>
> >
> >  #include <rte_log.h>
> >  #include <rte_bus.h>
> > @@ -492,6 +493,38 @@ rte_pci_scan(void)  }
> >
> >  #if defined(RTE_ARCH_X86)
> > +
> > +static uint64_t
> > +pci_device_amd_iommu_support_va(const struct rte_pci_addr *addr) {
> > +#define RD_AMD_CAP_VASIZE_SHIFT 15 #define
> RD_AMD_CAP_VASIZE_MASK
> > +(0x7F << RD_AMD_CAP_VASIZE_SHIFT)
> > +
> > +       char filename[PATH_MAX];
> > +       FILE *fp;
> > +       uint64_t amd_cap_reg = 0;
> > +
> > +       snprintf(filename, sizeof(filename),
> > +               "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
> > +               rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> > +               addr->function);
> > +
> > +       fp = fopen(filename, "r");
> > +       if (fp == NULL)
> > +               return 0;
> > +
> > +       /* We have an Amd IOMMU */
> > +       if (fscanf(fp, "%" PRIx64, &amd_cap_reg) != 1) {
> > +               RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
> > +               fclose(fp);
> > +               return 0;
> > +       }
> > +
> > +       fclose(fp);
> > +
> > +       return ((amd_cap_reg & RD_AMD_CAP_VASIZE_MASK) >>
> > +RD_AMD_CAP_VASIZE_SHIFT) + 1; }
> > +
> >  bool
> >  pci_device_iommu_support_va(const struct rte_pci_device *dev)  { @@
> > -501,6 +534,16 @@ pci_device_iommu_support_va(const struct
> rte_pci_device *dev)
> >         char filename[PATH_MAX];
> >         FILE *fp;
> >         uint64_t mgaw, vtd_cap_reg = 0;
> > +       struct stat s;
> > +
> > +       if (stat("/sys/class/iommu/ivhd2/amd-iommu", &s) == 0) {

AMD EPYC can be configured into various Logical NUMA divisions for both 1P and 2P (sockets) for Naples, Rome and Milan.  This leads various combinations as 1, 2, 4 for a single socket. Hence hard coding the check for ` ivhd2` could fail even for a valid NPS configuration.

> > +               mgaw = pci_device_amd_iommu_support_va(addr);

Instead of checking the device availability, one can exercise the iommu capability directly. For example

```
# ls /sys/bus/pci/devices/0000\:41\:00.0/iommu/amd-iommu/cap -l
-r--r--r-- 1 root root 4096 Oct 10 06:10 /sys/bus/pci/devices/0000:41:00.0/iommu/amd-iommu/cap
```

> > +               if (mgaw > 0) {
> > +                       rte_mem_set_dma_mask(mgaw);
> > +                       return true;
> > +               }
> > +               return false;
> > +       }
> >
> >         snprintf(filename, sizeof(filename),
> >                  "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",

Hence my suggestion would be retaining the same code flow as original function with added check for `amd-iommu` along with `intel-iommu` for x86 devices. The additional function call ` pci_device_amd_iommu_support_va` can be avoided.

> > --
> > 2.34.1
> >
> 
> Sorry, I picked you guys with @amd.com mail addresses.
> If there is someone with knowledge of this piece of AMD hw available, can
> this patch be reviewed?
> Thanks.
> 
> 
> --
> David Marchand

[PATCH v3] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 47 +++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index e521459870..8c6082ee7a 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -492,15 +492,18 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
 #define VTD_CAP_MGAW_SHIFT	16
 #define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
 	const struct rte_pci_addr *addr = &dev->addr;
 	char filename[PATH_MAX];
 	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	uint64_t mgaw, cap_reg = 0;
 
 	snprintf(filename, sizeof(filename),
 		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
@@ -508,26 +511,36 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
 		 addr->function);
 
 	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
-
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+	if (fp != NULL) {
+		/* We have an Intel IOMMU */
+		if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+			   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			fclose(fp);
+			return false;
+		}
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
 		fclose(fp);
-		return false;
+		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
 	}
+	else {
+		snprintf(filename, sizeof(filename),
+			 "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
+			  rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
+			  addr->function);
+
+		fp = fopen(filename, "r");
+		if (fp != NULL) {
+			/* We have an Amd IOMMU */
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+				fclose(fp);
+				return false;
+			}
 
-	fclose(fp);
-
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+			fclose(fp);
+			mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+		}
+	}
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

[PATCH v3] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 47 +++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index e521459870..8c6082ee7a 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -492,15 +492,18 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
 #define VTD_CAP_MGAW_SHIFT	16
 #define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
 	const struct rte_pci_addr *addr = &dev->addr;
 	char filename[PATH_MAX];
 	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	uint64_t mgaw, cap_reg = 0;
 
 	snprintf(filename, sizeof(filename),
 		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
@@ -508,26 +511,36 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
 		 addr->function);
 
 	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
-
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+	if (fp != NULL) {
+		/* We have an Intel IOMMU */
+		if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+			   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			fclose(fp);
+			return false;
+		}
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
 		fclose(fp);
-		return false;
+		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
 	}
+	else {
+		snprintf(filename, sizeof(filename),
+			 "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
+			  rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
+			  addr->function);
+
+		fp = fopen(filename, "r");
+		if (fp != NULL) {
+			/* We have an Amd IOMMU */
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+				fclose(fp);
+				return false;
+			}
 
-	fclose(fp);
-
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+			fclose(fp);
+			mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+		}
+	}
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

[PATCH v4] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 48 ++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ebd1395502..cdac634e6c 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -480,15 +480,18 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
 #define VTD_CAP_MGAW_SHIFT	16
 #define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
 	const struct rte_pci_addr *addr = &dev->addr;
 	char filename[PATH_MAX];
 	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	uint64_t mgaw = 0, cap_reg = 0;
 
 	snprintf(filename, sizeof(filename),
 		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
@@ -496,26 +499,39 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
 		 addr->function);
 
 	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
-
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+	if (fp != NULL) {
+		/* We have an Intel IOMMU */
+		if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+			   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			fclose(fp);
+			return false;
+		}
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
 		fclose(fp);
-		return false;
+		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
 	}
+	else {
+		snprintf(filename, sizeof(filename),
+			 "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
+			  rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
+			  addr->function);
+
+		fp = fopen(filename, "r");
+		if (fp != NULL) {
+			/* We have an Amd IOMMU */
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+				fclose(fp);
+				return false;
+			}
 
-	fclose(fp);
+			fclose(fp);
+			mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+		}
+	}
 
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+	if (mgaw == 0)
+		return false;
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

[PATCH v4] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 48 ++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ebd1395502..cdac634e6c 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -480,15 +480,18 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
 #define VTD_CAP_MGAW_SHIFT	16
 #define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
 	const struct rte_pci_addr *addr = &dev->addr;
 	char filename[PATH_MAX];
 	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	uint64_t mgaw = 0, cap_reg = 0;
 
 	snprintf(filename, sizeof(filename),
 		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
@@ -496,26 +499,39 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
 		 addr->function);
 
 	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
-
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+	if (fp != NULL) {
+		/* We have an Intel IOMMU */
+		if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+			   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			fclose(fp);
+			return false;
+		}
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
 		fclose(fp);
-		return false;
+		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
 	}
+	else {
+		snprintf(filename, sizeof(filename),
+			 "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
+			  rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
+			  addr->function);
+
+		fp = fopen(filename, "r");
+		if (fp != NULL) {
+			/* We have an Amd IOMMU */
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+				fclose(fp);
+				return false;
+			}
 
-	fclose(fp);
+			fclose(fp);
+			mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+		}
+	}
 
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+	if (mgaw == 0)
+		return false;
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

Re: [PATCH v3] pci: read amd iommu virtual address width

[Ferruh Yigit]

On 10/10/2022 10:47 PM, Michael Piszczek wrote:
> Add code to read the virtual address width for AMD processors.
> 
> Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
> ---
>   drivers/bus/pci/linux/pci.c | 47 +++++++++++++++++++++++--------------
>   1 file changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index e521459870..8c6082ee7a 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -492,15 +492,18 @@ rte_pci_scan(void)
>   }
>   
>   #if defined(RTE_ARCH_X86)
> +
>   bool
>   pci_device_iommu_support_va(const struct rte_pci_device *dev)
>   {
>   #define VTD_CAP_MGAW_SHIFT	16
>   #define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
> +#define RD_AMD_CAP_VASIZE_SHIFT 15
> +#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
>   	const struct rte_pci_addr *addr = &dev->addr;
>   	char filename[PATH_MAX];
>   	FILE *fp;
> -	uint64_t mgaw, vtd_cap_reg = 0;
> +	uint64_t mgaw, cap_reg = 0;
>   
>   	snprintf(filename, sizeof(filename),
>   		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
> @@ -508,26 +511,36 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
>   		 addr->function);
>   
>   	fp = fopen(filename, "r");
> -	if (fp == NULL) {
> -		/* We don't have an Intel IOMMU, assume VA supported */
> -		if (errno == ENOENT)
> -			return true;
> -
> -		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
> -			__func__, filename, strerror(errno));
> -		return false;
> -	}
> +	if (fp != NULL) {
> +		/* We have an Intel IOMMU */
> +		if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
> +			   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
> +			fclose(fp);
> +			return false;
> +		}
>   
> -	/* We have an Intel IOMMU */
> -	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
> -		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
>   		fclose(fp);
> -		return false;
> +		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
>   	}
> +	else {
> +		snprintf(filename, sizeof(filename),
> +			 "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
> +			  rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> +			  addr->function);
> +
> +		fp = fopen(filename, "r");
> +		if (fp != NULL) {
> +			/* We have an Amd IOMMU */
> +			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
> +				   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
> +				fclose(fp);
> +				return false;
> +			}

Instead what do you think to use 'glob()' with
"%s/" PCI_PRI_FMT "/iommu/*-iommu/cap" to detect the file name?

>   
> -	fclose(fp);
> -
> -	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
> +			fclose(fp);
> +			mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
> +		}
> +	}
>   
>   	/*
>   	 * Assuming there is no limitation by now. We can not know at this point

Re: [PATCH v4] pci: read amd iommu virtual address width

[Ferruh Yigit]

On 10/11/2022 3:08 PM, Michael Piszczek wrote:
> Add code to read the virtual address width for AMD processors.
> 
> Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
> ---
>   drivers/bus/pci/linux/pci.c | 48 ++++++++++++++++++++++++-------------
>   1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index ebd1395502..cdac634e6c 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -480,15 +480,18 @@ rte_pci_scan(void)
>   }
>   
>   #if defined(RTE_ARCH_X86)
> +
>   bool
>   pci_device_iommu_support_va(const struct rte_pci_device *dev)
>   {
>   #define VTD_CAP_MGAW_SHIFT	16
>   #define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
> +#define RD_AMD_CAP_VASIZE_SHIFT 15
> +#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
>   	const struct rte_pci_addr *addr = &dev->addr;
>   	char filename[PATH_MAX];
>   	FILE *fp;
> -	uint64_t mgaw, vtd_cap_reg = 0;
> +	uint64_t mgaw = 0, cap_reg = 0;
>   
>   	snprintf(filename, sizeof(filename),
>   		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
> @@ -496,26 +499,39 @@ pci_device_iommu_support_va(const struct rte_pci_device *dev)
>   		 addr->function);
>   
>   	fp = fopen(filename, "r");
> -	if (fp == NULL) {
> -		/* We don't have an Intel IOMMU, assume VA supported */
> -		if (errno == ENOENT)
> -			return true;
> -
> -		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
> -			__func__, filename, strerror(errno));
> -		return false;
> -	}
> +	if (fp != NULL) {
> +		/* We have an Intel IOMMU */
> +		if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
> +			   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
> +			fclose(fp);
> +			return false;
> +		}
>   
> -	/* We have an Intel IOMMU */
> -	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
> -		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
>   		fclose(fp);
> -		return false;
> +		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
>   	}
> +	else {
> +		snprintf(filename, sizeof(filename),
> +			 "%s/" PCI_PRI_FMT "/iommu/amd-iommu/cap",
> +			  rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> +			  addr->function);
> +


As suggested in v3, instead of first trying to open intel file later amd 
file, what do you think to use glob() function to detect the file?

> +		fp = fopen(filename, "r");
> +		if (fp != NULL) {
> +			/* We have an Amd IOMMU */
> +			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
> +				   RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
> +				fclose(fp);
> +				return false;
> +			}
>   
> -	fclose(fp);
> +			fclose(fp);
> +			mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
> +		}
> +	}
>   
> -	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
> +	if (mgaw == 0)
> +		return false;
>   
>   	/*
>   	 * Assuming there is no limitation by now. We can not know at this point

Re: [PATCH v4] pci: read amd iommu virtual address width

[Stephen Hemminger]

On Tue, 11 Oct 2022 16:08:53 +0200
Michael Piszczek <mpiszczek@ddn.com> wrote:

> +		mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
>  	}
> +	else {

Minor nit, the kernel style used by DPDK uses "cuddle else"

       } else {

[PATCH v5] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.
Updated pci_device_iommu_support_va() to use glob to find iommu
capability files.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 58 ++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ebd1395502..291090ba7b 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -4,6 +4,7 @@
 
 #include <string.h>
 #include <dirent.h>
+#include <glob.h>
 
 #include <rte_log.h>
 #include <rte_pci.h>
@@ -480,42 +481,53 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
-#define VTD_CAP_MGAW_SHIFT	16
-#define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define VTD_CAP_MGAW_SHIFT      16
+#define VTD_CAP_MGAW_MASK       (0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
+	int rc;
 	const struct rte_pci_addr *addr = &dev->addr;
-	char filename[PATH_MAX];
-	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	char pattern[PATH_MAX];
+	glob_t glob_results;
+	uint64_t mgaw = 0;
 
-	snprintf(filename, sizeof(filename),
-		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
+	snprintf(pattern, sizeof(pattern),
+		 "%s/" PCI_PRI_FMT "/iommu/*-iommu/cap",
 		 rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
+	rc = glob(pattern, 0, NULL, &glob_results);
+	if (rc != 0 && glob_results.gl_pathc == 1) {
+		const char *filename = glob_results.gl_pathv[0];
+		FILE *fp = fopen(filename, "r");
 
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+		if (fp != NULL) {
+			uint64_t cap_reg = 0;
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
-		fclose(fp);
-		return false;
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			}
+			else if (strstr(filename, "intel-iommu") != NULL) {
+				/* We have an Intel IOMMU */
+				mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+			}
+			else if (strstr(filename, "amd-iommu") != NULL) {
+				/* We have an Amd IOMMU */
+				mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+			}
+
+			fclose(fp);
+		}
 	}
 
-	fclose(fp);
+	globfree(&glob_results);
 
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+	if (mgaw == 0)
+		return false;
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

[PATCH v5] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.
Updated pci_device_iommu_support_va() to use glob to find iommu
capability files.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 58 ++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ebd1395502..291090ba7b 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -4,6 +4,7 @@
 
 #include <string.h>
 #include <dirent.h>
+#include <glob.h>
 
 #include <rte_log.h>
 #include <rte_pci.h>
@@ -480,42 +481,53 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
-#define VTD_CAP_MGAW_SHIFT	16
-#define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define VTD_CAP_MGAW_SHIFT      16
+#define VTD_CAP_MGAW_MASK       (0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
+	int rc;
 	const struct rte_pci_addr *addr = &dev->addr;
-	char filename[PATH_MAX];
-	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	char pattern[PATH_MAX];
+	glob_t glob_results;
+	uint64_t mgaw = 0;
 
-	snprintf(filename, sizeof(filename),
-		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
+	snprintf(pattern, sizeof(pattern),
+		 "%s/" PCI_PRI_FMT "/iommu/*-iommu/cap",
 		 rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
+	rc = glob(pattern, 0, NULL, &glob_results);
+	if (rc != 0 && glob_results.gl_pathc == 1) {
+		const char *filename = glob_results.gl_pathv[0];
+		FILE *fp = fopen(filename, "r");
 
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+		if (fp != NULL) {
+			uint64_t cap_reg = 0;
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
-		fclose(fp);
-		return false;
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			}
+			else if (strstr(filename, "intel-iommu") != NULL) {
+				/* We have an Intel IOMMU */
+				mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+			}
+			else if (strstr(filename, "amd-iommu") != NULL) {
+				/* We have an Amd IOMMU */
+				mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+			}
+
+			fclose(fp);
+		}
 	}
 
-	fclose(fp);
+	globfree(&glob_results);
 
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+	if (mgaw == 0)
+		return false;
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

[PATCH v6] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.
Updated pci_device_iommu_support_va() to use glob to find iommu
capability files.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 58 ++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ebd1395502..291090ba7b 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -4,6 +4,7 @@
 
 #include <string.h>
 #include <dirent.h>
+#include <glob.h>
 
 #include <rte_log.h>
 #include <rte_pci.h>
@@ -480,42 +481,53 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
-#define VTD_CAP_MGAW_SHIFT	16
-#define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define VTD_CAP_MGAW_SHIFT      16
+#define VTD_CAP_MGAW_MASK       (0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
+	int rc;
 	const struct rte_pci_addr *addr = &dev->addr;
-	char filename[PATH_MAX];
-	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	char pattern[PATH_MAX];
+	glob_t glob_results;
+	uint64_t mgaw = 0;
 
-	snprintf(filename, sizeof(filename),
-		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
+	snprintf(pattern, sizeof(pattern),
+		 "%s/" PCI_PRI_FMT "/iommu/*-iommu/cap",
 		 rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
+	rc = glob(pattern, 0, NULL, &glob_results);
+	if (rc != 0 && glob_results.gl_pathc == 1) {
+		const char *filename = glob_results.gl_pathv[0];
+		FILE *fp = fopen(filename, "r");
 
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+		if (fp != NULL) {
+			uint64_t cap_reg = 0;
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
-		fclose(fp);
-		return false;
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			}
+			else if (strstr(filename, "intel-iommu") != NULL) {
+				/* We have an Intel IOMMU */
+				mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+			}
+			else if (strstr(filename, "amd-iommu") != NULL) {
+				/* We have an Amd IOMMU */
+				mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+			}
+
+			fclose(fp);
+		}
 	}
 
-	fclose(fp);
+	globfree(&glob_results);
 
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+	if (mgaw == 0)
+		return false;
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

[PATCH v6] pci: read amd iommu virtual address width

[Michael Piszczek]

Add code to read the virtual address width for AMD processors.
Updated pci_device_iommu_support_va() to use glob to find iommu
capability files.

Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
---
 drivers/bus/pci/linux/pci.c | 58 ++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ebd1395502..291090ba7b 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -4,6 +4,7 @@
 
 #include <string.h>
 #include <dirent.h>
+#include <glob.h>
 
 #include <rte_log.h>
 #include <rte_pci.h>
@@ -480,42 +481,53 @@ rte_pci_scan(void)
 }
 
 #if defined(RTE_ARCH_X86)
+
 bool
 pci_device_iommu_support_va(const struct rte_pci_device *dev)
 {
-#define VTD_CAP_MGAW_SHIFT	16
-#define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define VTD_CAP_MGAW_SHIFT      16
+#define VTD_CAP_MGAW_MASK       (0x3fULL << VTD_CAP_MGAW_SHIFT)
+#define RD_AMD_CAP_VASIZE_SHIFT 15
+#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
+	int rc;
 	const struct rte_pci_addr *addr = &dev->addr;
-	char filename[PATH_MAX];
-	FILE *fp;
-	uint64_t mgaw, vtd_cap_reg = 0;
+	char pattern[PATH_MAX];
+	glob_t glob_results;
+	uint64_t mgaw = 0;
 
-	snprintf(filename, sizeof(filename),
-		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
+	snprintf(pattern, sizeof(pattern),
+		 "%s/" PCI_PRI_FMT "/iommu/*-iommu/cap",
 		 rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		/* We don't have an Intel IOMMU, assume VA supported */
-		if (errno == ENOENT)
-			return true;
+	rc = glob(pattern, 0, NULL, &glob_results);
+	if (rc != 0 && glob_results.gl_pathc == 1) {
+		const char *filename = glob_results.gl_pathv[0];
+		FILE *fp = fopen(filename, "r");
 
-		RTE_LOG(ERR, EAL, "%s(): can't open %s: %s\n",
-			__func__, filename, strerror(errno));
-		return false;
-	}
+		if (fp != NULL) {
+			uint64_t cap_reg = 0;
 
-	/* We have an Intel IOMMU */
-	if (fscanf(fp, "%" PRIx64, &vtd_cap_reg) != 1) {
-		RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
-		fclose(fp);
-		return false;
+			if (fscanf(fp, "%" PRIx64, &cap_reg) != 1) {
+				RTE_LOG(ERR, EAL, "%s(): can't read %s\n", __func__, filename);
+			}
+			else if (strstr(filename, "intel-iommu") != NULL) {
+				/* We have an Intel IOMMU */
+				mgaw = ((cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+			}
+			else if (strstr(filename, "amd-iommu") != NULL) {
+				/* We have an Amd IOMMU */
+				mgaw = ((cap_reg & RD_AMD_CAP_VASIZE_MASK) >> RD_AMD_CAP_VASIZE_SHIFT) + 1;
+			}
+
+			fclose(fp);
+		}
 	}
 
-	fclose(fp);
+	globfree(&glob_results);
 
-	mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1;
+	if (mgaw == 0)
+		return false;
 
 	/*
 	 * Assuming there is no limitation by now. We can not know at this point
-- 
2.34.1

Re: [PATCH v5] pci: read amd iommu virtual address width

[Stephen Hemminger]

On Thu, 13 Oct 2022 20:16:02 +0200
Michael Piszczek <mpiszczek@ddn.com> wrote:

> Add code to read the virtual address width for AMD processors.
> Updated pci_device_iommu_support_va() to use glob to find iommu
> capability files.
> 
> Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
> ---
>  drivers/bus/pci/linux/pci.c | 58 ++++++++++++++++++++++---------------
>  1 file changed, 35 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index ebd1395502..291090ba7b 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -4,6 +4,7 @@
>  
>  #include <string.h>
>  #include <dirent.h>
> +#include <glob.h>
>  
>  #include <rte_log.h>
>  #include <rte_pci.h>
> @@ -480,42 +481,53 @@ rte_pci_scan(void)
>  }
>  
>  #if defined(RTE_ARCH_X86)
> +
>  bool
>  pci_device_iommu_support_va(const struct rte_pci_device *dev)
>  {
> -#define VTD_CAP_MGAW_SHIFT	16
> -#define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
> +#define VTD_CAP_MGAW_SHIFT      16
> +#define VTD_CAP_MGAW_MASK       (0x3fULL << VTD_CAP_MGAW_SHIFT)
> +#define RD_AMD_CAP_VASIZE_SHIFT 15
> +#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
> +	int rc;
>  	const struct rte_pci_addr *addr = &dev->addr;
> -	char filename[PATH_MAX];
> -	FILE *fp;
> -	uint64_t mgaw, vtd_cap_reg = 0;
> +	char pattern[PATH_MAX];
> +	glob_t glob_results;
> +	uint64_t mgaw = 0;
>  
> -	snprintf(filename, sizeof(filename),
> -		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
> +	snprintf(pattern, sizeof(pattern),
> +		 "%s/" PCI_PRI_FMT "/iommu/*-iommu/cap",
>  		 rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
>  		 addr->function);
>  
> -	fp = fopen(filename, "r");
> -	if (fp == NULL) {
> -		/* We don't have an Intel IOMMU, assume VA supported */
> -		if (errno == ENOENT)
> -			return true;
> +	rc = glob(pattern, 0, NULL, &glob_results);
> +	if (rc != 0 && glob_results.gl_pathc == 1) {
> +		const char *filename = glob_results.gl_pathv[0];

Why not use fnmatch() instead of glob()?
Most of the places in DPDK use that to do this kind of matching.

Re: [PATCH v5] pci: read amd iommu virtual address width

[Ferruh Yigit]

On 10/24/2022 7:09 PM, Stephen Hemminger wrote:
> On Thu, 13 Oct 2022 20:16:02 +0200
> Michael Piszczek <mpiszczek@ddn.com> wrote:
> 
>> Add code to read the virtual address width for AMD processors.
>> Updated pci_device_iommu_support_va() to use glob to find iommu
>> capability files.
>>
>> Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
>> ---
>>   drivers/bus/pci/linux/pci.c | 58 ++++++++++++++++++++++---------------
>>   1 file changed, 35 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
>> index ebd1395502..291090ba7b 100644
>> --- a/drivers/bus/pci/linux/pci.c
>> +++ b/drivers/bus/pci/linux/pci.c
>> @@ -4,6 +4,7 @@
>>   
>>   #include <string.h>
>>   #include <dirent.h>
>> +#include <glob.h>
>>   
>>   #include <rte_log.h>
>>   #include <rte_pci.h>
>> @@ -480,42 +481,53 @@ rte_pci_scan(void)
>>   }
>>   
>>   #if defined(RTE_ARCH_X86)
>> +
>>   bool
>>   pci_device_iommu_support_va(const struct rte_pci_device *dev)
>>   {
>> -#define VTD_CAP_MGAW_SHIFT	16
>> -#define VTD_CAP_MGAW_MASK	(0x3fULL << VTD_CAP_MGAW_SHIFT)
>> +#define VTD_CAP_MGAW_SHIFT      16
>> +#define VTD_CAP_MGAW_MASK       (0x3fULL << VTD_CAP_MGAW_SHIFT)
>> +#define RD_AMD_CAP_VASIZE_SHIFT 15
>> +#define RD_AMD_CAP_VASIZE_MASK  (0x7F << RD_AMD_CAP_VASIZE_SHIFT)
>> +	int rc;
>>   	const struct rte_pci_addr *addr = &dev->addr;
>> -	char filename[PATH_MAX];
>> -	FILE *fp;
>> -	uint64_t mgaw, vtd_cap_reg = 0;
>> +	char pattern[PATH_MAX];
>> +	glob_t glob_results;
>> +	uint64_t mgaw = 0;
>>   
>> -	snprintf(filename, sizeof(filename),
>> -		 "%s/" PCI_PRI_FMT "/iommu/intel-iommu/cap",
>> +	snprintf(pattern, sizeof(pattern),
>> +		 "%s/" PCI_PRI_FMT "/iommu/*-iommu/cap",
>>   		 rte_pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
>>   		 addr->function);
>>   
>> -	fp = fopen(filename, "r");
>> -	if (fp == NULL) {
>> -		/* We don't have an Intel IOMMU, assume VA supported */
>> -		if (errno == ENOENT)
>> -			return true;
>> +	rc = glob(pattern, 0, NULL, &glob_results);
>> +	if (rc != 0 && glob_results.gl_pathc == 1) {
>> +		const char *filename = glob_results.gl_pathv[0];
> 
> Why not use fnmatch() instead of glob()?
> Most of the places in DPDK use that to do this kind of matching.

Since glob() returns matching string, it can be useful to open the file.
Is there an advantage of fnmatch() against globe()?

Re: [PATCH v6] pci: read amd iommu virtual address width

[David Marchand]

On Mon, Oct 24, 2022 at 5:35 PM Michael Piszczek <mpiszczek@ddn.com> wrote:
>
> Add code to read the virtual address width for AMD processors.
> Updated pci_device_iommu_support_va() to use glob to find iommu
> capability files.
>
> Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>

Please have a look at the ci.

Unit tests are failing with this patch because the default is changed
from VA to PA.
A quick way to reproduce is to run vdev unit tests as a normal user in
no huge mode.

Before change:
$ DPDK_TEST=vdev_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
2048 --log-level=*:debug
...
EAL: Bus vdev wants IOVA as 'DC'
EAL: Bus pci wants IOVA as 'DC'
EAL: Buses did not request a specific IOVA mode.
EAL: Physical addresses are unavailable, selecting IOVA as VA mode.
EAL: Selected IOVA mode 'VA'

After change:
$ DPDK_TEST=vdev_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
2048 --log-level=*:debug
...
EAL: Bus vdev wants IOVA as 'DC'
EAL: Bus pci wants IOVA as 'PA'
EAL: FATAL: Cannot use IOVA as 'PA' since physical addresses are not available
EAL: Cannot use IOVA as 'PA' since physical addresses are not available


-- 
David Marchand

Re: [PATCH v6] pci: read amd iommu virtual address width

[David Marchand]

On Tue, Oct 25, 2022 at 1:54 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, Oct 24, 2022 at 5:35 PM Michael Piszczek <mpiszczek@ddn.com> wrote:
> >
> > Add code to read the virtual address width for AMD processors.
> > Updated pci_device_iommu_support_va() to use glob to find iommu
> > capability files.
> >
> > Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
>
> Please have a look at the ci.
>
> Unit tests are failing with this patch because the default is changed
> from VA to PA.
> A quick way to reproduce is to run vdev unit tests as a normal user in
> no huge mode.
>
> Before change:
> $ DPDK_TEST=vdev_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
> 2048 --log-level=*:debug
> ...
> EAL: Bus vdev wants IOVA as 'DC'
> EAL: Bus pci wants IOVA as 'DC'
> EAL: Buses did not request a specific IOVA mode.
> EAL: Physical addresses are unavailable, selecting IOVA as VA mode.
> EAL: Selected IOVA mode 'VA'
>
> After change:
> $ DPDK_TEST=vdev_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
> 2048 --log-level=*:debug
> ...
> EAL: Bus vdev wants IOVA as 'DC'
> EAL: Bus pci wants IOVA as 'PA'
> EAL: FATAL: Cannot use IOVA as 'PA' since physical addresses are not available
> EAL: Cannot use IOVA as 'PA' since physical addresses are not available

I got no reply since a few months, marking as "Changes requested" in patchwork.
If you think this is still worth pursuing, please provide a new
revision that does not break IOVA default mode (and the CI).

Cc: Ferruh, for info

-- 
David Marchand

RE: [PATCH v6] pci: read amd iommu virtual address width

[Michael Piszczek]

Hi David,

Sorry I do not have a solution for this. Can you close the patch?

Michael Piszczek

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Tuesday, August 8, 2023 3:31 AM
To: Michael Piszczek <mpiszczek@ddn.com>
Cc: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@amd.com>
Subject: Re: [PATCH v6] pci: read amd iommu virtual address width

On Tue, Oct 25, 2022 at 1:54 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Mon, Oct 24, 2022 at 5:35 PM Michael Piszczek <mpiszczek@ddn.com> wrote:
> >
> > Add code to read the virtual address width for AMD processors.
> > Updated pci_device_iommu_support_va() to use glob to find iommu 
> > capability files.
> >
> > Signed-off-by: Michael Piszczek <mpiszczek@ddn.com>
>
> Please have a look at the ci.
>
> Unit tests are failing with this patch because the default is changed 
> from VA to PA.
> A quick way to reproduce is to run vdev unit tests as a normal user in 
> no huge mode.
>
> Before change:
> $ DPDK_TEST=vdev_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
> 2048 --log-level=*:debug
> ...
> EAL: Bus vdev wants IOVA as 'DC'
> EAL: Bus pci wants IOVA as 'DC'
> EAL: Buses did not request a specific IOVA mode.
> EAL: Physical addresses are unavailable, selecting IOVA as VA mode.
> EAL: Selected IOVA mode 'VA'
>
> After change:
> $ DPDK_TEST=vdev_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
> 2048 --log-level=*:debug
> ...
> EAL: Bus vdev wants IOVA as 'DC'
> EAL: Bus pci wants IOVA as 'PA'
> EAL: FATAL: Cannot use IOVA as 'PA' since physical addresses are not 
> available
> EAL: Cannot use IOVA as 'PA' since physical addresses are not 
> available

I got no reply since a few months, marking as "Changes requested" in patchwork.
If you think this is still worth pursuing, please provide a new revision that does not break IOVA default mode (and the CI).

Cc: Ferruh, for info

--
David Marchand