Hi Jason,
On 7/29/21 2:49 AM, Jason Gunthorpe wrote:
> Platform simply wants to run some code when the device is first
> opened/last closed. Use the core framework and locking for this. Aside
> from removing a bit of code this narrows the locking scope from a global
> lock.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxx>
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
Thanks
Eric
> ---
> drivers/vfio/platform/vfio_platform_common.c | 79 ++++++++-----------
> drivers/vfio/platform/vfio_platform_private.h | 1 -
> 2 files changed, 32 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index bdde8605178cd2..6af7ce7d619c25 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -218,65 +218,52 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev,
> return -EINVAL;
> }
>
> -static void vfio_platform_release(struct vfio_device *core_vdev)
> +static void vfio_platform_close_device(struct vfio_device *core_vdev)
> {
> struct vfio_platform_device *vdev =
> container_of(core_vdev, struct vfio_platform_device, vdev);
> + const char *extra_dbg = NULL;
> + int ret;
>
> - mutex_lock(&driver_lock);
> -
> - if (!(--vdev->refcnt)) {
> - const char *extra_dbg = NULL;
> - int ret;
> -
> - ret = vfio_platform_call_reset(vdev, &extra_dbg);
> - if (ret && vdev->reset_required) {
> - dev_warn(vdev->device, "reset driver is required and reset call failed in release (%d) %s\n",
> - ret, extra_dbg ? extra_dbg : "");
> - WARN_ON(1);
> - }
> - pm_runtime_put(vdev->device);
> - vfio_platform_regions_cleanup(vdev);
> - vfio_platform_irq_cleanup(vdev);
> + ret = vfio_platform_call_reset(vdev, &extra_dbg);
> + if (WARN_ON(ret && vdev->reset_required)) {
> + dev_warn(
> + vdev->device,
> + "reset driver is required and reset call failed in release (%d) %s\n",
> + ret, extra_dbg ? extra_dbg : "");
> }
> -
> - mutex_unlock(&driver_lock);
> + pm_runtime_put(vdev->device);
> + vfio_platform_regions_cleanup(vdev);
> + vfio_platform_irq_cleanup(vdev);
> }
>
> -static int vfio_platform_open(struct vfio_device *core_vdev)
> +static int vfio_platform_open_device(struct vfio_device *core_vdev)
> {
> struct vfio_platform_device *vdev =
> container_of(core_vdev, struct vfio_platform_device, vdev);
> + const char *extra_dbg = NULL;
> int ret;
>
> - mutex_lock(&driver_lock);
> -
> - if (!vdev->refcnt) {
> - const char *extra_dbg = NULL;
> -
> - ret = vfio_platform_regions_init(vdev);
> - if (ret)
> - goto err_reg;
> + ret = vfio_platform_regions_init(vdev);
> + if (ret)
> + return ret;
>
> - ret = vfio_platform_irq_init(vdev);
> - if (ret)
> - goto err_irq;
> + ret = vfio_platform_irq_init(vdev);
> + if (ret)
> + goto err_irq;
>
> - ret = pm_runtime_get_sync(vdev->device);
> - if (ret < 0)
> - goto err_rst;
> + ret = pm_runtime_get_sync(vdev->device);
> + if (ret < 0)
> + goto err_rst;
>
> - ret = vfio_platform_call_reset(vdev, &extra_dbg);
> - if (ret && vdev->reset_required) {
> - dev_warn(vdev->device, "reset driver is required and reset call failed in open (%d) %s\n",
> - ret, extra_dbg ? extra_dbg : "");
> - goto err_rst;
> - }
> + ret = vfio_platform_call_reset(vdev, &extra_dbg);
> + if (ret && vdev->reset_required) {
> + dev_warn(
> + vdev->device,
> + "reset driver is required and reset call failed in open (%d) %s\n",
> + ret, extra_dbg ? extra_dbg : "");
> + goto err_rst;
> }
> -
> - vdev->refcnt++;
> -
> - mutex_unlock(&driver_lock);
> return 0;
>
> err_rst:
> @@ -284,8 +271,6 @@ static int vfio_platform_open(struct vfio_device *core_vdev)
> vfio_platform_irq_cleanup(vdev);
> err_irq:
> vfio_platform_regions_cleanup(vdev);
> -err_reg:
> - mutex_unlock(&driver_lock);
> return ret;
> }
>
> @@ -616,8 +601,8 @@ static int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_stru
>
> static const struct vfio_device_ops vfio_platform_ops = {
> .name = "vfio-platform",
> - .open = vfio_platform_open,
> - .release = vfio_platform_release,
> + .open_device = vfio_platform_open_device,
> + .close_device = vfio_platform_close_device,
> .ioctl = vfio_platform_ioctl,
> .read = vfio_platform_read,
> .write = vfio_platform_write,
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index dfb834c1365946..520d2a8e8375b2 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -48,7 +48,6 @@ struct vfio_platform_device {
> u32 num_regions;
> struct vfio_platform_irq *irqs;
> u32 num_irqs;
> - int refcnt;
> struct mutex igate;
> const char *compat;
> const char *acpihid;
