diff mbox

[v6,22/26] x86-iommu: replace existing VT-d hooks into X86 ones

Message ID 1462418761-12714-23-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu May 5, 2016, 3:25 a.m. UTC
Previously, there are lots of VT-d hooks in common codes (like q35,
ioapic, etc.). A better way is to avoid using VT-d interfaces. Also, we
can start to abstract some common functions between Intel and future AMD
IOMMU device.

This patch cleaned up all the VT-d hooks into x86 ones, and moved IEC
notifier list from VT-d state to x86 state, so that this can be further
leveraged by AMD IOMMU codes.

Instead of searching in the global device tree every time, one static
variable is declared to store the default system x86 IOMMU device.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/acpi-build.c          |  4 +--
 hw/i386/intel_iommu.c         | 67 +++++++++++++------------------------
 hw/i386/x86-iommu.c           | 49 +++++++++++++++++++++++++++
 hw/intc/ioapic.c              |  6 ++--
 hw/pci-host/q35.c             | 11 +++----
 include/hw/i386/intel_iommu.h | 33 -------------------
 include/hw/i386/x86-iommu.h   | 77 ++++++++++++++++++++++++++++++++++++++++++-
 target-i386/kvm.c             |  7 ++--
 8 files changed, 160 insertions(+), 94 deletions(-)

Comments

Jan Kiszka May 5, 2016, 9:35 a.m. UTC | #1
On 2016-05-05 05:25, Peter Xu wrote:
> Previously, there are lots of VT-d hooks in common codes (like q35,
> ioapic, etc.). A better way is to avoid using VT-d interfaces. Also, we
> can start to abstract some common functions between Intel and future AMD
> IOMMU device.

Just don't introduce the VT-d variants in the first place. Will also
shorten your patch series again.

Jan
Peter Xu May 9, 2016, 5:23 a.m. UTC | #2
On Thu, May 05, 2016 at 11:35:43AM +0200, Jan Kiszka wrote:
> On 2016-05-05 05:25, Peter Xu wrote:
> > Previously, there are lots of VT-d hooks in common codes (like q35,
> > ioapic, etc.). A better way is to avoid using VT-d interfaces. Also, we
> > can start to abstract some common functions between Intel and future AMD
> > IOMMU device.
> 
> Just don't introduce the VT-d variants in the first place. Will also
> shorten your patch series again.

Yes, I can do that.

After waiting for some more review comments and making sure this is
the correct way to go, I can do the re-arrangement in next version.

Thanks,

-- peterx
diff mbox

Patch

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b064bc2..6cc686e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -51,7 +51,7 @@ 
 #include "hw/i386/ich9.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/q35.h"
-#include "hw/i386/intel_iommu.h"
+#include "hw/i386/x86-iommu.h"
 #include "hw/timer/hpet.h"
 
 #include "hw/acpi/aml-build.h"
@@ -2677,7 +2677,7 @@  static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
 
 static bool acpi_has_iommu(void)
 {
-    return !!vtd_iommu_get();
+    return !!x86_iommu_get_default();
 }
 
 static
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 51ad0b5..bee85e4 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -25,6 +25,7 @@ 
 #include "intel_iommu_internal.h"
 #include "hw/pci/pci.h"
 #include "hw/boards.h"
+#include "hw/i386/x86-iommu.h"
 
 /*#define DEBUG_INTEL_IOMMU*/
 #ifdef DEBUG_INTEL_IOMMU
