| Message ID | 20220901143747.32858-12-kevin.tian@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tidy up vfio_device life cycle | expand |
Hi kevin, On 9/1/22 16:37, Kevin Tian wrote: > Move vfio_device_ops from platform core to platform drivers so device > specific init/cleanup can be added. > > Introduce two new helpers vfio_platform_init/release_common() for the > use in driver @init/@release. > > vfio_platform_probe/remove_common() will be deprecated. > > Signed-off-by: Kevin Tian <kevin.tian@intel.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/vfio/platform/vfio_platform.c | 66 +++++++++++++++---- > drivers/vfio/platform/vfio_platform_common.c | 53 ++++++++++++--- > drivers/vfio/platform/vfio_platform_private.h | 15 +++++ > 3 files changed, 111 insertions(+), 23 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 04f40c5acfd6..82cedcebfd90 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -7,6 +7,7 @@ > #include <linux/module.h> > #include <linux/slab.h> > #include <linux/vfio.h> > +#include <linux/pm_runtime.h> > #include <linux/platform_device.h> > > #include "vfio_platform_private.h" > @@ -36,14 +37,11 @@ static int get_platform_irq(struct vfio_platform_device *vdev, int i) > return platform_get_irq_optional(pdev, i); > } > > -static int vfio_platform_probe(struct platform_device *pdev) > +static int vfio_platform_init_dev(struct vfio_device *core_vdev) > { > - struct vfio_platform_device *vdev; > - int ret; > - > - vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); > - if (!vdev) > - return -ENOMEM; > + struct vfio_platform_device *vdev = > + container_of(core_vdev, struct vfio_platform_device, vdev); > + struct platform_device *pdev = to_platform_device(core_vdev->dev); > > vdev->opaque = (void *) pdev; > vdev->name = pdev->name; > @@ -52,24 +50,64 @@ static int vfio_platform_probe(struct platform_device *pdev) > vdev->get_irq = get_platform_irq; > vdev->reset_required = reset_required; > > - ret = vfio_platform_probe_common(vdev, &pdev->dev); > - if (ret) { > - kfree(vdev); > - return ret; > - } > + return vfio_platform_init_common(vdev); > +} > + > +static const struct vfio_device_ops vfio_platform_ops; > +static int vfio_platform_probe(struct platform_device *pdev) > +{ > + struct vfio_platform_device *vdev; > + int ret; > + > + vdev = vfio_alloc_device(vfio_platform_device, vdev, &pdev->dev, > + &vfio_platform_ops); > + if (IS_ERR(vdev)) > + return PTR_ERR(vdev); > + > + ret = vfio_register_group_dev(&vdev->vdev); > + if (ret) > + goto out_put_vdev; > + > + pm_runtime_enable(&pdev->dev); > dev_set_drvdata(&pdev->dev, vdev); > return 0; > + > +out_put_vdev: > + vfio_put_device(&vdev->vdev); > + return ret; > +} > + > +static void vfio_platform_release_dev(struct vfio_device *core_vdev) > +{ > + struct vfio_platform_device *vdev = > + container_of(core_vdev, struct vfio_platform_device, vdev); > + > + vfio_platform_release_common(vdev); > + vfio_free_device(core_vdev); > } > > static int vfio_platform_remove(struct platform_device *pdev) > { > struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); > > - vfio_platform_remove_common(vdev); > - kfree(vdev); > + vfio_unregister_group_dev(&vdev->vdev); > + pm_runtime_disable(vdev->device); > + vfio_put_device(&vdev->vdev); > return 0; > } > > +static const struct vfio_device_ops vfio_platform_ops = { > + .name = "vfio-platform", > + .init = vfio_platform_init_dev, > + .release = vfio_platform_release_dev, > + .open_device = vfio_platform_open_device, > + .close_device = vfio_platform_close_device, > + .ioctl = vfio_platform_ioctl, > + .read = vfio_platform_read, > + .write = vfio_platform_write, > + .mmap = vfio_platform_mmap, > +}; > + > static struct platform_driver vfio_platform_driver = { > .probe = vfio_platform_probe, > .remove = vfio_platform_remove, > diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c > index 256f55b84e70..4c01bf0adebb 100644 > --- a/drivers/vfio/platform/vfio_platform_common.c > +++ b/drivers/vfio/platform/vfio_platform_common.c > @@ -218,7 +218,7 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev, > return -EINVAL; > } > > -static void vfio_platform_close_device(struct vfio_device *core_vdev) > +void vfio_platform_close_device(struct vfio_device *core_vdev) > { > struct vfio_platform_device *vdev = > container_of(core_vdev, struct vfio_platform_device, vdev); > @@ -236,8 +236,9 @@ static void vfio_platform_close_device(struct vfio_device *core_vdev) > vfio_platform_regions_cleanup(vdev); > vfio_platform_irq_cleanup(vdev); > } > +EXPORT_SYMBOL_GPL(vfio_platform_close_device); > > -static int vfio_platform_open_device(struct vfio_device *core_vdev) > +int vfio_platform_open_device(struct vfio_device *core_vdev) > { > struct vfio_platform_device *vdev = > container_of(core_vdev, struct vfio_platform_device, vdev); > @@ -273,9 +274,10 @@ static int vfio_platform_open_device(struct vfio_device *core_vdev) > vfio_platform_regions_cleanup(vdev); > return ret; > } > +EXPORT_SYMBOL_GPL(vfio_platform_open_device); > > -static long vfio_platform_ioctl(struct vfio_device *core_vdev, > - unsigned int cmd, unsigned long arg) > +long vfio_platform_ioctl(struct vfio_device *core_vdev, > + unsigned int cmd, unsigned long arg) > { > struct vfio_platform_device *vdev = > container_of(core_vdev, struct vfio_platform_device, vdev); > @@ -382,6 +384,7 @@ static long vfio_platform_ioctl(struct vfio_device *core_vdev, > > return -ENOTTY; > } > +EXPORT_SYMBOL_GPL(vfio_platform_ioctl); > > static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, > char __user *buf, size_t count, > @@ -438,8 +441,8 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, > return -EFAULT; > } > > -static ssize_t vfio_platform_read(struct vfio_device *core_vdev, > - char __user *buf, size_t count, loff_t *ppos) > +ssize_t vfio_platform_read(struct vfio_device *core_vdev, > + char __user *buf, size_t count, loff_t *ppos) > { > struct vfio_platform_device *vdev = > container_of(core_vdev, struct vfio_platform_device, vdev); > @@ -460,6 +463,7 @@ static ssize_t vfio_platform_read(struct vfio_device *core_vdev, > > return -EINVAL; > } > +EXPORT_SYMBOL_GPL(vfio_platform_read); > > static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, > const char __user *buf, size_t count, > @@ -515,8 +519,8 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, > return -EFAULT; > } > > -static ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __user *buf, > - size_t count, loff_t *ppos) > +ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __user *buf, > + size_t count, loff_t *ppos) > { > struct vfio_platform_device *vdev = > container_of(core_vdev, struct vfio_platform_device, vdev); > @@ -537,6 +541,7 @@ static ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __u > > return -EINVAL; > } > +EXPORT_SYMBOL_GPL(vfio_platform_write); > > static int vfio_platform_mmap_mmio(struct vfio_platform_region region, > struct vm_area_struct *vma) > @@ -558,7 +563,7 @@ static int vfio_platform_mmap_mmio(struct vfio_platform_region region, > req_len, vma->vm_page_prot); > } > > -static int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) > +int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) > { > struct vfio_platform_device *vdev = > container_of(core_vdev, struct vfio_platform_device, vdev); > @@ -598,6 +603,7 @@ static int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_stru > > return -EINVAL; > } > +EXPORT_SYMBOL_GPL(vfio_platform_mmap); > > static const struct vfio_device_ops vfio_platform_ops = { > .name = "vfio-platform", > @@ -639,6 +645,35 @@ static int vfio_platform_of_probe(struct vfio_platform_device *vdev, > * If the firmware is ACPI type, then acpi_disabled is 0. All other checks are > * valid checks. We cannot claim that this system is DT. > */ > +int vfio_platform_init_common(struct vfio_platform_device *vdev) > +{ > + int ret; > + struct device *dev = vdev->vdev.dev; > + > + ret = vfio_platform_acpi_probe(vdev, dev); > + if (ret) > + ret = vfio_platform_of_probe(vdev, dev); > + > + if (ret) > + return ret; > + > + vdev->device = dev; > + mutex_init(&vdev->igate); > + > + ret = vfio_platform_get_reset(vdev); > + if (ret && vdev->reset_required) > + dev_err(dev, "No reset function found for device %s\n", > + vdev->name); > + return ret; > +} > +EXPORT_SYMBOL_GPL(vfio_platform_init_common); > + > +void vfio_platform_release_common(struct vfio_platform_device *vdev) > +{ > + vfio_platform_put_reset(vdev); > +} > +EXPORT_SYMBOL_GPL(vfio_platform_release_common); > + > int vfio_platform_probe_common(struct vfio_platform_device *vdev, > struct device *dev) > { > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > index 691b43f4b2b2..a769d649fb97 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -81,6 +81,21 @@ struct vfio_platform_reset_node { > int vfio_platform_probe_common(struct vfio_platform_device *vdev, > struct device *dev); > void vfio_platform_remove_common(struct vfio_platform_device *vdev); > +int vfio_platform_init_common(struct vfio_platform_device *vdev); > +void vfio_platform_release_common(struct vfio_platform_device *vdev); > + > +int vfio_platform_open_device(struct vfio_device *core_vdev); > +void vfio_platform_close_device(struct vfio_device *core_vdev); > +long vfio_platform_ioctl(struct vfio_device *core_vdev, > + unsigned int cmd, unsigned long arg); > +ssize_t vfio_platform_read(struct vfio_device *core_vdev, > + char __user *buf, size_t count, > + loff_t *ppos); > +ssize_t vfio_platform_write(struct vfio_device *core_vdev, > + const char __user *buf, > + size_t count, loff_t *ppos); > +int vfio_platform_mmap(struct vfio_device *core_vdev, > + struct vm_area_struct *vma); > > int vfio_platform_irq_init(struct vfio_platform_device *vdev); > void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); Looks good to me. I also ran basic non regression testing Reviewed-by: Eric Auger <eric.auger@redhat.com> Tested-by: Eric Auger <eric.auger@redhat.com> Eric
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 04f40c5acfd6..82cedcebfd90 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -7,6 +7,7 @@ #include <linux/module.h> #include <linux/slab.h> #include <linux/vfio.h> +#include <linux/pm_runtime.h> #include <linux/platform_device.h> #include "vfio_platform_private.h" @@ -36,14 +37,11 @@ static int get_platform_irq(struct vfio_platform_device *vdev, int i) return platform_get_irq_optional(pdev, i); } -static int vfio_platform_probe(struct platform_device *pdev) +static int vfio_platform_init_dev(struct vfio_device *core_vdev) { - struct vfio_platform_device *vdev; - int ret; - - vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); - if (!vdev) - return -ENOMEM; + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); + struct platform_device *pdev = to_platform_device(core_vdev->dev); vdev->opaque = (void *) pdev; vdev->name = pdev->name; @@ -52,24 +50,64 @@ static int vfio_platform_probe(struct platform_device *pdev) vdev->get_irq = get_platform_irq; vdev->reset_required = reset_required; - ret = vfio_platform_probe_common(vdev, &pdev->dev); - if (ret) { - kfree(vdev); - return ret; - } + return vfio_platform_init_common(vdev); +} + +static const struct vfio_device_ops vfio_platform_ops; +static int vfio_platform_probe(struct platform_device *pdev) +{ + struct vfio_platform_device *vdev; + int ret; + + vdev = vfio_alloc_device(vfio_platform_device, vdev, &pdev->dev, + &vfio_platform_ops); + if (IS_ERR(vdev)) + return PTR_ERR(vdev); + + ret = vfio_register_group_dev(&vdev->vdev); + if (ret) + goto out_put_vdev; + + pm_runtime_enable(&pdev->dev); dev_set_drvdata(&pdev->dev, vdev); return 0; + +out_put_vdev: + vfio_put_device(&vdev->vdev); + return ret; +} + +static void vfio_platform_release_dev(struct vfio_device *core_vdev) +{ + struct vfio_platform_device *vdev = + container_of(core_vdev, struct vfio_platform_device, vdev); + + vfio_platform_release_common(vdev); + vfio_free_device(core_vdev); } static int vfio_platform_remove(struct platform_device *pdev) { struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); - vfio_platform_remove_common(vdev); - kfree(vdev); + vfio_unregister_group_dev(&vdev->vdev); + pm_runtime_disable(vdev->device); + vfio_put_device(&vdev->vdev); return 0; } +static const struct vfio_device_ops vfio_platform_ops = { + .name = "vfio-platform", + .init = vfio_platform_init_dev, + .release = vfio_platform_release_dev, + .open_device = vfio_platform_open_device, + .close_device = vfio_platform_close_device, + .ioctl = vfio_platform_ioctl, + .read = vfio_platform_read, + .write = vfio_platform_write, + .mmap = vfio_platform_mmap, +}; + static struct platform_driver vfio_platform_driver = { .probe = vfio_platform_probe, .remove = vfio_platform_remove, diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 256f55b84e70..4c01bf0adebb 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -218,7 +218,7 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev, return -EINVAL; } -static void vfio_platform_close_device(struct vfio_device *core_vdev) +void vfio_platform_close_device(struct vfio_device *core_vdev) { struct vfio_platform_device *vdev = container_of(core_vdev, struct vfio_platform_device, vdev); @@ -236,8 +236,9 @@ static void vfio_platform_close_device(struct vfio_device *core_vdev) vfio_platform_regions_cleanup(vdev); vfio_platform_irq_cleanup(vdev); } +EXPORT_SYMBOL_GPL(vfio_platform_close_device); -static int vfio_platform_open_device(struct vfio_device *core_vdev) +int vfio_platform_open_device(struct vfio_device *core_vdev) { struct vfio_platform_device *vdev = container_of(core_vdev, struct vfio_platform_device, vdev); @@ -273,9 +274,10 @@ static int vfio_platform_open_device(struct vfio_device *core_vdev) vfio_platform_regions_cleanup(vdev); return ret; } +EXPORT_SYMBOL_GPL(vfio_platform_open_device); -static long vfio_platform_ioctl(struct vfio_device *core_vdev, - unsigned int cmd, unsigned long arg) +long vfio_platform_ioctl(struct vfio_device *core_vdev, + unsigned int cmd, unsigned long arg) { struct vfio_platform_device *vdev = container_of(core_vdev, struct vfio_platform_device, vdev); @@ -382,6 +384,7 @@ static long vfio_platform_ioctl(struct vfio_device *core_vdev, return -ENOTTY; } +EXPORT_SYMBOL_GPL(vfio_platform_ioctl); static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, char __user *buf, size_t count, @@ -438,8 +441,8 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, return -EFAULT; } -static ssize_t vfio_platform_read(struct vfio_device *core_vdev, - char __user *buf, size_t count, loff_t *ppos) +ssize_t vfio_platform_read(struct vfio_device *core_vdev, + char __user *buf, size_t count, loff_t *ppos) { struct vfio_platform_device *vdev = container_of(core_vdev, struct vfio_platform_device, vdev); @@ -460,6 +463,7 @@ static ssize_t vfio_platform_read(struct vfio_device *core_vdev, return -EINVAL; } +EXPORT_SYMBOL_GPL(vfio_platform_read); static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, const char __user *buf, size_t count, @@ -515,8 +519,8 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, return -EFAULT; } -static ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __user *buf, - size_t count, loff_t *ppos) +ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __user *buf, + size_t count, loff_t *ppos) { struct vfio_platform_device *vdev = container_of(core_vdev, struct vfio_platform_device, vdev); @@ -537,6 +541,7 @@ static ssize_t vfio_platform_write(struct vfio_device *core_vdev, const char __u return -EINVAL; } +EXPORT_SYMBOL_GPL(vfio_platform_write); static int vfio_platform_mmap_mmio(struct vfio_platform_region region, struct vm_area_struct *vma) @@ -558,7 +563,7 @@ static int vfio_platform_mmap_mmio(struct vfio_platform_region region, req_len, vma->vm_page_prot); } -static int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) +int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma) { struct vfio_platform_device *vdev = container_of(core_vdev, struct vfio_platform_device, vdev); @@ -598,6 +603,7 @@ static int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_stru return -EINVAL; } +EXPORT_SYMBOL_GPL(vfio_platform_mmap); static const struct vfio_device_ops vfio_platform_ops = { .name = "vfio-platform", @@ -639,6 +645,35 @@ static int vfio_platform_of_probe(struct vfio_platform_device *vdev, * If the firmware is ACPI type, then acpi_disabled is 0. All other checks are * valid checks. We cannot claim that this system is DT. */ +int vfio_platform_init_common(struct vfio_platform_device *vdev) +{ + int ret; + struct device *dev = vdev->vdev.dev; + + ret = vfio_platform_acpi_probe(vdev, dev); + if (ret) + ret = vfio_platform_of_probe(vdev, dev); + + if (ret) + return ret; + + vdev->device = dev; + mutex_init(&vdev->igate); + + ret = vfio_platform_get_reset(vdev); + if (ret && vdev->reset_required) + dev_err(dev, "No reset function found for device %s\n", + vdev->name); + return ret; +} +EXPORT_SYMBOL_GPL(vfio_platform_init_common); + +void vfio_platform_release_common(struct vfio_platform_device *vdev) +{ + vfio_platform_put_reset(vdev); +} +EXPORT_SYMBOL_GPL(vfio_platform_release_common); + int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct device *dev) { diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 691b43f4b2b2..a769d649fb97 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -81,6 +81,21 @@ struct vfio_platform_reset_node { int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct device *dev); void vfio_platform_remove_common(struct vfio_platform_device *vdev); +int vfio_platform_init_common(struct vfio_platform_device *vdev); +void vfio_platform_release_common(struct vfio_platform_device *vdev); + +int vfio_platform_open_device(struct vfio_device *core_vdev); +void vfio_platform_close_device(struct vfio_device *core_vdev); +long vfio_platform_ioctl(struct vfio_device *core_vdev, + unsigned int cmd, unsigned long arg); +ssize_t vfio_platform_read(struct vfio_device *core_vdev, + char __user *buf, size_t count, + loff_t *ppos); +ssize_t vfio_platform_write(struct vfio_device *core_vdev, + const char __user *buf, + size_t count, loff_t *ppos); +int vfio_platform_mmap(struct vfio_device *core_vdev, + struct vm_area_struct *vma); int vfio_platform_irq_init(struct vfio_platform_device *vdev); void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev);