| Message ID | 4-v2-20d933792272+4ff-vfio1_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Embed struct vfio_device in all sub-structures | expand |
On Fri, 12 Mar 2021 20:55:56 -0400 Jason Gunthorpe <jgg@nvidia.com> wrote: > platform already allocates a struct vfio_platform_device with exactly > the same lifetime as vfio_device, switch to the new API and embed > vfio_device in vfio_platform_device. > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/vfio/platform/vfio_amba.c | 8 ++++--- > drivers/vfio/platform/vfio_platform.c | 21 ++++++++--------- > drivers/vfio/platform/vfio_platform_common.c | 23 +++++++------------ > drivers/vfio/platform/vfio_platform_private.h | 5 ++-- > 4 files changed, 26 insertions(+), 31 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Fri, 12 Mar 2021 20:55:56 -0400 Jason Gunthorpe <jgg@nvidia.com> wrote: > platform already allocates a struct vfio_platform_device with exactly > the same lifetime as vfio_device, switch to the new API and embed > vfio_device in vfio_platform_device. > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/vfio/platform/vfio_amba.c | 8 ++++--- > drivers/vfio/platform/vfio_platform.c | 21 ++++++++--------- > drivers/vfio/platform/vfio_platform_common.c | 23 +++++++------------ > drivers/vfio/platform/vfio_platform_private.h | 5 ++-- > 4 files changed, 26 insertions(+), 31 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c > index 3626c21501017e..f970eb2a999f29 100644 > --- a/drivers/vfio/platform/vfio_amba.c > +++ b/drivers/vfio/platform/vfio_amba.c > @@ -66,16 +66,18 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id) > if (ret) { > kfree(vdev->name); > kfree(vdev); > + return ret; > } > > - return ret; > + dev_set_drvdata(&adev->dev, vdev); > + return 0; > } > > static void vfio_amba_remove(struct amba_device *adev) > { > - struct vfio_platform_device *vdev = > - vfio_platform_remove_common(&adev->dev); > + struct vfio_platform_device *vdev = dev_get_drvdata(&adev->dev); > > + vfio_platform_remove_common(vdev); > kfree(vdev->name); > kfree(vdev); > } > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 9fb6818cea12cb..f7b3f64ecc7f6c 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -54,23 +54,22 @@ static int vfio_platform_probe(struct platform_device *pdev) > vdev->reset_required = reset_required; > > ret = vfio_platform_probe_common(vdev, &pdev->dev); > - if (ret) > + if (ret) { > kfree(vdev); > - > - return ret; > + return ret; > + } > + dev_set_drvdata(&pdev->dev, vdev); > + return 0; > } > > static int vfio_platform_remove(struct platform_device *pdev) > { > - struct vfio_platform_device *vdev; > - > - vdev = vfio_platform_remove_common(&pdev->dev); > - if (vdev) { > - kfree(vdev); > - return 0; > - } > + struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); > > - return -EINVAL; > + vfio_platform_remove_common(vdev); > + kfree(vdev->name); We don't own that to free it, _probe set this via: vdev->name = pdev->name; Thanks, Alex
On Tue, Mar 16, 2021 at 03:33:55PM -0600, Alex Williamson wrote: > > static int vfio_platform_remove(struct platform_device *pdev) > > { > > - struct vfio_platform_device *vdev; > > - > > - vdev = vfio_platform_remove_common(&pdev->dev); > > - if (vdev) { > > - kfree(vdev); > > - return 0; > > - } > > + struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); > > > > - return -EINVAL; > > + vfio_platform_remove_common(vdev); > > + kfree(vdev->name); > > > We don't own that to free it, _probe set this via: > > vdev->name = pdev->name; Gah, yes, this is a copy&pasto mistake from the amba code Thanks, Jason
Hi, On 3/13/21 1:55 AM, Jason Gunthorpe wrote: > platform already allocates a struct vfio_platform_device with exactly > the same lifetime as vfio_device, switch to the new API and embed > vfio_device in vfio_platform_device. Without "kfree(vdev->name);" pointed out by Alex, Acked-by: Eric Auger <eric.auger@redhat.com> Tested-by: Eric Auger <eric.auger@redhat.com> Thanks Eric > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/vfio/platform/vfio_amba.c | 8 ++++--- > drivers/vfio/platform/vfio_platform.c | 21 ++++++++--------- > drivers/vfio/platform/vfio_platform_common.c | 23 +++++++------------ > drivers/vfio/platform/vfio_platform_private.h | 5 ++-- > 4 files changed, 26 insertions(+), 31 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c > index 3626c21501017e..f970eb2a999f29 100644 > --- a/drivers/vfio/platform/vfio_amba.c > +++ b/drivers/vfio/platform/vfio_amba.c > @@ -66,16 +66,18 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id) > if (ret) { > kfree(vdev->name); > kfree(vdev); > + return ret; > } > > - return ret; > + dev_set_drvdata(&adev->dev, vdev); > + return 0; > } > > static void vfio_amba_remove(struct amba_device *adev) > { > - struct vfio_platform_device *vdev = > - vfio_platform_remove_common(&adev->dev); > + struct vfio_platform_device *vdev = dev_get_drvdata(&adev->dev); > > + vfio_platform_remove_common(vdev); > kfree(vdev->name); > kfree(vdev); > } > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 9fb6818cea12cb..f7b3f64ecc7f6c 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -54,23 +54,22 @@ static int vfio_platform_probe(struct platform_device *pdev) > vdev->reset_required = reset_required; > > ret = vfio_platform_probe_common(vdev, &pdev->dev); > - if (ret) > + if (ret) { > kfree(vdev); > - > - return ret; > + return ret; > + } > + dev_set_drvdata(&pdev->dev, vdev); > + return 0; > } > > static int vfio_platform_remove(struct platform_device *pdev) > { > - struct vfio_platform_device *vdev; > - > - vdev = vfio_platform_remove_common(&pdev->dev); > - if (vdev) { > - kfree(vdev); > - return 0; > - } > + struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); > > - return -EINVAL; > + vfio_platform_remove_common(vdev); > + kfree(vdev->name); > + kfree(vdev); > + return 0; > } > > static struct platform_driver vfio_platform_driver = { > diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c > index fb4b385191f288..6eb749250ee41c 100644 > --- a/drivers/vfio/platform/vfio_platform_common.c > +++ b/drivers/vfio/platform/vfio_platform_common.c > @@ -659,8 +659,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, > struct iommu_group *group; > int ret; > > - if (!vdev) > - return -EINVAL; > + vfio_init_group_dev(&vdev->vdev, dev, &vfio_platform_ops, vdev); > > ret = vfio_platform_acpi_probe(vdev, dev); > if (ret) > @@ -685,13 +684,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, > goto put_reset; > } > > - ret = vfio_add_group_dev(dev, &vfio_platform_ops, vdev); > + ret = vfio_register_group_dev(&vdev->vdev); > if (ret) > goto put_iommu; > > mutex_init(&vdev->igate); > > - pm_runtime_enable(vdev->device); > + pm_runtime_enable(dev); > return 0; > > put_iommu: > @@ -702,19 +701,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, > } > EXPORT_SYMBOL_GPL(vfio_platform_probe_common); > > -struct vfio_platform_device *vfio_platform_remove_common(struct device *dev) > +void vfio_platform_remove_common(struct vfio_platform_device *vdev) > { > - struct vfio_platform_device *vdev; > - > - vdev = vfio_del_group_dev(dev); > + vfio_unregister_group_dev(&vdev->vdev); > > - if (vdev) { > - pm_runtime_disable(vdev->device); > - vfio_platform_put_reset(vdev); > - vfio_iommu_group_put(dev->iommu_group, dev); > - } > - > - return vdev; > + pm_runtime_disable(vdev->device); > + vfio_platform_put_reset(vdev); > + vfio_iommu_group_put(vdev->vdev.dev->iommu_group, vdev->vdev.dev); > } > EXPORT_SYMBOL_GPL(vfio_platform_remove_common); > > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > index 289089910643ac..a5ba82c8cbc354 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -9,6 +9,7 @@ > > #include <linux/types.h> > #include <linux/interrupt.h> > +#include <linux/vfio.h> > > #define VFIO_PLATFORM_OFFSET_SHIFT 40 > #define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1) > @@ -42,6 +43,7 @@ struct vfio_platform_region { > }; > > struct vfio_platform_device { > + struct vfio_device vdev; > struct vfio_platform_region *regions; > u32 num_regions; > struct vfio_platform_irq *irqs; > @@ -80,8 +82,7 @@ struct vfio_platform_reset_node { > > extern int vfio_platform_probe_common(struct vfio_platform_device *vdev, > struct device *dev); > -extern struct vfio_platform_device *vfio_platform_remove_common > - (struct device *dev); > +void vfio_platform_remove_common(struct vfio_platform_device *vdev); > > extern int vfio_platform_irq_init(struct vfio_platform_device *vdev); > extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); >
diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c index 3626c21501017e..f970eb2a999f29 100644 --- a/drivers/vfio/platform/vfio_amba.c +++ b/drivers/vfio/platform/vfio_amba.c @@ -66,16 +66,18 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id) if (ret) { kfree(vdev->name); kfree(vdev); + return ret; } - return ret; + dev_set_drvdata(&adev->dev, vdev); + return 0; } static void vfio_amba_remove(struct amba_device *adev) { - struct vfio_platform_device *vdev = - vfio_platform_remove_common(&adev->dev); + struct vfio_platform_device *vdev = dev_get_drvdata(&adev->dev); + vfio_platform_remove_common(vdev); kfree(vdev->name); kfree(vdev); } diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 9fb6818cea12cb..f7b3f64ecc7f6c 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -54,23 +54,22 @@ static int vfio_platform_probe(struct platform_device *pdev) vdev->reset_required = reset_required; ret = vfio_platform_probe_common(vdev, &pdev->dev); - if (ret) + if (ret) { kfree(vdev); - - return ret; + return ret; + } + dev_set_drvdata(&pdev->dev, vdev); + return 0; } static int vfio_platform_remove(struct platform_device *pdev) { - struct vfio_platform_device *vdev; - - vdev = vfio_platform_remove_common(&pdev->dev); - if (vdev) { - kfree(vdev); - return 0; - } + struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); - return -EINVAL; + vfio_platform_remove_common(vdev); + kfree(vdev->name); + kfree(vdev); + return 0; } static struct platform_driver vfio_platform_driver = { diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index fb4b385191f288..6eb749250ee41c 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -659,8 +659,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct iommu_group *group; int ret; - if (!vdev) - return -EINVAL; + vfio_init_group_dev(&vdev->vdev, dev, &vfio_platform_ops, vdev); ret = vfio_platform_acpi_probe(vdev, dev); if (ret) @@ -685,13 +684,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, goto put_reset; } - ret = vfio_add_group_dev(dev, &vfio_platform_ops, vdev); + ret = vfio_register_group_dev(&vdev->vdev); if (ret) goto put_iommu; mutex_init(&vdev->igate); - pm_runtime_enable(vdev->device); + pm_runtime_enable(dev); return 0; put_iommu: @@ -702,19 +701,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, } EXPORT_SYMBOL_GPL(vfio_platform_probe_common); -struct vfio_platform_device *vfio_platform_remove_common(struct device *dev) +void vfio_platform_remove_common(struct vfio_platform_device *vdev) { - struct vfio_platform_device *vdev; - - vdev = vfio_del_group_dev(dev); + vfio_unregister_group_dev(&vdev->vdev); - if (vdev) { - pm_runtime_disable(vdev->device); - vfio_platform_put_reset(vdev); - vfio_iommu_group_put(dev->iommu_group, dev); - } - - return vdev; + pm_runtime_disable(vdev->device); + vfio_platform_put_reset(vdev); + vfio_iommu_group_put(vdev->vdev.dev->iommu_group, vdev->vdev.dev); } EXPORT_SYMBOL_GPL(vfio_platform_remove_common); diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 289089910643ac..a5ba82c8cbc354 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -9,6 +9,7 @@ #include <linux/types.h> #include <linux/interrupt.h> +#include <linux/vfio.h> #define VFIO_PLATFORM_OFFSET_SHIFT 40 #define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1) @@ -42,6 +43,7 @@ struct vfio_platform_region { }; struct vfio_platform_device { + struct vfio_device vdev; struct vfio_platform_region *regions; u32 num_regions; struct vfio_platform_irq *irqs; @@ -80,8 +82,7 @@ struct vfio_platform_reset_node { extern int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct device *dev); -extern struct vfio_platform_device *vfio_platform_remove_common - (struct device *dev); +void vfio_platform_remove_common(struct vfio_platform_device *vdev); extern int vfio_platform_irq_init(struct vfio_platform_device *vdev); extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev);