vfio: platform: Fix using devices in PM Domains

Commit Message

Geert Uytterhoeven April 11, 2018, 9:15 a.m. UTC

If a device is part of a PM Domain (e.g. power and/or clock domain), its
power state is managed using Runtime PM.  Without Runtime PM, the device
may not be powered up, causing subtle failures, crashes, or system
lock-ups when the device is accessed by the guest.

Fix this by adding Runtime PM support, powering the device when the VFIO
device is opened by the guest.

Note that while more fine-grained power management could be implemented
on the guest side, if exported, this would be inherently unsafe, as
abusing it may kill the whole system.

Signed-off-by: Geert Uytterhoeven <[e-mail]>
---
 drivers/vfio/platform/vfio_platform_common.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Geert Uytterhoeven April 11, 2018, 9:15 a.m. UTC | #1

Hi all,

This patch series improves reset support for vfio-platform:
  - The first patch fixes a bug I ran into while working on this.
  - The second patch implements generic DT reset controller support, for
    devices that are connected to an SoC-internal reset controller and
    can be reset in a generic way.  This avoids having to write/change a
    vfio-specific reset driver for each and every device to be
    passed-through to a guest.

Changes compared to v2:
  - Add Reviewed-by,
  - Merge similar checks in vfio_platform_has_reset().

Changes compared to v1:
  - Add Reviewed-by,
  - Don't store error values in vdev->reset_control,
  - Use of_reset_control_get_exclusive() instead of
    __of_reset_control_get(),
  - Improve description.

This has been tested on R-Car H3/Salvator-XS by exporting a GPIO module
to a QEMU+KWM guest: the GPIO module is reset before QEMU opens the vfio
device, and reset again after QEMU has released it, as can be witnessed
by the LEDs connected to the GPIOs.

Thanks!

Geert Uytterhoeven (2):
  vfio: platform: Fix reset module leak in error path
  vfio: platform: Add generic DT reset support

 drivers/vfio/platform/vfio_platform_common.c  | 35 +++++++++++++++++++++------
 drivers/vfio/platform/vfio_platform_private.h |  1 +
 2 files changed, 29 insertions(+), 7 deletions(-)

Geert Uytterhoeven April 11, 2018, 9:21 a.m. UTC | #2

On Wed, Apr 11, 2018 at 11:15 AM, Geert Uytterhoeven
<[e-mail]> wrote:
> If a device is part of a PM Domain (e.g. power and/or clock domain), its
> power state is managed using Runtime PM.  Without Runtime PM, the device
> may not be powered up, causing subtle failures, crashes, or system
> lock-ups when the device is accessed by the guest.
>
> Fix this by adding Runtime PM support, powering the device when the VFIO
> device is opened by the guest.
>
> Note that while more fine-grained power management could be implemented
> on the guest side, if exported, this would be inherently unsafe, as
> abusing it may kill the whole system.
>
> Signed-off-by: Geert Uytterhoeven <[e-mail]>

Please ignore, this was sent accidentally with another series.

Gr{oetje,eeting}s,

                        Geert

Patch

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 3c13327b2777f8ec..9d46fce2446daebe 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -17,6 +17,7 @@ 
 #include <linux/iommu.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -255,6 +256,8 @@  static void vfio_platform_release(void *device_data)
 		const char *extra_dbg = NULL;
 		int ret;
 
+		pm_runtime_put(vdev->device);
+
 		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",
@@ -297,6 +300,10 @@  static int vfio_platform_open(void *device_data)
 				 ret, extra_dbg ? extra_dbg : "");
 			goto err_rst;
 		}
+
+		ret = pm_runtime_get_sync(vdev->device);
+		if (ret < 0)
+			goto err_rst;
 	}
 
 	vdev->refcnt++;
@@ -712,6 +719,7 @@  int vfio_platform_probe_common(struct vfio_platform_device *vdev,
 
 	mutex_init(&vdev->igate);
 
+	pm_runtime_enable(vdev->device);
 	return 0;
 
 put_iommu:
@@ -729,6 +737,7 @@  struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
 	vdev = vfio_del_group_dev(dev);
 
 	if (vdev) {
+		pm_runtime_disable(vdev->device);
 		vfio_platform_put_reset(vdev);
 		vfio_iommu_group_put(dev->iommu_group, dev);
 	}