[v2,1/2] bus/cdx: add support for devices without MSI
Checks
Commit Message
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
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?
[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.
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>
@@ -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;
}
@@ -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;
}