On Tue, Feb 20, 2024 at 9:11 AM Shannon Nelson <shannon.nelson@xxxxxxx> wrote:
>
> This addresses a couple of things found while testing the FLR and AER
> handling with the VFs.
> - release irqs before calling vp_modern_remove()
> - make sure we have a valid struct pointer before using it to release irqs
> - make sure the FW is alive before trying to add a new device
>
> Signed-off-by: Shannon Nelson <shannon.nelson@xxxxxxx>
> ---
> drivers/vdpa/pds/aux_drv.c | 2 +-
> drivers/vdpa/pds/vdpa_dev.c | 20 +++++++++++++++++---
> drivers/vdpa/pds/vdpa_dev.h | 1 +
> 3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/pds/aux_drv.c b/drivers/vdpa/pds/aux_drv.c
> index 186e9ee22eb1..f57330cf9024 100644
> --- a/drivers/vdpa/pds/aux_drv.c
> +++ b/drivers/vdpa/pds/aux_drv.c
> @@ -93,8 +93,8 @@ static void pds_vdpa_remove(struct auxiliary_device *aux_dev)
> struct device *dev = &aux_dev->dev;
>
> vdpa_mgmtdev_unregister(&vdpa_aux->vdpa_mdev);
> + pds_vdpa_release_irqs(vdpa_aux->pdsv);
> vp_modern_remove(&vdpa_aux->vd_mdev);
> - pci_free_irq_vectors(vdpa_aux->padev->vf_pdev);
>
> pds_vdpa_debugfs_del_vdpadev(vdpa_aux);
> kfree(vdpa_aux);
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index 25c0fe5ec3d5..301d95e08596 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -426,12 +426,18 @@ static int pds_vdpa_request_irqs(struct pds_vdpa_device *pdsv)
> return err;
> }
>
> -static void pds_vdpa_release_irqs(struct pds_vdpa_device *pdsv)
> +void pds_vdpa_release_irqs(struct pds_vdpa_device *pdsv)
> {
> - struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
> - struct pds_vdpa_aux *vdpa_aux = pdsv->vdpa_aux;
> + struct pds_vdpa_aux *vdpa_aux;
> + struct pci_dev *pdev;
> int qid;
>
> + if (!pdsv)
> + return;
> +
> + pdev = pdsv->vdpa_aux->padev->vf_pdev;
> + vdpa_aux = pdsv->vdpa_aux;
> +
> if (!vdpa_aux->nintrs)
> return;
>
> @@ -612,6 +618,7 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> struct device *dma_dev;
> struct pci_dev *pdev;
> struct device *dev;
> + u8 status;
> int err;
> int i;
>
> @@ -638,6 +645,13 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> dma_dev = &pdev->dev;
> pdsv->vdpa_dev.dma_dev = dma_dev;
>
> + status = pds_vdpa_get_status(&pdsv->vdpa_dev);
> + if (status == 0xff) {
> + dev_err(dev, "Broken PCI - status %#x\n", status);
> + err = -ENXIO;
> + goto err_unmap;
> + }
This seems a violation of the virtio spec.
Note that spec has a DEVICE_NEEDS_RESET(64) or do we have a vendor
specific way to detect this instead?
Thanks
> +
> pdsv->supported_features = mgmt->supported_features;
>
> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h
> index d984ba24a7da..84bdb45871ff 100644
> --- a/drivers/vdpa/pds/vdpa_dev.h
> +++ b/drivers/vdpa/pds/vdpa_dev.h
> @@ -46,5 +46,6 @@ struct pds_vdpa_device {
>
> #define PDS_VDPA_PACKED_INVERT_IDX 0x8000
>
> +void pds_vdpa_release_irqs(struct pds_vdpa_device *pdsv);
> int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux);
> #endif /* _VDPA_DEV_H_ */
> --
> 2.17.1
>
