On 5/6/2022 4:51 AM, Jason Gunthorpe wrote:
> Having a consistent pointer in the drvdata will allow the next patch to
> make use of the drvdata from some of the core code helpers.
>
> Use a WARN_ON inside vfio_pci_core_enable() to detect drivers that miss
> this.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 15 +++++++++++----
> drivers/vfio/pci/mlx5/main.c | 15 +++++++++++----
> drivers/vfio/pci/vfio_pci.c | 2 +-
> drivers/vfio/pci/vfio_pci_core.c | 4 ++++
> 4 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 767b5d47631a49..e92376837b29e6 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -337,6 +337,14 @@ static int vf_qm_cache_wb(struct hisi_qm *qm)
> return 0;
> }
>
> +static struct hisi_acc_vf_core_device *hssi_acc_drvdata(struct pci_dev *pdev)
> +{
> + struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev);
> +
> + return container_of(core_device, struct hisi_acc_vf_core_device,
> + core_device);
> +}
> +
> static void vf_qm_fun_reset(struct hisi_acc_vf_core_device *hisi_acc_vdev,
> struct hisi_qm *qm)
> {
> @@ -962,7 +970,7 @@ hisi_acc_vfio_pci_get_device_state(struct vfio_device *vdev,
>
> static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev)
> {
> - struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(&pdev->dev);
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hssi_acc_drvdata(pdev);
>
> if (hisi_acc_vdev->core_device.vdev.migration_flags !=
> VFIO_MIGRATION_STOP_COPY)
> @@ -1274,11 +1282,10 @@ static int hisi_acc_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device
> &hisi_acc_vfio_pci_ops);
> }
>
> + dev_set_drvdata(&pdev->dev, &hisi_acc_vdev->core_device);
> ret = vfio_pci_core_register_device(&hisi_acc_vdev->core_device);
> if (ret)
> goto out_free;
> -
> - dev_set_drvdata(&pdev->dev, hisi_acc_vdev);
> return 0;
>
> out_free:
> @@ -1289,7 +1296,7 @@ static int hisi_acc_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device
>
> static void hisi_acc_vfio_pci_remove(struct pci_dev *pdev)
> {
> - struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(&pdev->dev);
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hssi_acc_drvdata(pdev);
>
> vfio_pci_core_unregister_device(&hisi_acc_vdev->core_device);
> vfio_pci_core_uninit_device(&hisi_acc_vdev->core_device);
> diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c
> index bbec5d288fee97..9f59f5807b8ab1 100644
> --- a/drivers/vfio/pci/mlx5/main.c
> +++ b/drivers/vfio/pci/mlx5/main.c
> @@ -39,6 +39,14 @@ struct mlx5vf_pci_core_device {
> struct mlx5_vf_migration_file *saving_migf;
> };
>
> +static struct mlx5vf_pci_core_device *mlx5vf_drvdata(struct pci_dev *pdev)
> +{
> + struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev);
> +
> + return container_of(core_device, struct mlx5vf_pci_core_device,
> + core_device);
> +}
> +
> static struct page *
> mlx5vf_get_migration_page(struct mlx5_vf_migration_file *migf,
> unsigned long offset)
> @@ -505,7 +513,7 @@ static int mlx5vf_pci_get_device_state(struct vfio_device *vdev,
>
> static void mlx5vf_pci_aer_reset_done(struct pci_dev *pdev)
> {
> - struct mlx5vf_pci_core_device *mvdev = dev_get_drvdata(&pdev->dev);
> + struct mlx5vf_pci_core_device *mvdev = mlx5vf_drvdata(pdev);
>
> if (!mvdev->migrate_cap)
> return;
> @@ -614,11 +622,10 @@ static int mlx5vf_pci_probe(struct pci_dev *pdev,
> }
> }
>
> + dev_set_drvdata(&pdev->dev, &mvdev->core_device);
> ret = vfio_pci_core_register_device(&mvdev->core_device);
> if (ret)
> goto out_free;
> -
> - dev_set_drvdata(&pdev->dev, mvdev);
> return 0;
>
> out_free:
> @@ -629,7 +636,7 @@ static int mlx5vf_pci_probe(struct pci_dev *pdev,
>
> static void mlx5vf_pci_remove(struct pci_dev *pdev)
> {
> - struct mlx5vf_pci_core_device *mvdev = dev_get_drvdata(&pdev->dev);
> + struct mlx5vf_pci_core_device *mvdev = mlx5vf_drvdata(pdev);
>
> vfio_pci_core_unregister_device(&mvdev->core_device);
> vfio_pci_core_uninit_device(&mvdev->core_device);
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 58839206d1ca7f..e34db35b8d61a1 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -151,10 +151,10 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> return -ENOMEM;
> vfio_pci_core_init_device(vdev, pdev, &vfio_pci_ops);
>
> + dev_set_drvdata(&pdev->dev, vdev);
> ret = vfio_pci_core_register_device(vdev);
> if (ret)
> goto out_free;
> - dev_set_drvdata(&pdev->dev, vdev);
> return 0;
>
> out_free:
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 06b6f3594a1316..65587fd5c021bb 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1821,6 +1821,10 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
> struct pci_dev *pdev = vdev->pdev;
> int ret;
>
> + /* Drivers must set the vfio_pci_core_device to their drvdata */
> + if (WARN_ON(vdev != dev_get_drvdata(&vdev->pdev->dev)))
> + return -EINVAL;
> +
> if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL)
> return -EINVAL;
>
Thanks Jason.
I have applied this patch locally and confirmed that my power
management changes works fine without my dev_set_drvdata() patch.
Regards,
Abhishek
