[v2,03/15] bus/pci: rework MSIX discovery with VFIO

Message ID 20230821113549.3191921-4-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Cleanup PCI(e) drivers |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand Aug. 21, 2023, 11:35 a.m. UTC
  This is a preparatory step before using new helpers for finding PCI
capabilities.
In the code querying PCI capabilities for checking MSIX availability,
replace direct calls to VFIO fd with the existing helpers for reading
PCI configuration space: this requires setting VFIO fd in the PCI
device object than was done before this change and removes the need to
pass around this vfio_dev_fd variable.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)
  

Comments

Chenbo Xia Sept. 6, 2023, 1:03 p.m. UTC | #1
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; ferruh.yigit@amd.com; Xia, Chenbo
> <chenbo.xia@intel.com>; nipun.gupta@amd.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [PATCH v2 03/15] bus/pci: rework MSIX discovery with VFIO
> 
> This is a preparatory step before using new helpers for finding PCI
> capabilities.
> In the code querying PCI capabilities for checking MSIX availability,
> replace direct calls to VFIO fd with the existing helpers for reading
> PCI configuration space: this requires setting VFIO fd in the PCI
> device object than was done before this change and removes the need to
> pass around this vfio_dev_fd variable.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/bus/pci/linux/pci_vfio.c | 27 ++++++++++-----------------
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index 8fa7fa458f..958f8b3b52 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -107,23 +107,16 @@ pci_vfio_write_config(const struct rte_pci_device
> *dev,
> 
>  /* get PCI BAR number where MSI-X interrupts are */
>  static int
> -pci_vfio_get_msix_bar(const struct rte_pci_device *dev, int fd,
> +pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
>  	struct pci_msix_table *msix_table)
>  {
>  	int ret;
>  	uint32_t reg;
>  	uint16_t flags;
>  	uint8_t cap_id, cap_offset;
> -	uint64_t size, offset;
> -
> -	if (pci_vfio_get_region(dev, VFIO_PCI_CONFIG_REGION_INDEX,
> -		&size, &offset) != 0) {
> -		RTE_LOG(ERR, EAL, "Cannot get offset of CONFIG region.\n");
> -		return -1;
> -	}
> 
>  	/* read PCI capability pointer from config space */
> -	ret = pread64(fd, &reg, sizeof(reg), offset + PCI_CAPABILITY_LIST);
> +	ret = rte_pci_read_config(dev, &reg, sizeof(reg),
> PCI_CAPABILITY_LIST);
>  	if (ret != sizeof(reg)) {
>  		RTE_LOG(ERR, EAL,
>  			"Cannot read capability pointer from PCI config
> space!\n");
> @@ -136,7 +129,7 @@ pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
> int fd,
>  	while (cap_offset) {
> 
>  		/* read PCI capability ID */
> -		ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset);
> +		ret = rte_pci_read_config(dev, &reg, sizeof(reg), cap_offset);
>  		if (ret != sizeof(reg)) {
>  			RTE_LOG(ERR, EAL,
>  				"Cannot read capability ID from PCI config
> space!\n");
> @@ -148,7 +141,7 @@ pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
> int fd,
> 
>  		/* if we haven't reached MSI-X, check next capability */
>  		if (cap_id != PCI_CAP_ID_MSIX) {
> -			ret = pread64(fd, &reg, sizeof(reg), offset +
> cap_offset);
> +			ret = rte_pci_read_config(dev, &reg, sizeof(reg),
> cap_offset);
>  			if (ret != sizeof(reg)) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot read capability pointer from PCI
> config space!\n");
> @@ -163,14 +156,14 @@ pci_vfio_get_msix_bar(const struct rte_pci_device
> *dev, int fd,
>  		/* else, read table offset */
>  		else {
>  			/* table offset resides in the next 4 bytes */
> -			ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset
> + 4);
> +			ret = rte_pci_read_config(dev, &reg, sizeof(reg),
> cap_offset + 4);
>  			if (ret != sizeof(reg)) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot read table offset from PCI config
> space!\n");
>  				return -1;
>  			}
> 
> -			ret = pread64(fd, &flags, sizeof(flags), offset +
> cap_offset + 2);
> +			ret = rte_pci_read_config(dev, &flags, sizeof(flags),
> cap_offset + 2);
>  			if (ret != sizeof(flags)) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot read table flags from PCI config
> space!\n");
> @@ -306,9 +299,6 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev,
> int vfio_dev_fd)
>  		if (rte_intr_fd_set(dev->intr_handle, fd))
>  			return -1;
> 
> -		if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
> -			return -1;
> -
>  		switch (i) {
>  		case VFIO_PCI_MSIX_IRQ_INDEX:
>  			intr_mode = RTE_INTR_MODE_MSIX;
> @@ -838,6 +828,9 @@ pci_vfio_map_resource_primary(struct rte_pci_device
> *dev)
>  	if (ret)
>  		return ret;
> 
> +	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
> +		goto err_vfio_dev_fd;
> +
>  	/* allocate vfio_res and get region info */
>  	vfio_res = rte_zmalloc("VFIO_RES", sizeof(*vfio_res), 0);
>  	if (vfio_res == NULL) {
> @@ -869,7 +862,7 @@ pci_vfio_map_resource_primary(struct rte_pci_device
> *dev)
>  	/* get MSI-X BAR, if any (we have to know where it is because we
> can't
>  	 * easily mmap it when using VFIO)
>  	 */
> -	ret = pci_vfio_get_msix_bar(dev, vfio_dev_fd, &vfio_res->msix_table);
> +	ret = pci_vfio_get_msix_bar(dev, &vfio_res->msix_table);
>  	if (ret < 0) {
>  		RTE_LOG(ERR, EAL, "%s cannot get MSI-X BAR number!\n",
>  				pci_addr);
> --
> 2.41.0

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  

Patch

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 8fa7fa458f..958f8b3b52 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -107,23 +107,16 @@  pci_vfio_write_config(const struct rte_pci_device *dev,
 
 /* get PCI BAR number where MSI-X interrupts are */
 static int
-pci_vfio_get_msix_bar(const struct rte_pci_device *dev, int fd,
+pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
 	struct pci_msix_table *msix_table)
 {
 	int ret;
 	uint32_t reg;
 	uint16_t flags;
 	uint8_t cap_id, cap_offset;
-	uint64_t size, offset;
-
-	if (pci_vfio_get_region(dev, VFIO_PCI_CONFIG_REGION_INDEX,
-		&size, &offset) != 0) {
-		RTE_LOG(ERR, EAL, "Cannot get offset of CONFIG region.\n");
-		return -1;
-	}
 
 	/* read PCI capability pointer from config space */
-	ret = pread64(fd, &reg, sizeof(reg), offset + PCI_CAPABILITY_LIST);
+	ret = rte_pci_read_config(dev, &reg, sizeof(reg), PCI_CAPABILITY_LIST);
 	if (ret != sizeof(reg)) {
 		RTE_LOG(ERR, EAL,
 			"Cannot read capability pointer from PCI config space!\n");
@@ -136,7 +129,7 @@  pci_vfio_get_msix_bar(const struct rte_pci_device *dev, int fd,
 	while (cap_offset) {
 
 		/* read PCI capability ID */
-		ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset);
+		ret = rte_pci_read_config(dev, &reg, sizeof(reg), cap_offset);
 		if (ret != sizeof(reg)) {
 			RTE_LOG(ERR, EAL,
 				"Cannot read capability ID from PCI config space!\n");
@@ -148,7 +141,7 @@  pci_vfio_get_msix_bar(const struct rte_pci_device *dev, int fd,
 
 		/* if we haven't reached MSI-X, check next capability */
 		if (cap_id != PCI_CAP_ID_MSIX) {
-			ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset);
+			ret = rte_pci_read_config(dev, &reg, sizeof(reg), cap_offset);
 			if (ret != sizeof(reg)) {
 				RTE_LOG(ERR, EAL,
 					"Cannot read capability pointer from PCI config space!\n");
@@ -163,14 +156,14 @@  pci_vfio_get_msix_bar(const struct rte_pci_device *dev, int fd,
 		/* else, read table offset */
 		else {
 			/* table offset resides in the next 4 bytes */
-			ret = pread64(fd, &reg, sizeof(reg), offset + cap_offset + 4);
+			ret = rte_pci_read_config(dev, &reg, sizeof(reg), cap_offset + 4);
 			if (ret != sizeof(reg)) {
 				RTE_LOG(ERR, EAL,
 					"Cannot read table offset from PCI config space!\n");
 				return -1;
 			}
 
-			ret = pread64(fd, &flags, sizeof(flags), offset + cap_offset + 2);
+			ret = rte_pci_read_config(dev, &flags, sizeof(flags), cap_offset + 2);
 			if (ret != sizeof(flags)) {
 				RTE_LOG(ERR, EAL,
 					"Cannot read table flags from PCI config space!\n");
@@ -306,9 +299,6 @@  pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 		if (rte_intr_fd_set(dev->intr_handle, fd))
 			return -1;
 
-		if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
-			return -1;
-
 		switch (i) {
 		case VFIO_PCI_MSIX_IRQ_INDEX:
 			intr_mode = RTE_INTR_MODE_MSIX;
@@ -838,6 +828,9 @@  pci_vfio_map_resource_primary(struct rte_pci_device *dev)
 	if (ret)
 		return ret;
 
+	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
+		goto err_vfio_dev_fd;
+
 	/* allocate vfio_res and get region info */
 	vfio_res = rte_zmalloc("VFIO_RES", sizeof(*vfio_res), 0);
 	if (vfio_res == NULL) {
@@ -869,7 +862,7 @@  pci_vfio_map_resource_primary(struct rte_pci_device *dev)
 	/* get MSI-X BAR, if any (we have to know where it is because we can't
 	 * easily mmap it when using VFIO)
 	 */
-	ret = pci_vfio_get_msix_bar(dev, vfio_dev_fd, &vfio_res->msix_table);
+	ret = pci_vfio_get_msix_bar(dev, &vfio_res->msix_table);
 	if (ret < 0) {
 		RTE_LOG(ERR, EAL, "%s cannot get MSI-X BAR number!\n",
 				pci_addr);