@@ -191,7 +192,7 @@  static void vtd_reset_context_cache(IntelIOMMUState *s)
 
     VTD_DPRINTF(CACHE, "global context_cache_gen=1");
     while (g_hash_table_iter_next (&bus_it, NULL, (void**)&vtd_bus)) {
-        for (devfn_it = 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it) {
+        for (devfn_it = 0; devfn_it < X86_IOMMU_PCI_DEVFN_MAX; ++devfn_it) {
             vtd_as = vtd_bus->dev_as[devfn_it];
             if (!vtd_as) {
                 continue;
@@ -903,17 +904,7 @@  static void vtd_root_table_setup(IntelIOMMUState *s)
 static void vtd_iec_notify_all(IntelIOMMUState *s, bool global,
                                uint32_t index, uint32_t mask)
 {
-    VTD_IEC_Notifier *notifier;
-
-    VTD_DPRINTF(INV, "notify IEC invalidate: global=%d, index=%u, mask=%u",
-                global, index, mask);
-
-    QLIST_FOREACH(notifier, &s->iec_notifiers, list) {
-        if (notifier->iec_notify) {
-            notifier->iec_notify(notifier->private, global,
-                                 index, mask);
-        }
-    }
+    x86_iommu_iec_notify_all(X86_IOMMU_DEVICE(s), global, index, mask);
 }
 
 static void vtd_interrupt_remap_table_setup(IntelIOMMUState *s)
@@ -994,7 +985,7 @@  static void vtd_context_device_invalidate(IntelIOMMUState *s,
     vtd_bus = vtd_find_as_from_bus_num(s, VTD_SID_TO_BUS(source_id));
     if (vtd_bus) {
         devfn = VTD_SID_TO_DEVFN(source_id);
-        for (devfn_it = 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it) {
+        for (devfn_it = 0; devfn_it < X86_IOMMU_PCI_DEVFN_MAX; ++devfn_it) {
             vtd_as = vtd_bus->dev_as[devfn_it];
             if (vtd_as && ((devfn_it & mask) == (devfn & mask))) {
                 VTD_DPRINTF(INV, "invalidate context-cahce of devfn 0x%"PRIx16,
@@ -2037,7 +2028,7 @@  static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index,
         return -VTD_FR_IR_IRTE_RSVD;
     }
 
-    if (sid != VTD_SID_INVALID) {
+    if (sid != X86_IOMMU_SID_INVALID) {
         /* Validate IRTE SID */
         switch (entry->sid_vtype) {
         case VTD_SVT_NONE:
@@ -2219,10 +2210,11 @@  do_not_translate:
     return 0;
 }
 
-int vtd_int_remap(void *iommu, MSIMessage *src, MSIMessage *dst,
-                  uint16_t sid)
+static int vtd_int_remap(X86IOMMUState *iommu, MSIMessage *src,
+                         MSIMessage *dst, uint16_t sid)
 {
-    return vtd_interrupt_remap_msi(iommu, src, dst, sid);
+    return vtd_interrupt_remap_msi(INTEL_IOMMU_DEVICE(iommu),
+                                   src, dst, sid);
 }
 
 static MemTxResult vtd_mem_ir_read(void *opaque, hwaddr addr,
@@ -2248,7 +2240,7 @@  static MemTxResult vtd_mem_ir_write(void *opaque, hwaddr addr,
 {
     int ret = 0;
     MSIMessage from = {0}, to = {0};
-    uint16_t sid = VTD_SID_INVALID;
+    uint16_t sid = X86_IOMMU_SID_INVALID;
 
     from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
     from.data = (uint32_t) value;
@@ -2293,24 +2285,18 @@  static const MemoryRegionOps vtd_mem_ir_ops = {
     },
 };
 
-void vtd_iec_register_notifier(IntelIOMMUState *s, vtd_iec_notify_fn fn,
-                               void *data)
-{
-    VTD_IEC_Notifier *notifier = g_new0(VTD_IEC_Notifier, 1);
-    notifier->iec_notify = fn;
-    notifier->private = data;
-    QLIST_INSERT_HEAD(&s->iec_notifiers, notifier, list);
-}
-
-VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
+static AddressSpace *vtd_find_add_as(X86IOMMUState *x86_iommu, PCIBus *bus,
+                                     int devfn)
 {
+    IntelIOMMUState *s = (IntelIOMMUState *)x86_iommu;
     uintptr_t key = (uintptr_t)bus;
     VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
     VTDAddressSpace *vtd_dev_as;
 
     if (!vtd_bus) {
         /* No corresponding free() */
-        vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * VTD_PCI_DEVFN_MAX);
+        vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \
+                            X86_IOMMU_PCI_DEVFN_MAX);
         vtd_bus->bus = bus;
         key = (uintptr_t)bus;
         g_hash_table_insert(s->vtd_as_by_busptr, &key, vtd_bus);
@@ -2335,20 +2321,7 @@  VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
         address_space_init(&vtd_dev_as->as,
                            &vtd_dev_as->iommu, "intel_iommu");
     }
-    return vtd_dev_as;
-}
-
-IntelIOMMUState *vtd_iommu_get(void)
-{
-    bool ambiguous = false;
-    Object *intel_iommu = NULL;
-
-    intel_iommu = object_resolve_path_type("", TYPE_X86_IOMMU_DEVICE,
-                                 &ambiguous);
-    if (ambiguous)
-        intel_iommu = NULL;
-
-    return (IntelIOMMUState *)intel_iommu;
+    return &vtd_dev_as->as;
 }
 
 /* Do the initialization. It will also be called when reset, so pay
@@ -2452,6 +2425,7 @@  static void vtd_reset(DeviceState *dev)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
     IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
     VTD_DPRINTF(GENERAL, "");
     memset(s->vtd_as_by_bus_num, 0, sizeof(s->vtd_as_by_bus_num));
@@ -2463,18 +2437,21 @@  static void vtd_realize(DeviceState *dev, Error **errp)
                                      g_free, g_free);
     s->vtd_as_by_busptr = g_hash_table_new_full(vtd_uint64_hash, vtd_uint64_equal,
                                               g_free, g_free);
-    QLIST_INIT(&s->iec_notifiers);
     vtd_init(s);
+    x86_iommu_set_default(x86_iommu);
 }
 
 static void vtd_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    X86IOMMUClass *x86_class = X86_IOMMU_CLASS(klass);
 
     dc->reset = vtd_reset;
-    dc->realize = vtd_realize;
     dc->vmsd = &vtd_vmstate;
     dc->props = vtd_properties;
+    x86_class->realize = vtd_realize;
+    x86_class->find_add_as = vtd_find_add_as;
+    x86_class->int_remap = vtd_int_remap;
 }
 
 static const TypeInfo vtd_info = {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 7338f98..77da9c8 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -22,8 +22,56 @@ 
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
 
+/* Default X86 IOMMU device */
+static X86IOMMUState *x86_iommu_default = NULL;
+
+void x86_iommu_set_default(X86IOMMUState *x86_iommu)
+{
+    assert(x86_iommu);
+    assert(x86_iommu_default == NULL);
+    x86_iommu_default = x86_iommu;
+}
+
+X86IOMMUState *x86_iommu_get_default(void)
+{
+    return x86_iommu_default;
+}
+
+void x86_iommu_iec_register_notifier(X86IOMMUState *iommu,
+                                     iec_notify_fn fn, void *data)
+{
+    IEC_Notifier *notifier = g_new0(IEC_Notifier, 1);
+    notifier->iec_notify = fn;
+    notifier->private = data;
+    QLIST_INSERT_HEAD(&iommu->iec_notifiers, notifier, list);
+}
+
+void x86_iommu_iec_notify_all(X86IOMMUState *iommu, bool global,
+                              uint32_t index, uint32_t mask)
+{
+    IEC_Notifier *notifier;
+    QLIST_FOREACH(notifier, &iommu->iec_notifiers, list) {
+        if (notifier->iec_notify) {
+            notifier->iec_notify(notifier->private, global,
+                                 index, mask);
+        }
+    }
+}
+
+static void x86_iommu_realize(DeviceState *dev, Error **errp)
+{
+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
+    X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+    QLIST_INIT(&x86_iommu->iec_notifiers);
+    if (x86_class->realize) {
+        x86_class->realize(dev, errp);
+    }
+}
+
 static void x86_iommu_class_init(ObjectClass *klass, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->realize = x86_iommu_realize;
 }
 
 static const TypeInfo x86_iommu_info = {
@@ -31,6 +79,7 @@  static const TypeInfo x86_iommu_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(X86IOMMUState),
     .class_init    = x86_iommu_class_init,
+    .class_size    = sizeof(X86IOMMUClass),
     .abstract      = true,
 };
 
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index ce06954..e924100 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -30,7 +30,7 @@ 
 #include "sysemu/kvm.h"
 #include "target-i386/cpu.h"
 #include "hw/i386/apic-msidef.h"
-#include "hw/i386/intel_iommu.h"
+#include "hw/i386/x86-iommu.h"
 
 //#define DEBUG_IOAPIC
 
@@ -375,12 +375,12 @@  static void ioapic_realize(DeviceState *dev, Error **errp)
 
 #ifdef CONFIG_KVM
     if (kvm_irqchip_is_split()) {
-        IntelIOMMUState *iommu = vtd_iommu_get();
+        X86IOMMUState *iommu = x86_iommu_get_default();
         if (iommu) {
             /* Register this IOAPIC with IOMMU IEC notifier, so that
              * when there are IR invalidates, we can be notified to
              * update kernel IR cache. */
-            vtd_iec_register_notifier(iommu, ioapic_iec_notifier, s);
+            x86_iommu_iec_register_notifier(iommu, ioapic_iec_notifier, s);
         }
     }
 #endif
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index fe19eff..4aea9cf 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -426,13 +426,10 @@  static void mch_reset(DeviceState *qdev)
 
 static AddressSpace *q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
-    IntelIOMMUState *s = opaque;
-    VTDAddressSpace *vtd_as;
-
-    assert(0 <= devfn && devfn <= VTD_PCI_DEVFN_MAX);
-
-    vtd_as = vtd_find_add_as(s, bus, devfn);
-    return &vtd_as->as;
+    X86IOMMUState *x86_iommu = opaque;
+    X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(x86_iommu);
+    assert(0 <= devfn && devfn <= X86_IOMMU_PCI_DEVFN_MAX);
+    return x86_class->find_add_as(x86_iommu, bus, devfn);
 }
 
 static void mch_init_dmar(MCHPCIState *mch)
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 3f4a46e..18dafa0 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -38,7 +38,6 @@ 
 #define VTD_PCI_BUS_MAX             256
 #define VTD_PCI_SLOT_MAX            32
 #define VTD_PCI_FUNC_MAX            8
-#define VTD_PCI_DEVFN_MAX           256
 #define VTD_PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define VTD_PCI_FUNC(devfn)         ((devfn) & 0x07)
 #define VTD_SID_TO_BUS(sid)         (((sid) >> 8) & 0xff)
@@ -221,24 +220,6 @@  struct VTD_MSIMessage {
 /* When IR is enabled, all MSI/MSI-X data bits should be zero */
 #define VTD_IR_MSI_DATA          (0)
 
-/**
- * vtd_iec_notify_fn - IEC (Interrupt Entry Cache) notifier hook,
- *                     triggered when IR invalidation happens.
- * @private: private data
- * @global: whether this is a global IEC invalidation
- * @index: IRTE index to invalidate (start from)
- * @mask: invalidation mask
- */
-typedef void (*vtd_iec_notify_fn)(void *private, bool global,
-                                  uint32_t index, uint32_t mask);
-
-struct VTD_IEC_Notifier {
-    vtd_iec_notify_fn iec_notify;
-    void *private;
-    QLIST_ENTRY(VTD_IEC_Notifier) list;
-};
-typedef struct VTD_IEC_Notifier VTD_IEC_Notifier;
-
 /* The iommu (DMAR) device state struct */
 struct IntelIOMMUState {
     X86IOMMUState x86_iommu;
@@ -280,20 +261,6 @@  struct IntelIOMMUState {
     dma_addr_t intr_root;           /* Interrupt remapping table pointer */
     uint32_t intr_size;             /* Number of IR table entries */
     bool intr_eime;                 /* Extended interrupt mode enabled */
-    QLIST_HEAD(, VTD_IEC_Notifier) iec_notifiers; /* IEC notify list */
 };
 
-/* Find the VTD Address space associated with the given bus pointer,
- * create a new one if none exists
- */
-VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn);
-/* Get default IOMMU object */
-IntelIOMMUState *vtd_iommu_get(void);
-int vtd_int_remap(void *iommu, MSIMessage *src, MSIMessage *dst, uint16_t sid);
-/* Register IEC invalidate notifier */
-void vtd_iec_register_notifier(IntelIOMMUState *s, vtd_iec_notify_fn fn,
-                               void *data);
-
-#define  VTD_SID_INVALID  (0xffff)
-
 #endif
diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
index 3987755..5ccc070 100644
--- a/include/hw/i386/x86-iommu.h
+++ b/include/hw/i386/x86-iommu.h
@@ -21,15 +21,90 @@ 
 #define IOMMU_COMMON_H
 
 #include "hw/sysbus.h"
+#include "hw/pci/pci.h"
+#include "exec/memory.h"
 
 #define  TYPE_X86_IOMMU_DEVICE  ("x86-iommu")
+
 #define  X86_IOMMU_DEVICE(obj) \
-     OBJECT_CHECK(X86IOMMUState, (obj), TYPE_X86_IOMMU_DEVICE)
+    OBJECT_CHECK(X86IOMMUState, (obj), TYPE_X86_IOMMU_DEVICE)
+#define  X86_IOMMU_CLASS(klass) \
+    OBJECT_CLASS_CHECK(X86IOMMUClass, (klass), TYPE_X86_IOMMU_DEVICE)
+#define  X86_IOMMU_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(X86IOMMUClass, obj, TYPE_X86_IOMMU_DEVICE)
+
+#define X86_IOMMU_PCI_DEVFN_MAX           256
+#define X86_IOMMU_SID_INVALID             (0xffff)
 
 typedef struct X86IOMMUState X86IOMMUState;
+typedef struct X86IOMMUClass X86IOMMUClass;
+
+struct X86IOMMUClass {
+    SysBusDeviceClass parent;
+    /* Intel/AMD specific realize() hook */
+    DeviceRealize realize;
+    /* Find/Add IOMMU address space for specific PCI device */
+    AddressSpace *(*find_add_as)(X86IOMMUState *s, PCIBus *bus, int devfn);
+    /* MSI-based interrupt remapping */
+    int (*int_remap)(X86IOMMUState *iommu, MSIMessage *src,
+                     MSIMessage *dst, uint16_t sid);
+};
+
+/**
+ * iec_notify_fn - IEC (Interrupt Entry Cache) notifier hook,
+ *                 triggered when IR invalidation happens.
+ * @private: private data
+ * @global: whether this is a global IEC invalidation
+ * @index: IRTE index to invalidate (start from)
+ * @mask: invalidation mask
+ */
+typedef void (*iec_notify_fn)(void *private, bool global,
+                              uint32_t index, uint32_t mask);
+
+struct IEC_Notifier {
+    iec_notify_fn iec_notify;
+    void *private;
+    QLIST_ENTRY(IEC_Notifier) list;
+};
+typedef struct IEC_Notifier IEC_Notifier;
 
 struct X86IOMMUState {
     SysBusDevice busdev;
+    QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
 };
 
+/**
+ * x86_iommu_set_default - set default IOMMU device
+ * @x86_iommu: pointer to default device
+ * @return: (none)
+ */
+void x86_iommu_set_default(X86IOMMUState *x86_iommu);
+
+/**
+ * x86_iommu_get_default - get default IOMMU device
+ * @return: pointer to default IOMMU device
+ */
+X86IOMMUState *x86_iommu_get_default(void);
+
+/**
+ * x86_iommu_iec_register_notifier - register IEC (Interrupt Entry
+ *                                   Cache) notifiers
+ * @iommu: IOMMU device to register
+ * @fn: IEC notifier hook function
+ * @data: notifier private data
+ */
+void x86_iommu_iec_register_notifier(X86IOMMUState *iommu,
+                                     iec_notify_fn fn, void *data);
+
+/**
+ * x86_iommu_iec_notify_all - Notify IEC invalidations
+ * @iommu: IOMMU device that sends the notification
+ * @global: whether this is a global invalidation. If true, @index
+ *          and @mask are undefined.
+ * @index: starting index of interrupt entry to invalidate
+ * @mask: index mask for the invalidation
+ */
+void x86_iommu_iec_notify_all(X86IOMMUState *iommu, bool global,
+                              uint32_t index, uint32_t mask);
+
 #endif
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6f601cb..9cffc92 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -3329,19 +3329,20 @@  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
 int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
                              uint64_t address, uint32_t data, PCIDevice *dev)
 {
-    IntelIOMMUState *iommu = vtd_iommu_get();
+    X86IOMMUState *iommu = x86_iommu_get_default();
 
     if (iommu) {
         int ret;
         MSIMessage src, dst;
+        X86IOMMUClass *class = X86_IOMMU_GET_CLASS(iommu);
 
         src.address = route->u.msi.address_hi;
         src.address <<= VTD_MSI_ADDR_HI_SHIFT;
         src.address |= route->u.msi.address_lo;
         src.data = route->u.msi.data;
 
-        ret = vtd_int_remap(iommu, &src, &dst, dev ? pci_requester_id(dev) :
-                            VTD_SID_INVALID);
+        ret = class->int_remap(iommu, &src, &dst, dev ? \
+                               pci_requester_id(dev) : X86_IOMMU_SID_INVALID);
         if (ret) {
             trace_kvm_x86_fixup_msi_error(route->gsi);
             return 1;