[v5] pci: read amd iommu virtual address width

Message ID 20221013181602.873166-2-mpiszczek@ddn.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v5] pci: read amd iommu virtual address width |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS

Commit Message

Michael Piszczek Oct. 13, 2022, 6:16 p.m. UTC
  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(-)
  

Comments

Stephen Hemminger Oct. 24, 2022, 6:09 p.m. UTC | #1
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.
  
Ferruh Yigit Oct. 25, 2022, 7:56 a.m. UTC | #2
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()?
  

Patch

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