| Message ID | 1-v4-c841817a0349+8f-vfio_get_from_dev_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove vfio_device_get_from_dev() | expand |
On Thu, 5 May 2022 20:21:39 -0300 Jason Gunthorpe <jgg@nvidia.com> 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. Stale comment, I'll s/vfio_pci_core_enable/vfio_pci_core_register_device/ on commit. Looks ok otherwise. Thanks, Alex > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > 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; >
On Fri, May 06, 2022 at 09:42:53AM -0600, Alex Williamson wrote: > On Thu, 5 May 2022 20:21:39 -0300 > Jason Gunthorpe <jgg@nvidia.com> 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. > > Stale comment, I'll > > s/vfio_pci_core_enable/vfio_pci_core_register_device/ > > on commit. Looks ok otherwise. Thanks, Yes, thanks Jason
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@nvidia.com> > --- > 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
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;
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@nvidia.com> --- 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(-)