[v2,1/2] bus/cdx: add support for devices without MSI

Message ID 20231103112016.1945684-1-shubham.rohila@amd.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/2] bus/cdx: add support for devices without MSI |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Shubham Rohila Nov. 3, 2023, 11:20 a.m. UTC
  From: Nikhil Agarwal <nikhil.agarwal@amd.com>

Update the cleanup routine for cdx device to support
device without MSI. Also, set vfio_dev_fd for such devices
This fd can be used for BME reload operations.

Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
---
 v2
 - New patch in the series
 drivers/bus/cdx/cdx.c      |  2 +-
 drivers/bus/cdx/cdx_vfio.c | 19 +++++++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)
  

Comments

Gupta, Nipun Nov. 6, 2023, 5:04 p.m. UTC | #1
On 11/3/2023 4:50 PM, Shubham Rohila wrote:
> From: Nikhil Agarwal <nikhil.agarwal@amd.com>
> 
> Update the cleanup routine for cdx device to support
> device without MSI. Also, set vfio_dev_fd for such devices
> This fd can be used for BME reload operations.
> 
> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
> ---
>   v2
>   - New patch in the series
>   drivers/bus/cdx/cdx.c      |  2 +-
>   drivers/bus/cdx/cdx_vfio.c | 19 +++++++++----------
>   2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
> index 541aae76c3..62b108e082 100644
> --- a/drivers/bus/cdx/cdx.c
> +++ b/drivers/bus/cdx/cdx.c
> @@ -405,9 +405,9 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
>   	return ret;
>   
>   error_probe:
> +	cdx_vfio_unmap_resource(dev);
>   	rte_intr_instance_free(dev->intr_handle);
>   	dev->intr_handle = NULL;
> -	cdx_vfio_unmap_resource(dev);
>   error_map_device:
>   	return ret;
>   }
> diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
> index 8a3ac0b995..8cac79782e 100644
> --- a/drivers/bus/cdx/cdx_vfio.c
> +++ b/drivers/bus/cdx/cdx_vfio.c
> @@ -101,13 +101,12 @@ cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev)
>   	struct mapped_cdx_res_list *vfio_res_list;
>   	int ret, vfio_dev_fd;
>   
> -	if (rte_intr_fd_get(dev->intr_handle) < 0)
> -		return -1;

Why is this check removed? If VFIO fd is not there we may not proceed 
with other VFIO cleanup?
  
Agarwal, Nikhil Nov. 6, 2023, 5:54 p.m. UTC | #2
[AMD Official Use Only - General]

Hi Nipun,

> -----Original Message-----
> > diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
> > index 8a3ac0b995..8cac79782e 100644
> > --- a/drivers/bus/cdx/cdx_vfio.c
> > +++ b/drivers/bus/cdx/cdx_vfio.c
> > @@ -101,13 +101,12 @@ cdx_vfio_unmap_resource_primary(struct
> rte_cdx_device *dev)
> >     struct mapped_cdx_res_list *vfio_res_list;
> >     int ret, vfio_dev_fd;
> >
> > -   if (rte_intr_fd_get(dev->intr_handle) < 0)
> > -           return -1;
>
> Why is this check removed? If VFIO fd is not there we may not proceed with
> other VFIO cleanup?

This check was removed from here but added later for interrupt specific cleanup
section. It was causing other cleanup in the function to be skipped in case device
does not support MSI.
  
Gupta, Nipun Nov. 7, 2023, 2:25 p.m. UTC | #3
On 11/3/2023 4:50 PM, Shubham Rohila wrote:
> From: Nikhil Agarwal <nikhil.agarwal@amd.com>
> 
> Update the cleanup routine for cdx device to support
> device without MSI. Also, set vfio_dev_fd for such devices
> This fd can be used for BME reload operations.
> 
> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>

Acked-by: Nipun Gupta <nipun.gupta@amd.com>
  

Patch

diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index 541aae76c3..62b108e082 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -405,9 +405,9 @@  cdx_probe_one_driver(struct rte_cdx_driver *dr,
 	return ret;
 
 error_probe:
+	cdx_vfio_unmap_resource(dev);
 	rte_intr_instance_free(dev->intr_handle);
 	dev->intr_handle = NULL;
-	cdx_vfio_unmap_resource(dev);
 error_map_device:
 	return ret;
 }
diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
index 8a3ac0b995..8cac79782e 100644
--- a/drivers/bus/cdx/cdx_vfio.c
+++ b/drivers/bus/cdx/cdx_vfio.c
@@ -101,13 +101,12 @@  cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev)
 	struct mapped_cdx_res_list *vfio_res_list;
 	int ret, vfio_dev_fd;
 
-	if (rte_intr_fd_get(dev->intr_handle) < 0)
-		return -1;
-
-	if (close(rte_intr_fd_get(dev->intr_handle)) < 0) {
-		CDX_BUS_ERR("Error when closing eventfd file descriptor for %s",
-			dev->device.name);
-		return -1;
+	if (rte_intr_fd_get(dev->intr_handle) >= 0) {
+		if (close(rte_intr_fd_get(dev->intr_handle)) < 0) {
+			CDX_BUS_ERR("Error when closing eventfd file descriptor for %s",
+				dev->device.name);
+			return -1;
+		}
 	}
 
 	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
@@ -185,6 +184,9 @@  cdx_vfio_setup_interrupts(struct rte_cdx_device *dev, int vfio_dev_fd,
 {
 	int i, ret;
 
+	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
+		return -1;
+
 	if (num_irqs == 0)
 		return 0;
 
@@ -227,9 +229,6 @@  cdx_vfio_setup_interrupts(struct rte_cdx_device *dev, int vfio_dev_fd,
 		if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_VFIO_MSIX))
 			return -1;
 
-		if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
-			return -1;
-
 		return 0;
 	}