From 3a528a458d4a2ba4236e98ef3f4efe5482323972 Mon Sep 17 00:00:00 2001

From: Peter Xu <peterx@redhat.com>

Date: Wed, 9 Oct 2019 12:39:44 +0100

Subject: [PATCH 18/22] intel_iommu: Sanity check vfio-pci config on machine

 init done

RH-Author: Peter Xu <peterx@redhat.com>

Message-id: <20191009123947.21505-3-peterx@redhat.com>

Patchwork-id: 91347

O-Subject: [RHEL-8.2.0 qemu-kvm PATCH 2/5] intel_iommu: Sanity check vfio-pci config on machine init done

Bugzilla: 1738440

RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

RH-Acked-by: Auger Eric <eric.auger@redhat.com>

RH-Acked-by: Alex Williamson <alex.williamson@redhat.com>

This check was previously only happened when the IOMMU is enabled in

the guest.  It was always too late because the enabling of IOMMU

normally only happens during the boot of guest OS.  It means that we

can bail out and exit directly during the guest OS boots if the

configuration of devices are not supported.  Or, if the guest didn't

enable vIOMMU at all, then the user can use the guest normally but as

long as it reconfigure the guest OS to enable the vIOMMU then reboot,

the user will see the panic right after the reset when the next boot

starts.

Let's make this failure even earlier so that we force the user to use

caching-mode for vfio-pci devices when with the vIOMMU.  So the user

won't get surprise at least during execution of the guest, which seems

a bit nicer.

This will affect some user who didn't enable vIOMMU in the guest OS

but was using vfio-pci and the vtd device in the past.  However I hope

it's not a majority because not enabling vIOMMU with the device

attached is actually meaningless.

We still keep the old assertion for safety so far because the hotplug

path could still reach it, so far.

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Signed-off-by: Peter Xu <peterx@redhat.com>

Message-Id: <20190916080718.3299-2-peterx@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

(cherry picked from commit 28cf553afeb29b0c4f339c600171552a72a68cb7)

Signed-off-by: Peter Xu <peterx@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

---

 hw/i386/intel_iommu.c | 39 ++++++++++++++++++++++++++++++++++++---

 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c

index 22d2e52..44d19cc 100644

--- a/hw/i386/intel_iommu.c

+++ b/hw/i386/intel_iommu.c

@@ -33,6 +33,7 @@

 #include "hw/i386/x86-iommu.h"

 #include "hw/pci-host/q35.h"

 #include "sysemu/kvm.h"

+#include "sysemu/sysemu.h"

 #include "hw/i386/apic_internal.h"

 #include "kvm_i386.h"

 #include "trace.h"

@@ -40,6 +41,13 @@

 static void vtd_address_space_refresh_all(IntelIOMMUState *s);

 static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n);

+static void vtd_panic_require_caching_mode(void)

+{

+    error_report("We need to set caching-mode=on for intel-iommu to enable "

+                 "device assignment with IOMMU protection.");

+    exit(1);

+}

+

 static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,

                             uint64_t wmask, uint64_t w1cmask)

 {

@@ -2554,9 +2562,7 @@ static void vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,

     IntelIOMMUState *s = vtd_as->iommu_state;

     if (!s->caching_mode && new & IOMMU_NOTIFIER_MAP) {

-        error_report("We need to set caching-mode=on for intel-iommu to enable "

-                     "device assignment with IOMMU protection.");

-        exit(1);

+        vtd_panic_require_caching_mode();

     }

     /* Update per-address-space notifier flags */

@@ -3303,6 +3309,32 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)

     return true;

 }

+static int vtd_machine_done_notify_one(Object *child, void *unused)

+{

+    IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());

+

+    /*

+     * We hard-coded here because vfio-pci is the only special case

+     * here.  Let's be more elegant in the future when we can, but so

+     * far there seems to be no better way.

+     */

+    if (object_dynamic_cast(child, "vfio-pci") && !iommu->caching_mode) {

+        vtd_panic_require_caching_mode();

+    }

+

+    return 0;

+}

+

+static void vtd_machine_done_hook(Notifier *notifier, void *unused)

+{

+    object_child_foreach_recursive(object_get_root(),

+                                   vtd_machine_done_notify_one, NULL);

+}

+

+static Notifier vtd_machine_done_notify = {

+    .notify = vtd_machine_done_hook,

+};

+

 static void vtd_realize(DeviceState *dev, Error **errp)

 {

     MachineState *ms = MACHINE(qdev_get_machine());

@@ -3333,6 +3365,7 @@ static void vtd_realize(DeviceState *dev, Error **errp)

     pci_setup_iommu(bus, vtd_host_dma_iommu, dev);

     /* Pseudo address space under root PCI bus. */

     pcms->ioapic_as = vtd_host_dma_iommu(bus, s, Q35_PSEUDO_DEVFN_IOAPIC);

+    qemu_add_machine_init_done_notifier(&vtd_machine_done_notify);

 }

 static void vtd_class_init(ObjectClass *klass, void *data)

-- 

1.8.3.1