[PATCH v2 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV

[Halil Pasic]

The virtio specification tells that the device is to present
VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the
device "can only access certain memory addresses with said access
specified and/or granted by the platform". This is the case for a
protected VMs, as the device can access only memory addresses that are
in pages that are currently shared (only the guest can share/unsare its
pages).

No VM, however, starts out as a protected VM, but some VMs may be
converted to protected VMs if the guest decides so.

Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via
the property iommu_on is a minor disaster. Since the correctness of the
paravirtualized virtio devices depends (and thus in a sense the
correctness of the hypervisor) it, then the hypervisor should have the
last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or
not.

Currently presenting a PV guest with a (paravirtualized) virtio-ccw
device has catastrophic consequences for the VM (after the hypervisors
access to protected memory). This is especially grave in case of device
hotplug (because in this case the guest is more likely to be in the
middle of something important).

Let us manage the VIRTIO_F_ACCESS_PLATFORM virtio feature automatically
for virtio-ccw devices, i.e. force it before we start the protected VM.
If the VM should cease to be protected, the original value is restored.

Signed-off-by: Halil Pasic <address@hidden>
---

NOTES:

* Doing more system_resets() is a big hack.  We should look into this.
* The user interface implications of this patch are also an ugly can of
worms. We need to discuss them.


v1 --> v2:
* Use the default or user supplied iommu_on flag when when !PV
* Use virtio functions for feature manipulation

Link to v1:
https://www.mail-archive.com/address@hidden/msg683775.html

Unfortunately the v1 did not see much discussion because we had more
pressing issues.

---
 hw/s390x/s390-virtio-ccw.c |  2 ++
 hw/s390x/virtio-ccw.c      | 14 ++++++++++++++
 hw/s390x/virtio-ccw.h      |  1 +
 3 files changed, 17 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index f660070d22..705e6b153a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -330,6 +330,7 @@ static void s390_machine_unprotect(S390CcwMachineState *ms)
     migrate_del_blocker(pv_mig_blocker);
     error_free_or_abort(&pv_mig_blocker);
     qemu_balloon_inhibit(false);
+    subsystem_reset();
 }
 
 static int s390_machine_protect(S390CcwMachineState *ms)
@@ -382,6 +383,7 @@ static int s390_machine_protect(S390CcwMachineState *ms)
     if (rc) {
         goto out_err;
     }
+    subsystem_reset();
     return rc;
 
 out_err:
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 64f928fc7d..67d5bc68ba 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -874,6 +874,20 @@ static void virtio_ccw_reset(DeviceState *d)
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
     VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
     VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
+    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());
+
+    /*
+     * An attempt to use a paravirt device without VIRTIO_F_IOMMU_PLATFORM
+     * in PV, has catastrophic consequences for the VM. Let's force
+     * VIRTIO_F_IOMMU_PLATFORM not already specified.
+     */
+    if (ms->pv && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
+        virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
+        dev->forced_iommu_platform = true;
+    } else if (!ms->pv && dev->forced_iommu_platform) {
+        virtio_clear_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
+        dev->forced_iommu_platform = false;
+    }
 
     virtio_ccw_reset_virtio(dev, vdev);
     if (vdc->parent_reset) {
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 3453aa1f98..34ff7b0b4e 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -99,6 +99,7 @@ struct VirtioCcwDevice {
     IndAddr *summary_indicator;
     uint64_t ind_bit;
     bool force_revision_1;
+    bool forced_iommu_platform;
 };
 
 /* The maximum virtio revision we support. */

base-commit: 0ffd3d64bd1bb8b84950e52159a0062fdab34628
-- 
2.17.1