qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/12] virtio-iommu: Add ACPI support
@ 2021-10-20 17:27 Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 01/12] hw/acpi: Add VIOT table Jean-Philippe Brucker
                   ` (11 more replies)
  0 siblings, 12 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

Allow instantiating a virtio-iommu device on ACPI systems by adding a
Virtual I/O Translation table (VIOT). Enable x86 support for VIOT.

Patches 5-8 need acks from Arm maintainers in order for this to go
through the x86 tree.

Changes since v4 [1]:
* New patch 2 removes unnecessary x86_iommu_get_type().
* Patch 3 now moves the IOMMU variable into PCMachineState, in addition
  to the check for duplicate IOMMUs.
* Fixed assertion error in patch 6.
* Addressed the other comments.
* Added more review and test tags, thanks!

The boot-bypass series [2] isn't a hard requirement for this series but
will be needed for complete support. In the meantime, storage accessed
by firmware needs to bypass the IOMMU some other way, using either a
virtio-blk device without the iommu_platform property, or a bypass
bridge (docs/iommu-bypass.txt).

You can find a description of the VIOT table, which will be included in
next ACPI version, here: https://jpbrucker.net/virtio-iommu/viot/viot-v9.pdf

[1] https://lore.kernel.org/qemu-devel/20211001173358.863017-1-jean-philippe@linaro.org/
[2] https://lore.kernel.org/qemu-devel/20210930185050.262759-1-jean-philippe@linaro.org/

Jean-Philippe Brucker (12):
  hw/acpi: Add VIOT table
  hw/i386/pc: Remove x86_iommu_get_type()
  hw/i386/pc: Move IOMMU singleton into PCMachineState
  hw/i386/pc: Allow instantiating a virtio-iommu device
  hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu
  hw/arm/virt: Remove device tree restriction for virtio-iommu
  hw/arm/virt: Reject instantiation of multiple IOMMUs
  hw/arm/virt: Use object_property_set instead of qdev_prop_set
  tests/acpi: allow updates of VIOT expected data files
  tests/acpi: add test cases for VIOT
  tests/acpi: add expected blob for VIOT test on virt machine
  tests/acpi: add expected blobs for VIOT test on q35 machine

 hw/acpi/viot.h                 |  13 ++++
 include/hw/i386/pc.h           |   1 +
 include/hw/i386/x86-iommu.h    |  12 ----
 hw/acpi/viot.c                 | 114 +++++++++++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c       |   7 ++
 hw/arm/virt.c                  |  20 +++---
 hw/i386/acpi-build.c           |  28 ++++----
 hw/i386/amd_iommu.c            |   2 -
 hw/i386/intel_iommu.c          |   3 -
 hw/i386/pc.c                   |  26 +++++++-
 hw/i386/x86-iommu-stub.c       |   5 --
 hw/i386/x86-iommu.c            |  31 +++------
 hw/virtio/virtio-iommu-pci.c   |  12 +---
 tests/qtest/bios-tables-test.c |  38 +++++++++++
 hw/acpi/Kconfig                |   4 ++
 hw/acpi/meson.build            |   1 +
 hw/arm/Kconfig                 |   1 +
 hw/i386/Kconfig                |   1 +
 tests/data/acpi/q35/DSDT.viot  | Bin 0 -> 9398 bytes
 tests/data/acpi/q35/VIOT.viot  | Bin 0 -> 112 bytes
 tests/data/acpi/virt/VIOT      | Bin 0 -> 88 bytes
 21 files changed, 242 insertions(+), 77 deletions(-)
 create mode 100644 hw/acpi/viot.h
 create mode 100644 hw/acpi/viot.c
 create mode 100644 tests/data/acpi/q35/DSDT.viot
 create mode 100644 tests/data/acpi/q35/VIOT.viot
 create mode 100644 tests/data/acpi/virt/VIOT

-- 
2.33.0



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 01/12] hw/acpi: Add VIOT table
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type() Jean-Philippe Brucker
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

Add a function that generates a Virtual I/O Translation table (VIOT),
describing the topology of paravirtual IOMMUs. The table is created if a
virtio-iommu device is present. It contains a virtio-iommu node and PCI
Range nodes for endpoints managed by the IOMMU. By default, a single
node describes all PCI devices. When passing the
"default_bus_bypass_iommu" machine option and "bypass_iommu" PXB option,
only buses that do not bypass the IOMMU are described by PCI Range
nodes.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/acpi/viot.h      |  13 +++++
 hw/acpi/viot.c      | 114 ++++++++++++++++++++++++++++++++++++++++++++
 hw/acpi/Kconfig     |   4 ++
 hw/acpi/meson.build |   1 +
 4 files changed, 132 insertions(+)
 create mode 100644 hw/acpi/viot.h
 create mode 100644 hw/acpi/viot.c

diff --git a/hw/acpi/viot.h b/hw/acpi/viot.h
new file mode 100644
index 0000000000..9fe565bb87
--- /dev/null
+++ b/hw/acpi/viot.h
@@ -0,0 +1,13 @@
+/*
+ * ACPI Virtual I/O Translation Table implementation
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef VIOT_H
+#define VIOT_H
+
+void build_viot(MachineState *ms, GArray *table_data, BIOSLinker *linker,
+                uint16_t virtio_iommu_bdf, const char *oem_id,
+                const char *oem_table_id);
+
+#endif /* VIOT_H */
diff --git a/hw/acpi/viot.c b/hw/acpi/viot.c
new file mode 100644
index 0000000000..c1af75206e
--- /dev/null
+++ b/hw/acpi/viot.c
@@ -0,0 +1,114 @@
+/*
+ * ACPI Virtual I/O Translation table implementation
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/viot.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
+
+struct viot_pci_ranges {
+    GArray *blob;
+    size_t count;
+    uint16_t output_node;
+};
+
+/* Build PCI range for a given PCI host bridge */
+static int build_pci_range_node(Object *obj, void *opaque)
+{
+    struct viot_pci_ranges *pci_ranges = opaque;
+    GArray *blob = pci_ranges->blob;
+
+    if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
+        PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
+
+        if (bus && !pci_bus_bypass_iommu(bus)) {
+            int min_bus, max_bus;
+
+            pci_bus_range(bus, &min_bus, &max_bus);
+
+            /* Type */
+            build_append_int_noprefix(blob, 1 /* PCI range */, 1);
+            /* Reserved */
+            build_append_int_noprefix(blob, 0, 1);
+            /* Length */
+            build_append_int_noprefix(blob, 24, 2);
+            /* Endpoint start */
+            build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 4);
+            /* PCI Segment start */
+            build_append_int_noprefix(blob, 0, 2);
+            /* PCI Segment end */
+            build_append_int_noprefix(blob, 0, 2);
+            /* PCI BDF start */
+            build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 2);
+            /* PCI BDF end */
+            build_append_int_noprefix(blob, PCI_BUILD_BDF(max_bus, 0xff), 2);
+            /* Output node */
+            build_append_int_noprefix(blob, pci_ranges->output_node, 2);
+            /* Reserved */
+            build_append_int_noprefix(blob, 0, 6);
+
+            pci_ranges->count++;
+        }
+    }
+
+    return 0;
+}
+
+/*
+ * Generate a VIOT table with one PCI-based virtio-iommu that manages PCI
+ * endpoints.
+ *
+ * Defined in the ACPI Specification (Version TBD)
+ */
+void build_viot(MachineState *ms, GArray *table_data, BIOSLinker *linker,
+                uint16_t virtio_iommu_bdf, const char *oem_id,
+                const char *oem_table_id)
+{
+    /* The virtio-iommu node follows the 48-bytes header */
+    int viommu_off = 48;
+    AcpiTable table = { .sig = "VIOT", .rev = 0,
+                        .oem_id = oem_id, .oem_table_id = oem_table_id };
+    struct viot_pci_ranges pci_ranges = {
+        .output_node = viommu_off,
+        .blob = g_array_new(false, true /* clear */, 1),
+    };
+
+    /* Build the list of PCI ranges that this viommu manages */
+    object_child_foreach_recursive(OBJECT(ms), build_pci_range_node,
+                                   &pci_ranges);
+
+    /* ACPI table header */
+    acpi_table_begin(&table, table_data);
+    /* Node count */
+    build_append_int_noprefix(table_data, pci_ranges.count + 1, 2);
+    /* Node offset */
+    build_append_int_noprefix(table_data, viommu_off, 2);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 8);
+
+    /* Virtio-iommu node */
+    /* Type */
+    build_append_int_noprefix(table_data, 3 /* virtio-pci IOMMU */, 1);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 1);
+    /* Length */
+    build_append_int_noprefix(table_data, 16, 2);
+    /* PCI Segment */
+    build_append_int_noprefix(table_data, 0, 2);
+    /* PCI BDF number */
+    build_append_int_noprefix(table_data, virtio_iommu_bdf, 2);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 8);
+
+    /* PCI ranges found above */
+    g_array_append_vals(table_data, pci_ranges.blob->data,
+                        pci_ranges.blob->len);
+    g_array_free(pci_ranges.blob, true);
+
+    acpi_table_end(linker, &table);
+}
+
diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index 3b5e118c54..622b0b50b7 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -51,6 +51,10 @@ config ACPI_VMGENID
     default y
     depends on PC
 
+config ACPI_VIOT
+    bool
+    depends on ACPI
+
 config ACPI_HW_REDUCED
     bool
     select ACPI
diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index 7d8c0eb43e..adf6347bc4 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -20,6 +20,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), if_false: files(
 acpi_ss.add(when: 'CONFIG_ACPI_PIIX4', if_true: files('piix4.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
+acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
 acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
 acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type()
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 01/12] hw/acpi: Add VIOT table Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
  2021-10-21 13:29   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState Jean-Philippe Brucker
                   ` (9 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

To generate the IOMMU ACPI table, acpi-build.c can use base QEMU types
instead of a special IommuType value.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/hw/i386/x86-iommu.h | 12 ------------
 hw/i386/acpi-build.c        | 20 +++++++++-----------
 hw/i386/amd_iommu.c         |  2 --
 hw/i386/intel_iommu.c       |  3 ---
 hw/i386/x86-iommu-stub.c    |  5 -----
 hw/i386/x86-iommu.c         |  5 -----
 6 files changed, 9 insertions(+), 38 deletions(-)

diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
index 9de92d33a1..5ba0c056d6 100644
--- a/include/hw/i386/x86-iommu.h
+++ b/include/hw/i386/x86-iommu.h
@@ -33,12 +33,6 @@ OBJECT_DECLARE_TYPE(X86IOMMUState, X86IOMMUClass, X86_IOMMU_DEVICE)
 typedef struct X86IOMMUIrq X86IOMMUIrq;
 typedef struct X86IOMMU_MSIMessage X86IOMMU_MSIMessage;
 
-typedef enum IommuType {
-    TYPE_INTEL,
-    TYPE_AMD,
-    TYPE_NONE
-} IommuType;
-
 struct X86IOMMUClass {
     SysBusDeviceClass parent;
     /* Intel/AMD specific realize() hook */
@@ -71,7 +65,6 @@ struct X86IOMMUState {
     OnOffAuto intr_supported;   /* Whether vIOMMU supports IR */
     bool dt_supported;          /* Whether vIOMMU supports DT */
     bool pt_supported;          /* Whether vIOMMU supports pass-through */
-    IommuType type;             /* IOMMU type - AMD/Intel     */
     QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
 };
 
@@ -140,11 +133,6 @@ struct X86IOMMU_MSIMessage {
  */
 X86IOMMUState *x86_iommu_get_default(void);
 
-/*
- * x86_iommu_get_type - get IOMMU type
- */
-IommuType x86_iommu_get_type(void);
-
 /**
  * x86_iommu_iec_register_notifier - register IEC (Interrupt Entry
  *                                   Cache) notifiers
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 81418b7911..ab49e799ff 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2488,6 +2488,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     X86MachineState *x86ms = X86_MACHINE(machine);
+    X86IOMMUState *iommu = x86_iommu_get_default();
     GArray *table_offsets;
     unsigned facs, dsdt, rsdt, fadt;
     AcpiPmInfo pm;
@@ -2604,17 +2605,14 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         build_mcfg(tables_blob, tables->linker, &mcfg, x86ms->oem_id,
                    x86ms->oem_table_id);
     }
-    if (x86_iommu_get_default()) {
-        IommuType IOMMUType = x86_iommu_get_type();
-        if (IOMMUType == TYPE_AMD) {
-            acpi_add_table(table_offsets, tables_blob);
-            build_amd_iommu(tables_blob, tables->linker, x86ms->oem_id,
-                            x86ms->oem_table_id);
-        } else if (IOMMUType == TYPE_INTEL) {
-            acpi_add_table(table_offsets, tables_blob);
-            build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
-                           x86ms->oem_table_id);
-        }
+    if (object_dynamic_cast(OBJECT(iommu), TYPE_AMD_IOMMU_DEVICE)) {
+        acpi_add_table(table_offsets, tables_blob);
+        build_amd_iommu(tables_blob, tables->linker, x86ms->oem_id,
+                        x86ms->oem_table_id);
+    } else if (object_dynamic_cast(OBJECT(iommu), TYPE_INTEL_IOMMU_DEVICE)) {
+        acpi_add_table(table_offsets, tables_blob);
+        build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
+                       x86ms->oem_table_id);
     }
     if (machine->nvdimms_state->is_enabled) {
         nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 9242a0d3ed..91fe34ae58 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1538,7 +1538,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
 {
     int ret = 0;
     AMDVIState *s = AMD_IOMMU_DEVICE(dev);
-    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
     MachineState *ms = MACHINE(qdev_get_machine());
     PCMachineState *pcms = PC_MACHINE(ms);
     X86MachineState *x86ms = X86_MACHINE(ms);
@@ -1548,7 +1547,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
                                      amdvi_uint64_equal, g_free, g_free);
 
     /* This device should take care of IOMMU PCI properties */
-    x86_iommu->type = TYPE_AMD;
     if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) {
         return;
     }
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 75f075547f..c27b20090e 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3806,9 +3806,6 @@ static void vtd_realize(DeviceState *dev, Error **errp)
     X86MachineState *x86ms = X86_MACHINE(ms);
     PCIBus *bus = pcms->bus;
     IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
-    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
-
-    x86_iommu->type = TYPE_INTEL;
 
     if (!vtd_decide_config(s, errp)) {
         return;
diff --git a/hw/i386/x86-iommu-stub.c b/hw/i386/x86-iommu-stub.c
index c5ba077f9d..781b5ff922 100644
--- a/hw/i386/x86-iommu-stub.c
+++ b/hw/i386/x86-iommu-stub.c
@@ -36,8 +36,3 @@ bool x86_iommu_ir_supported(X86IOMMUState *s)
 {
     return false;
 }
-
-IommuType x86_iommu_get_type(void)
-{
-    abort();
-}
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 86ad03972e..dc968c7a53 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -98,11 +98,6 @@ X86IOMMUState *x86_iommu_get_default(void)
     return x86_iommu_default;
 }
 
-IommuType x86_iommu_get_type(void)
-{
-    return x86_iommu_default->type;
-}
-
 static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
     X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 01/12] hw/acpi: Add VIOT table Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type() Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
  2021-10-21 13:34   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

We're about to support a third vIOMMU for x86, virtio-iommu which
doesn't inherit X86IOMMUState. Move the IOMMU singleton into
PCMachineState, so it can be shared between all three vIOMMUs.

The x86_iommu_get_default() helper is still needed by KVM and IOAPIC to
fetch the default IRQ-remapping IOMMU. Since virtio-iommu doesn't
support IRQ remapping, this interface doesn't need to change for the
moment. We could later replace X86IOMMUState with an "IRQ remapping
IOMMU" interface if necessary.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/hw/i386/pc.h |  1 +
 hw/i386/pc.c         | 12 +++++++++++-
 hw/i386/x86-iommu.c  | 26 ++++++++------------------
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 11426e26dc..b72e5bf9d1 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -35,6 +35,7 @@ typedef struct PCMachineState {
     I2CBus *smbus;
     PFlashCFI01 *flash[2];
     ISADevice *pcspk;
+    DeviceState *iommu;
 
     /* Configuration options: */
     uint64_t max_ram_below_4g;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 54e4c00dce..fcbf328e8d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1330,6 +1330,15 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
                object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
         pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
+        PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+
+        if (pcms->iommu) {
+            error_setg(errp, "QEMU does not support multiple vIOMMUs "
+                       "for x86 yet.");
+            return;
+        }
+        pcms->iommu = dev;
     }
 }
 
@@ -1384,7 +1393,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
         object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
-        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
         return HOTPLUG_HANDLER(machine);
     }
 
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index dc968c7a53..01d11325a6 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -77,25 +77,17 @@ void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *msg_out)
     msg_out->data = msg.msi_data;
 }
 
-/* Default X86 IOMMU device */
-static X86IOMMUState *x86_iommu_default = NULL;
-
-static void x86_iommu_set_default(X86IOMMUState *x86_iommu)
+X86IOMMUState *x86_iommu_get_default(void)
 {
-    assert(x86_iommu);
+    MachineState *ms = MACHINE(qdev_get_machine());
+    PCMachineState *pcms =
+        PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 
-    if (x86_iommu_default) {
-        error_report("QEMU does not support multiple vIOMMUs "
-                     "for x86 yet.");
-        exit(1);
+    if (pcms &&
+        object_dynamic_cast(OBJECT(pcms->iommu), TYPE_X86_IOMMU_DEVICE)) {
+        return X86_IOMMU_DEVICE(pcms->iommu);
     }
-
-    x86_iommu_default = x86_iommu;
-}
-
-X86IOMMUState *x86_iommu_get_default(void)
-{
-    return x86_iommu_default;
+    return NULL;
 }
 
 static void x86_iommu_realize(DeviceState *dev, Error **errp)
@@ -131,8 +123,6 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
     if (x86_class->realize) {
         x86_class->realize(dev, errp);
     }
-
-    x86_iommu_set_default(X86_IOMMU_DEVICE(dev));
 }
 
 static Property x86_iommu_properties[] = {
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (2 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21 13:47   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 05/12] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu Jean-Philippe Brucker
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

Allow instantiating a virtio-iommu device by adding an ACPI Virtual I/O
Translation table (VIOT), which describes the relation between the
virtio-iommu and the endpoints it manages.

Add a hotplug handler for virtio-iommu on x86 and set the necessary
reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
region is reserved for MSIs. DMA transactions to this range either
trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.

Although virtio-iommu does not support IRQ remapping it must be informed
of the reserved region so that it can forward DMA transactions targeting
this region.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/i386/acpi-build.c | 10 +++++++++-
 hw/i386/pc.c         | 16 +++++++++++++++-
 hw/i386/Kconfig      |  1 +
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ab49e799ff..3ca6cc8118 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -68,9 +68,11 @@
 #include "qom/qom-qobject.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #include "hw/acpi/ipmi.h"
 #include "hw/acpi/hmat.h"
+#include "hw/acpi/viot.h"
 
 /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
  * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
@@ -2488,7 +2490,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     X86MachineState *x86ms = X86_MACHINE(machine);
-    X86IOMMUState *iommu = x86_iommu_get_default();
+    DeviceState *iommu = pcms->iommu;
     GArray *table_offsets;
     unsigned facs, dsdt, rsdt, fadt;
     AcpiPmInfo pm;
@@ -2613,6 +2615,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         acpi_add_table(table_offsets, tables_blob);
         build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
                        x86ms->oem_table_id);
+    } else if (object_dynamic_cast(OBJECT(iommu), TYPE_VIRTIO_IOMMU_PCI)) {
+        PCIDevice *pdev = PCI_DEVICE(iommu);
+
+        acpi_add_table(table_offsets, tables_blob);
+        build_viot(machine, tables_blob, tables->linker, pci_get_bdf(pdev),
+                   x86ms->oem_id, x86ms->oem_table_id);
     }
     if (machine->nvdimms_state->is_enabled) {
         nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fcbf328e8d..f47f7866c7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -83,6 +83,7 @@
 #include "hw/i386/intel_iommu.h"
 #include "hw/net/ne2000-isa.h"
 #include "standard-headers/asm-x86/bootparam.h"
+#include "hw/virtio/virtio-iommu.h"
 #include "hw/virtio/virtio-pmem-pci.h"
 #include "hw/virtio/virtio-mem-pci.h"
 #include "hw/mem/memory-device.h"
@@ -1330,7 +1331,19 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
                object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
         pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
-    } else if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        /* Declare the APIC range as the reserved MSI region */
+        char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
+                                              VIRTIO_IOMMU_RESV_MEM_T_MSI);
+
+        object_property_set_uint(OBJECT(dev), "len-reserved-regions", 1, errp);
+        object_property_set_str(OBJECT(dev), "reserved-regions[0]",
+                                resv_prop_str, errp);
+        g_free(resv_prop_str);
+    }
+
+    if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
         PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
         if (pcms->iommu) {
@@ -1394,6 +1407,7 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
         object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
         object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
         return HOTPLUG_HANDLER(machine);
     }
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 962d2c981b..d22ac4a4b9 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -59,6 +59,7 @@ config PC_ACPI
     select ACPI_X86
     select ACPI_CPU_HOTPLUG
     select ACPI_MEMORY_HOTPLUG
+    select ACPI_VIOT
     select SMBUS_EEPROM
     select PFLASH_CFI01
     depends on ACPI_SMBUS
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 05/12] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (3 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21 13:49   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 06/12] hw/arm/virt: Remove device tree restriction " Jean-Philippe Brucker
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

When a virtio-iommu is instantiated, describe it using the ACPI VIOT
table.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/virt-acpi-build.c | 7 +++++++
 hw/arm/Kconfig           | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6cec97352b..e26639e1e1 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -55,6 +55,7 @@
 #include "kvm_arm.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/ghes.h"
+#include "hw/acpi/viot.h"
 
 #define ARM_SPI_BASE 32
 
@@ -934,6 +935,12 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     }
 #endif
 
+    if (vms->iommu == VIRT_IOMMU_VIRTIO) {
+        acpi_add_table(table_offsets, tables_blob);
+        build_viot(ms, tables_blob, tables->linker, vms->virtio_iommu_bdf,
+                   vms->oem_id, vms->oem_table_id);
+    }
+
     /* XSDT is pointed to by RSDP */
     xsdt = tables_blob->len;
     build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 2d37d29f02..e652590943 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -27,6 +27,7 @@ config ARM_VIRT
     select DIMM
     select ACPI_HW_REDUCED
     select ACPI_APEI
+    select ACPI_VIOT
 
 config CHEETAH
     bool
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 06/12] hw/arm/virt: Remove device tree restriction for virtio-iommu
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (4 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 05/12] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21 13:53   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 07/12] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

virtio-iommu is now supported with ACPI VIOT as well as device tree.
Remove the restriction that prevents from instantiating a virtio-iommu
device under ACPI.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/virt.c                | 10 ++--------
 hw/virtio/virtio-iommu-pci.c | 12 ++----------
 2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f7f456bf58..3da7a86e37 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2561,16 +2561,10 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
     if (device_is_dynamic_sysbus(mc, dev) ||
-       (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
         return HOTPLUG_HANDLER(machine);
     }
-    if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
-        VirtMachineState *vms = VIRT_MACHINE(machine);
-
-        if (!vms->bootinfo.firmware_loaded || !virt_is_acpi_enabled(vms)) {
-            return HOTPLUG_HANDLER(machine);
-        }
-    }
     return NULL;
 }
 
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 770c286be7..5c0b87316a 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -48,16 +48,8 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
 
     if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) {
-        MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
-
-        error_setg(errp,
-                   "%s machine fails to create iommu-map device tree bindings",
-                   mc->name);
-        error_append_hint(errp,
-                          "Check your machine implements a hotplug handler "
-                          "for the virtio-iommu-pci device\n");
-        error_append_hint(errp, "Check the guest is booted without FW or with "
-                          "-no-acpi\n");
+        error_setg(errp, "Check your machine implements a hotplug handler "
+                         "for the virtio-iommu-pci device");
         return;
     }
     for (int i = 0; i < s->nb_reserved_regions; i++) {
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 07/12] hw/arm/virt: Reject instantiation of multiple IOMMUs
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (5 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 06/12] hw/arm/virt: Remove device tree restriction " Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21 13:54   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 08/12] hw/arm/virt: Use object_property_set instead of qdev_prop_set Jean-Philippe Brucker
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

We do not support instantiating multiple IOMMUs. Before adding a
virtio-iommu, check that no other IOMMU is present. This will detect
both "iommu=smmuv3" machine parameter and another virtio-iommu instance.

Fixes: 70e89132c9 ("hw/arm/virt: Add the virtio-iommu device tree mappings")
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/virt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3da7a86e37..25db8d4262 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2441,6 +2441,11 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
         hwaddr db_start = 0, db_end = 0;
         char *resv_prop_str;
 
+        if (vms->iommu != VIRT_IOMMU_NONE) {
+            error_setg(errp, "virt machine does not support multiple IOMMUs");
+            return;
+        }
+
         switch (vms->msi_controller) {
         case VIRT_MSI_CTRL_NONE:
             return;
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 08/12] hw/arm/virt: Use object_property_set instead of qdev_prop_set
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (6 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 07/12] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

To propagate errors to the caller of the pre_plug callback, use the
object_poperty_set*() functions directly instead of the qdev_prop_set*()
helpers.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/virt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 25db8d4262..7a061ab401 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2465,8 +2465,9 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                                         db_start, db_end,
                                         VIRTIO_IOMMU_RESV_MEM_T_MSI);
 
-        qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
-        qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
+        object_property_set_uint(OBJECT(dev), "len-reserved-regions", 1, errp);
+        object_property_set_str(OBJECT(dev), "reserved-regions[0]",
+                                resv_prop_str, errp);
         g_free(resv_prop_str);
     }
 }
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (7 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 08/12] hw/arm/virt: Use object_property_set instead of qdev_prop_set Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
  2021-10-21 14:00   ` Igor Mammedov
  2021-10-20 17:27 ` [PATCH v5 10/12] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

Create empty data files and allow updates for the upcoming VIOT tests.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
 tests/data/acpi/q35/DSDT.viot               | 0
 tests/data/acpi/q35/VIOT.viot               | 0
 tests/data/acpi/virt/VIOT                   | 0
 4 files changed, 3 insertions(+)
 create mode 100644 tests/data/acpi/q35/DSDT.viot
 create mode 100644 tests/data/acpi/q35/VIOT.viot
 create mode 100644 tests/data/acpi/virt/VIOT

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..29b5b1eabc 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,4 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/VIOT",
+"tests/data/acpi/q35/DSDT.viot",
+"tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT
new file mode 100644
index 0000000000..e69de29bb2
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 10/12] tests/acpi: add test cases for VIOT
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (8 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
  2021-10-21  9:02   ` Eric Auger
  2021-10-20 17:27 ` [PATCH v5 11/12] tests/acpi: add expected blob for VIOT test on virt machine Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 12/12] tests/acpi: add expected blobs for VIOT test on q35 machine Jean-Philippe Brucker
  11 siblings, 2 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

Add two test cases for VIOT, one on the q35 machine and the other on
virt. To test complex topologies the q35 test has two PCIe buses that
bypass the IOMMU (and are therefore not described by VIOT), and two
buses that are translated by virtio-iommu.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 4f11d03055..599b155201 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1403,6 +1403,42 @@ static void test_acpi_virt_tcg(void)
     free_test_data(&data);
 }
 
+static void test_acpi_q35_viot(void)
+{
+    test_data data = {
+        .machine = MACHINE_Q35,
+        .variant = ".viot",
+    };
+
+    /*
+     * To keep things interesting, two buses bypass the IOMMU.
+     * VIOT should only describes the other two buses.
+     */
+    test_acpi_one("-machine default_bus_bypass_iommu=on "
+                  "-device virtio-iommu-pci "
+                  "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
+                  "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
+                  "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
+                  &data);
+    free_test_data(&data);
+}
+
+static void test_acpi_virt_viot(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+        .ram_start = 0x40000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+    };
+
+    test_acpi_one("-cpu cortex-a57 "
+                  "-device virtio-iommu-pci", &data);
+    free_test_data(&data);
+}
+
 static void test_oem_fields(test_data *data)
 {
     int i;
@@ -1567,12 +1603,14 @@ int main(int argc, char *argv[])
         if (strcmp(arch, "x86_64") == 0) {
             qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
         }
+        qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
     } else if (strcmp(arch, "aarch64") == 0) {
         qtest_add_func("acpi/virt", test_acpi_virt_tcg);
         qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
         qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
         qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
         qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
+        qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 11/12] tests/acpi: add expected blob for VIOT test on virt machine
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (9 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 10/12] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  2021-10-20 17:27 ` [PATCH v5 12/12] tests/acpi: add expected blobs for VIOT test on q35 machine Jean-Philippe Brucker
  11 siblings, 0 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

The VIOT blob contains the following:

[000h 0000   4]                    Signature : "VIOT"    [Virtual I/O Translation Table]
[004h 0004   4]                 Table Length : 00000058
[008h 0008   1]                     Revision : 00
[009h 0009   1]                     Checksum : 66
[00Ah 0010   6]                       Oem ID : "BOCHS "
[010h 0016   8]                 Oem Table ID : "BXPC    "
[018h 0024   4]                 Oem Revision : 00000001
[01Ch 0028   4]              Asl Compiler ID : "BXPC"
[020h 0032   4]        Asl Compiler Revision : 00000001

[024h 0036   2]                   Node count : 0002
[026h 0038   2]                  Node offset : 0030
[028h 0040   8]                     Reserved : 0000000000000000

[030h 0048   1]                         Type : 03 [VirtIO-PCI IOMMU]
[031h 0049   1]                     Reserved : 00
[032h 0050   2]                       Length : 0010

[034h 0052   2]                  PCI Segment : 0000
[036h 0054   2]               PCI BDF number : 0008
[038h 0056   8]                     Reserved : 0000000000000000

[040h 0064   1]                         Type : 01 [PCI Range]
[041h 0065   1]                     Reserved : 00
[042h 0066   2]                       Length : 0018

[044h 0068   4]               Endpoint start : 00000000
[048h 0072   2]            PCI Segment start : 0000
[04Ah 0074   2]              PCI Segment end : 0000
[04Ch 0076   2]                PCI BDF start : 0000
[04Eh 0078   2]                  PCI BDF end : 00FF
[050h 0080   2]                  Output node : 0030
[052h 0082   6]                     Reserved : 000000000000

Acked-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 tests/data/acpi/virt/VIOT                   | Bin 0 -> 88 bytes
 2 files changed, 1 deletion(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 29b5b1eabc..fa213e4738 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1,3 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/VIOT",
 "tests/data/acpi/q35/DSDT.viot",
 "tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d 100644
GIT binary patch
literal 88
zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX
I{D-Rq0Q5fy0RR91

literal 0
HcmV?d00001

-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 12/12] tests/acpi: add expected blobs for VIOT test on q35 machine
  2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
                   ` (10 preceding siblings ...)
  2021-10-20 17:27 ` [PATCH v5 11/12] tests/acpi: add expected blob for VIOT test on virt machine Jean-Philippe Brucker
@ 2021-10-20 17:27 ` Jean-Philippe Brucker
  11 siblings, 0 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-20 17:27 UTC (permalink / raw)
  To: mst, imammedo, peter.maydell
  Cc: Jean-Philippe Brucker, ehabkost, eric.auger, jasowang,
	richard.henderson, qemu-devel, peterx, shannon.zhaosl, qemu-arm,
	ani, pbonzini

Add expected blobs of the VIOT and DSDT table for the VIOT test on the
q35 machine.

Since the test instantiates a virtio device and two PCIe expander
bridges, DSDT.viot has more blocks than the base DSDT.

The VIOT table generated for the q35 test is:

[000h 0000   4]                    Signature : "VIOT"    [Virtual I/O Translation Table]
[004h 0004   4]                 Table Length : 00000070
[008h 0008   1]                     Revision : 00
[009h 0009   1]                     Checksum : 3D
[00Ah 0010   6]                       Oem ID : "BOCHS "
[010h 0016   8]                 Oem Table ID : "BXPC    "
[018h 0024   4]                 Oem Revision : 00000001
[01Ch 0028   4]              Asl Compiler ID : "BXPC"
[020h 0032   4]        Asl Compiler Revision : 00000001

[024h 0036   2]                   Node count : 0003
[026h 0038   2]                  Node offset : 0030
[028h 0040   8]                     Reserved : 0000000000000000

[030h 0048   1]                         Type : 03 [VirtIO-PCI IOMMU]
[031h 0049   1]                     Reserved : 00
[032h 0050   2]                       Length : 0010

[034h 0052   2]                  PCI Segment : 0000
[036h 0054   2]               PCI BDF number : 0010
[038h 0056   8]                     Reserved : 0000000000000000

[040h 0064   1]                         Type : 01 [PCI Range]
[041h 0065   1]                     Reserved : 00
[042h 0066   2]                       Length : 0018

[044h 0068   4]               Endpoint start : 00003000
[048h 0072   2]            PCI Segment start : 0000
[04Ah 0074   2]              PCI Segment end : 0000
[04Ch 0076   2]                PCI BDF start : 3000
[04Eh 0078   2]                  PCI BDF end : 30FF
[050h 0080   2]                  Output node : 0030
[052h 0082   6]                     Reserved : 000000000000

[058h 0088   1]                         Type : 01 [PCI Range]
[059h 0089   1]                     Reserved : 00
[05Ah 0090   2]                       Length : 0018

[05Ch 0092   4]               Endpoint start : 00001000
[060h 0096   2]            PCI Segment start : 0000
[062h 0098   2]              PCI Segment end : 0000
[064h 0100   2]                PCI BDF start : 1000
[066h 0102   2]                  PCI BDF end : 10FF
[068h 0104   2]                  Output node : 0030
[06Ah 0106   6]                     Reserved : 000000000000

And the DSDT diff is:

@@ -5,13 +5,13 @@
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/q35/DSDT, Wed Oct 20 16:33:00 2021
+ * Disassembly of /tmp/aml-4TWUB1, Wed Oct 20 16:16:49 2021
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x00002061 (8289)
+ *     Length           0x000024B6 (9398)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
- *     Checksum         0xF9
+ *     Checksum         0xA6
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPC    "
  *     OEM Revision     0x00000001 (1)
@@ -3114,6 +3114,339 @@
         }
     }

+    Scope (\_SB)
+    {
+        Device (PC30)
+        {
+            Name (_UID, 0x30)  // _UID: Unique ID
+            Name (_BBN, 0x30)  // _BBN: BIOS Bus Number
+            Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
+            Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
+            Method (_OSC, 4, NotSerialized)  // _OSC: Operating System Capabilities
+            {
+                CreateDWordField (Arg3, Zero, CDW1)
+                If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+                {
+                    CreateDWordField (Arg3, 0x04, CDW2)
+                    CreateDWordField (Arg3, 0x08, CDW3)
+                    Local0 = CDW3 /* \_SB_.PC30._OSC.CDW3 */
+                    Local0 &= 0x1F
+                    If ((Arg1 != One))
+                    {
+                        CDW1 |= 0x08
+                    }
+
+                    If ((CDW3 != Local0))
+                    {
+                        CDW1 |= 0x10
+                    }
+
+                    CDW3 = Local0
+                }
+                Else
+                {
+                    CDW1 |= 0x04
+                }
+
+                Return (Arg3)
+            }
+
+            Method (_PRT, 0, NotSerialized)  // _PRT: PCI Routing Table
+            {
+                Local0 = Package (0x80){}
+                Local1 = Zero
+                While ((Local1 < 0x80))
+                {
+                    Local2 = (Local1 >> 0x02)
+                    Local3 = ((Local1 + Local2) & 0x03)
+                    If ((Local3 == Zero))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKD,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == One))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKA,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == 0x02))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKB,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == 0x03))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKC,
+                                Zero
+                            }
+                    }
+
+                    Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF)
+                    Local4 [One] = (Local1 & 0x03)
+                    Local0 [Local1] = Local4
+                    Local1++
+                }
+
+                Return (Local0)
+            }
+
+            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+            {
+                WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+                    0x0000,             // Granularity
+                    0x0030,             // Range Minimum
+                    0x0030,             // Range Maximum
+                    0x0000,             // Translation Offset
+                    0x0001,             // Length
+                    ,, )
+            })
+        }
+    }
+
+    Scope (\_SB)
+    {
+        Device (PC20)
+        {
+            Name (_UID, 0x20)  // _UID: Unique ID
+            Name (_BBN, 0x20)  // _BBN: BIOS Bus Number
+            Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
+            Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
+            Method (_OSC, 4, NotSerialized)  // _OSC: Operating System Capabilities
+            {
+                CreateDWordField (Arg3, Zero, CDW1)
+                If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+                {
+                    CreateDWordField (Arg3, 0x04, CDW2)
+                    CreateDWordField (Arg3, 0x08, CDW3)
+                    Local0 = CDW3 /* \_SB_.PC20._OSC.CDW3 */
+                    Local0 &= 0x1F
+                    If ((Arg1 != One))
+                    {
+                        CDW1 |= 0x08
+                    }
+
+                    If ((CDW3 != Local0))
+                    {
+                        CDW1 |= 0x10
+                    }
+
+                    CDW3 = Local0
+                }
+                Else
+                {
+                    CDW1 |= 0x04
+                }
+
+                Return (Arg3)
+            }
+
+            Method (_PRT, 0, NotSerialized)  // _PRT: PCI Routing Table
+            {
+                Local0 = Package (0x80){}
+                Local1 = Zero
+                While ((Local1 < 0x80))
+                {
+                    Local2 = (Local1 >> 0x02)
+                    Local3 = ((Local1 + Local2) & 0x03)
+                    If ((Local3 == Zero))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKD,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == One))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKA,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == 0x02))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKB,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == 0x03))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKC,
+                                Zero
+                            }
+                    }
+
+                    Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF)
+                    Local4 [One] = (Local1 & 0x03)
+                    Local0 [Local1] = Local4
+                    Local1++
+                }
+
+                Return (Local0)
+            }
+
+            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+            {
+                WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+                    0x0000,             // Granularity
+                    0x0020,             // Range Minimum
+                    0x0020,             // Range Maximum
+                    0x0000,             // Translation Offset
+                    0x0001,             // Length
+                    ,, )
+            })
+        }
+    }
+
+    Scope (\_SB)
+    {
+        Device (PC10)
+        {
+            Name (_UID, 0x10)  // _UID: Unique ID
+            Name (_BBN, 0x10)  // _BBN: BIOS Bus Number
+            Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
+            Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
+            Method (_OSC, 4, NotSerialized)  // _OSC: Operating System Capabilities
+            {
+                CreateDWordField (Arg3, Zero, CDW1)
+                If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+                {
+                    CreateDWordField (Arg3, 0x04, CDW2)
+                    CreateDWordField (Arg3, 0x08, CDW3)
+                    Local0 = CDW3 /* \_SB_.PC10._OSC.CDW3 */
+                    Local0 &= 0x1F
+                    If ((Arg1 != One))
+                    {
+                        CDW1 |= 0x08
+                    }
+
+                    If ((CDW3 != Local0))
+                    {
+                        CDW1 |= 0x10
+                    }
+
+                    CDW3 = Local0
+                }
+                Else
+                {
+                    CDW1 |= 0x04
+                }
+
+                Return (Arg3)
+            }
+
+            Method (_PRT, 0, NotSerialized)  // _PRT: PCI Routing Table
+            {
+                Local0 = Package (0x80){}
+                Local1 = Zero
+                While ((Local1 < 0x80))
+                {
+                    Local2 = (Local1 >> 0x02)
+                    Local3 = ((Local1 + Local2) & 0x03)
+                    If ((Local3 == Zero))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKD,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == One))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKA,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == 0x02))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKB,
+                                Zero
+                            }
+                    }
+
+                    If ((Local3 == 0x03))
+                    {
+                        Local4 = Package (0x04)
+                            {
+                                Zero,
+                                Zero,
+                                LNKC,
+                                Zero
+                            }
+                    }
+
+                    Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF)
+                    Local4 [One] = (Local1 & 0x03)
+                    Local0 [Local1] = Local4
+                    Local1++
+                }
+
+                Return (Local0)
+            }
+
+            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+            {
+                WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+                    0x0000,             // Granularity
+                    0x0010,             // Range Minimum
+                    0x0010,             // Range Maximum
+                    0x0000,             // Translation Offset
+                    0x0001,             // Length
+                    ,, )
+            })
+        }
+    }
+
     Scope (\_SB.PCI0)
     {
         Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
@@ -3121,9 +3454,9 @@
             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
                 0x0000,             // Granularity
                 0x0000,             // Range Minimum
-                0x00FF,             // Range Maximum
+                0x000F,             // Range Maximum
                 0x0000,             // Translation Offset
-                0x0100,             // Length
+                0x0010,             // Length
                 ,, )
             IO (Decode16,
                 0x0CF8,             // Range Minimum
@@ -3278,6 +3611,26 @@
                 }
             }

+            Device (S10)
+            {
+                Name (_ADR, 0x00020000)  // _ADR: Address
+            }
+
+            Device (S18)
+            {
+                Name (_ADR, 0x00030000)  // _ADR: Address
+            }
+
+            Device (S20)
+            {
+                Name (_ADR, 0x00040000)  // _ADR: Address
+            }
+
+            Device (S28)
+            {
+                Name (_ADR, 0x00050000)  // _ADR: Address
+            }
+
             Method (PCNT, 0, NotSerialized)
             {
             }

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   2 --
 tests/data/acpi/q35/DSDT.viot               | Bin 0 -> 9398 bytes
 tests/data/acpi/q35/VIOT.viot               | Bin 0 -> 112 bytes
 3 files changed, 2 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index fa213e4738..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.viot",
-"tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..20e5c87b6be3be9df70a8a74d82f68bb2c6a25cf 100644
GIT binary patch
literal 9398
zcmeHNO>7&-8J*>iv|O&FB}G~OOGG$H|5C)1oWHhc5OS9yDTx$CQgH$r;8Idr*-4Q_
z5(9Az1F`}niVsB-#zBvCpcK8er(A2Gm-gmc1N78GFS!;)e6sq!nH_0{q=4wuOC(@-
zzuEWZn>V|&Z#cVN(`$Y8BxA-O3af6pnJv6pceD627-Muyd#4h+!rD7tq0}*wk(klG
zo#e;Z6wJ&|y~29Q`2Dv3x$nRGk#BSZw(*5?ccUA8u;*_u0$tw-EQ=1D;$){;=(KlT
ztL|2EW~W^7?EC}EG&`<1ikh9g*I;I6z2dRV%6MnB>BjE7!Cde1e7_d&ZKtL!r(ON$
zrEllXzx?^-ck}mu`Hx@SdBchUuz<aZ{iT35{!QPS^ZU-Hy{pD8qVsd}{pnA8{In=@
z=uF^K$vs;XQC0K%Z?tz`^cs1Ww{fCgtLNO@Jlf?pnyESK{718l+0;4o_veFygTIJF
z=G<UT)f6g=X1QK$NtFh)k*fAix!$TWo3RIvk}TG~?NB`ZO0ni;>kY4y%{<>}w(=~X
z5<j4#?)$#w_<j46p3i!I^59^f^(Q~+b=hZKoBiv*@qM;$O^LRptjP}()RqU2Sd1~W
z<_FJEYpB25yJsuSe1~^2($AqO^-5zcV)>TsEKgmGN+6-yyU#8cJb=YDilX&sl}vNm
znkgAR^O<3kj4if>{e=j!wRfMauC5=lrlvKPX~i#454Cp}R_d*JS$9laZ$ra6)<ns8
zFZy28G%xJ%nit&F>LDi%G<tIc=VA0=l$jSC&UvcQat~XR46h%rI$!}a%nQsw7u8Zn
zeY8_|n=K=G-?mI#8VX$W-Fg-qFWcT}7MCyz{$^Xaa7hZ>Law-k6NOr}VI&_28U=2l
zwqDKFE8eTwwozDdms#eix?5a|w4b2p;2_v0L~z5n%BYU^52<*cWuDH1GYUm@1+?))
zte^45>Rz)t*<T5V#)B9B{_o~<?^i#W{ib^6uWmJ<y59Va-+!WajrGs;u38a{fLr`N
zvT@rUu>DljxJ?^&Z?-?vyJn3C>3D=qux{Y*bs5|5n)Qmi$TD^Zdn4GU$ocJS2Hh-<
z`wbt+^+v0nUVdjMos8k`WGl7hA`{03ju%<lrgAHSpd^DRf-*}_#Ly0mB!LSfVgWcQ
z&T$@~G9)JI=hz5m0vkrel+Xy{Oh7pkAu-V!j*W7rY(bO}Q$nMH2`FbGB&N)QaV4<4
zo)~9JXiP9=;}NPl<C@MmXG&;XFlFNrsyfFsonxFSp<}vEgsRSQP3O3#b6nSnP}ON_
zI!#Tdsp~|j>ckUB>FI=~GokB5sOq#dotCE4(sd$KbtW~PNlj-`*NIToiD#j5J#9^=
zt?NXn>YUJYPG~wObe#xQos*i*NloXZt`niEb4t@WrRki~bs|)CI+{*L)9L6s5vn><
zn$DD_Go|Z9sOn5>I@6lYw5}7Os&iV?Ij!lO)^#FOb!If38BJ$K*NIToIiu;E(R9w}
zIuWWmPiZ<&X*y5oIuWWmF_XaEC!a&Jn$B5WCqh-{X-(&8P3LJ{Cqh-{8P3dyPr@^t
zSqL9?X9Uwd3W@23*s~h*tj0X6GZCuHa~kuU#yqDp5vt7d8uPryJg+kms?5hU=3^T3
zF`bD}Wj@ZC(q$g!O!<s|TrlN>8zC{}6`a#@&S^R4^qdGqP7?;2R}8e&tROWvQv=6H
z92zJBq!mFFflLYm6*z851|ybipMj(TdT5{sr8F3*z)AyEpk$x|dT5{s(Hd!>0vkq@
zQ#DWlJv2~+Xc5Q4Km|68LX8|s1}dO3RCU5YMYWs>16818pa`Wal7S+WI$@v!%b74x
z1xf}gpgboTC_<?d1}d<e2?JH2WS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+UG69y`<
zoCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!%b74x1xf}gpqxnticso=
zfeI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-7^uK<CJa=8l7R{+XOe*;
zlsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZulLa7r5DzKah16818paRO7
zWS|J8P8g`bawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPONd}5g>V$y`EN8+%6(|{~
zfN~}oC_<?d1}d<e2?JH2WS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cV
zN(L&RoJj_XQ0jz%3M^;BKouw%sDN@N87M-j69$S%F;GORfg)556rp0E3KIsZFv&m_
zCK;&0gn=qd7^uP|167!0pb8TPsxV=o3X=>}VUmF=Oc<!bgn=qdGEjv{2C6V&porwv
zg@Gc%72};r28s}uL59SXeG3CcB>NTyib(b?87M;8w`8CQu~c-n0}Ew+yT^a5?$Zy_
zdt<cAw{QRaVw8TR(y9incNq1?S{{zOR-lbb8_#4|ds!>e@gi+Xv?-gJ>B<^b1Nw=Z
z*3LE-SrYqJGV^R@&21zAXyj6pR`TO9mf$NvH`pC2v*OrRf}7jc6r&?FqJgECm2u1w
z@^Z<;#&QiU2AODfGj+<E{&<olqHs5c=QqQX7^@eVDZUQ0Pfb8;@|je}t2EgyI<|Ir
z$rP<eF(BLqk7qm0M$@>n9kzkeQ+b%ZFpwRKC*L`HLCId=*$Zf&n!S)2oxRZBt&(=J
zl!oQLP`ed5L3&q7@ACBSKzjF>^lq5m8xU_{LF+rK_muP=Pwx$+_l`;Lh3Sif>0>8I
zUsTc;dHUi&`r<L^i(&f3!SwMHq+e9hFY@$@1L+r!NxvATFAb(2J3;!AlD@>#mj=?8
zj!9n%)0YR+kDnlYSxH~!>B|G@%g3ZIBYlh>WrOJ*kzPJLFs;CF7wflrS!Rq6dF*Ih
zht&-ZV=JXshq=a3b)sx!bwlGatyhN$$WV2nY-Dvq;|xo{G|r?j%u0r;6J;Z-8yYtm
zy*f-)hN=@~BdZ%42Qzwgn7<5FC(1@vH*{a0(yPOyW~e$*HnO^*`*c>X4l|vh>O`4d
zopbJJJ$rYmSuNmI3tp?(QLOnpt1pSS6J_hl(Tab)QEbY))WYC|H%++p^=$dAcY`;t
z|2q5j>+igE{q4=y-(hXTt*<PHck6^@m5i_LuWiG|sy;l4md?MnExUdDM-=2))~y$7
zhFhsM4YyH!#lQl1jy8)1i@I*r!y<c{kWsf;T*(6N<y%>-HCJjCGhkGRyZ+J@t_KYg
zjhx84^-3-`pQC1o7f10-%55#a7U^5PZT6v?%jK?S%41l)&nMbXT)7yNuQo_1383Zo
z!D1u}nBPC!8$3Z00&5D(?n7r*miPTkHBPclmdF^a3N?(ZKeKqsZMrq{7#XHpD_%}V
z4g$6r7+YU>>E4zRu&u!G1J?aw#r|E-H|}nt$PYFLQ*yzBg}5mF*}H?KxGbAr_7lCX
zb$~Qq2BXgRpB(Bry6(Z|9<{N{u6NO@doT2#8-npW1D9{piUva+gOlK0-=r6d#h7@r
zNEL3dO}gaJw%MK^EpDhTwCDQ9=x(ki+|_2wKBk*T-z`J+qw|8RBl;$=B7>y_y*5~g
zZXVgX_6IN0J&o~?KMW7t*XBOm-53GYd@Gf-Oor74nulg4@zS)FBr$CJ>b1-{44zWG
z*eT`~SUBQoAkjVLtj6dRANb*z%016AKLd82QAZrU5fpE%l!Cfj<1N(?&eF^k@o>q{
z)ZI&X>fkwIVOJ>BXgB=2_y78eH}i4=*3@D_I<mt@OKtpa*V+l*?^+SI5!`&cyMf;q
zY_sq5HUUbA_dXumq5|WvAdiAnZ_|n#mK4I0=wXSA5qayzhUGANKeo5USa7q8y7#sW
z7HsYLG&uHsF$I{h(t8suf<+aU!#`bByra~*b|LKAtms<ozVF%-b}hVSWY^AzUF(Rh
z#qRsAJz>{6jApd_uC2}p>*jBX`NtDWN$V?ldozZ$IDkvWL;Im^m?$?eNx|Q@xceX4
zhw%gk4P*b|UtRF?^gH?BfPN>_UxaDjW0-ia{U>pT{ZTt}A0KKw5@+tebmwV&!x7dd
z%INV)FU9dB)&3}^uNK@Go|d>ja7XY@Lh^W8in$V%E0ZMgB|}n-XFf^Fj%CLT{^`R~
ztX{4+Mpc4N8ZqG-uIs*i-!@L;negK)NZ-8FNsq-0v>nIKcG70dis1Q0(Q|Z$lsLaQ
zx(w~zXBIEJVj?`TR;;eWR~873uRGKhJ8>sY(>kY+;16|Y$)u~;-c2@hIq?w*y5uL#
z+>0W>6*CxS=goPK;VT9lEXadPbkN33bUq{k8xaLZ<R~v#I6}lk!4V?PGGpXB?0>z7
B)Oi2^

literal 0
HcmV?d00001

diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..9b179266ccbf84f1c250ee646812d17e27987764 100644
GIT binary patch
literal 112
zcmWIZ^baXu00LVle`k+i1*eDrX9XZ&1PX!JAex!M0Hgv8m>C3sGzdcgBZCA3T-xBj
Q0Zb)W9Hva*zW_`e0M!8s0RR91

literal 0
HcmV?d00001

-- 
2.33.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 10/12] tests/acpi: add test cases for VIOT
  2021-10-20 17:27 ` [PATCH v5 10/12] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
@ 2021-10-21  9:02   ` Eric Auger
  2021-10-26  9:47     ` Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
  1 sibling, 1 reply; 27+ messages in thread
From: Eric Auger @ 2021-10-21  9:02 UTC (permalink / raw)
  To: Jean-Philippe Brucker, mst, imammedo, peter.maydell
  Cc: ehabkost, jasowang, richard.henderson, qemu-devel, peterx,
	shannon.zhaosl, qemu-arm, ani, pbonzini

Hi Jean,

On 10/20/21 7:27 PM, Jean-Philippe Brucker wrote:
> Add two test cases for VIOT, one on the q35 machine and the other on
> virt. To test complex topologies the q35 test has two PCIe buses that
> bypass the IOMMU (and are therefore not described by VIOT), and two
> buses that are translated by virtio-iommu.
>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 4f11d03055..599b155201 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1403,6 +1403,42 @@ static void test_acpi_virt_tcg(void)
>      free_test_data(&data);
>  }
>  
> +static void test_acpi_q35_viot(void)
> +{
> +    test_data data = {
> +        .machine = MACHINE_Q35,
> +        .variant = ".viot",
> +    };
> +
> +    /*
> +     * To keep things interesting, two buses bypass the IOMMU.
> +     * VIOT should only describes the other two buses.
> +     */
> +    test_acpi_one("-machine default_bus_bypass_iommu=on "
Just a reminder that we still have a dependency on the
default_bus_bypass_iommu fix here. Otherwise those tests will fail. So
the fix needs to be upstreamed along with that series.

Eric
> +                  "-device virtio-iommu-pci "
> +                  "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
> +                  "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
> +                  "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
> +static void test_acpi_virt_viot(void)
> +{
> +    test_data data = {
> +        .machine = "virt",
> +        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> +        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> +        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> +        .ram_start = 0x40000000ULL,
> +        .scan_len = 128ULL * 1024 * 1024,
> +    };
> +
> +    test_acpi_one("-cpu cortex-a57 "
> +                  "-device virtio-iommu-pci", &data);
> +    free_test_data(&data);
> +}
> +
>  static void test_oem_fields(test_data *data)
>  {
>      int i;
> @@ -1567,12 +1603,14 @@ int main(int argc, char *argv[])
>          if (strcmp(arch, "x86_64") == 0) {
>              qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
>          }
> +        qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
>      } else if (strcmp(arch, "aarch64") == 0) {
>          qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>          qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
>          qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
>          qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
>          qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
> +        qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
>      }
>      ret = g_test_run();
>      boot_sector_cleanup(disk);



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files
  2021-10-20 17:27 ` [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
@ 2021-10-21  9:02   ` Eric Auger
  2021-10-21 14:00   ` Igor Mammedov
  1 sibling, 0 replies; 27+ messages in thread
From: Eric Auger @ 2021-10-21  9:02 UTC (permalink / raw)
  To: Jean-Philippe Brucker, mst, imammedo, peter.maydell
  Cc: ehabkost, jasowang, richard.henderson, qemu-devel, peterx,
	shannon.zhaosl, qemu-arm, ani, pbonzini



On 10/20/21 7:27 PM, Jean-Philippe Brucker wrote:
> Create empty data files and allow updates for the upcoming VIOT tests.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
>  tests/data/acpi/q35/DSDT.viot               | 0
>  tests/data/acpi/q35/VIOT.viot               | 0
>  tests/data/acpi/virt/VIOT                   | 0
>  4 files changed, 3 insertions(+)
>  create mode 100644 tests/data/acpi/q35/DSDT.viot
>  create mode 100644 tests/data/acpi/q35/VIOT.viot
>  create mode 100644 tests/data/acpi/virt/VIOT
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..29b5b1eabc 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,4 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/virt/VIOT",
> +"tests/data/acpi/q35/DSDT.viot",
> +"tests/data/acpi/q35/VIOT.viot",
> diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT
> new file mode 100644
> index 0000000000..e69de29bb2



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 10/12] tests/acpi: add test cases for VIOT
  2021-10-20 17:27 ` [PATCH v5 10/12] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
@ 2021-10-21  9:02   ` Eric Auger
  1 sibling, 0 replies; 27+ messages in thread
From: Eric Auger @ 2021-10-21  9:02 UTC (permalink / raw)
  To: Jean-Philippe Brucker, mst, imammedo, peter.maydell
  Cc: ehabkost, jasowang, richard.henderson, qemu-devel, peterx,
	shannon.zhaosl, qemu-arm, ani, pbonzini

Hi Jean,

On 10/20/21 7:27 PM, Jean-Philippe Brucker wrote:
> Add two test cases for VIOT, one on the q35 machine and the other on
> virt. To test complex topologies the q35 test has two PCIe buses that
> bypass the IOMMU (and are therefore not described by VIOT), and two
> buses that are translated by virtio-iommu.
>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

small conflict to be resolved since
 220ffd949b ("tests: bios-tables-test: use qtest_has_accel() API to
register TCG only tests")

Eric
> ---
>  tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 4f11d03055..599b155201 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1403,6 +1403,42 @@ static void test_acpi_virt_tcg(void)
>      free_test_data(&data);
>  }
>  
> +static void test_acpi_q35_viot(void)
> +{
> +    test_data data = {
> +        .machine = MACHINE_Q35,
> +        .variant = ".viot",
> +    };
> +
> +    /*
> +     * To keep things interesting, two buses bypass the IOMMU.
> +     * VIOT should only describes the other two buses.
> +     */
> +    test_acpi_one("-machine default_bus_bypass_iommu=on "
> +                  "-device virtio-iommu-pci "
> +                  "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
> +                  "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
> +                  "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
> +static void test_acpi_virt_viot(void)
> +{
> +    test_data data = {
> +        .machine = "virt",
> +        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> +        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> +        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> +        .ram_start = 0x40000000ULL,
> +        .scan_len = 128ULL * 1024 * 1024,
> +    };
> +
> +    test_acpi_one("-cpu cortex-a57 "
> +                  "-device virtio-iommu-pci", &data);
> +    free_test_data(&data);
> +}
> +
>  static void test_oem_fields(test_data *data)
>  {
>      int i;
> @@ -1567,12 +1603,14 @@ int main(int argc, char *argv[])
>          if (strcmp(arch, "x86_64") == 0) {
>              qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
>          }
> +        qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
>      } else if (strcmp(arch, "aarch64") == 0) {
>          qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>          qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
>          qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
>          qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
>          qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
> +        qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
>      }
>      ret = g_test_run();
>      boot_sector_cleanup(disk);



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState
  2021-10-20 17:27 ` [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState Jean-Philippe Brucker
@ 2021-10-21  9:02   ` Eric Auger
  2021-10-21 13:34   ` Igor Mammedov
  1 sibling, 0 replies; 27+ messages in thread
From: Eric Auger @ 2021-10-21  9:02 UTC (permalink / raw)
  To: Jean-Philippe Brucker, mst, imammedo, peter.maydell
  Cc: ehabkost, jasowang, richard.henderson, qemu-devel, peterx,
	shannon.zhaosl, qemu-arm, ani, pbonzini



On 10/20/21 7:27 PM, Jean-Philippe Brucker wrote:
> We're about to support a third vIOMMU for x86, virtio-iommu which
> doesn't inherit X86IOMMUState. Move the IOMMU singleton into
> PCMachineState, so it can be shared between all three vIOMMUs.
>
> The x86_iommu_get_default() helper is still needed by KVM and IOAPIC to
> fetch the default IRQ-remapping IOMMU. Since virtio-iommu doesn't
> support IRQ remapping, this interface doesn't need to change for the
> moment. We could later replace X86IOMMUState with an "IRQ remapping
> IOMMU" interface if necessary.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> ---
>  include/hw/i386/pc.h |  1 +
>  hw/i386/pc.c         | 12 +++++++++++-
>  hw/i386/x86-iommu.c  | 26 ++++++++------------------
>  3 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 11426e26dc..b72e5bf9d1 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -35,6 +35,7 @@ typedef struct PCMachineState {
>      I2CBus *smbus;
>      PFlashCFI01 *flash[2];
>      ISADevice *pcspk;
> +    DeviceState *iommu;
>  
>      /* Configuration options: */
>      uint64_t max_ram_below_4g;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 54e4c00dce..fcbf328e8d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1330,6 +1330,15 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
>                 object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
>          pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
> +        PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +
> +        if (pcms->iommu) {
> +            error_setg(errp, "QEMU does not support multiple vIOMMUs "
> +                       "for x86 yet.");
> +            return;
> +        }
> +        pcms->iommu = dev;
>      }
>  }
>  
> @@ -1384,7 +1393,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> -        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
>          return HOTPLUG_HANDLER(machine);
>      }
>  
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index dc968c7a53..01d11325a6 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -77,25 +77,17 @@ void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *msg_out)
>      msg_out->data = msg.msi_data;
>  }
>  
> -/* Default X86 IOMMU device */
> -static X86IOMMUState *x86_iommu_default = NULL;
> -
> -static void x86_iommu_set_default(X86IOMMUState *x86_iommu)
> +X86IOMMUState *x86_iommu_get_default(void)
>  {
> -    assert(x86_iommu);
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    PCMachineState *pcms =
> +        PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
>  
> -    if (x86_iommu_default) {
> -        error_report("QEMU does not support multiple vIOMMUs "
> -                     "for x86 yet.");
> -        exit(1);
> +    if (pcms &&
> +        object_dynamic_cast(OBJECT(pcms->iommu), TYPE_X86_IOMMU_DEVICE)) {
> +        return X86_IOMMU_DEVICE(pcms->iommu);
>      }
> -
> -    x86_iommu_default = x86_iommu;
> -}
> -
> -X86IOMMUState *x86_iommu_get_default(void)
> -{
> -    return x86_iommu_default;
> +    return NULL;
>  }
>  
>  static void x86_iommu_realize(DeviceState *dev, Error **errp)
> @@ -131,8 +123,6 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
>      if (x86_class->realize) {
>          x86_class->realize(dev, errp);
>      }
> -
> -    x86_iommu_set_default(X86_IOMMU_DEVICE(dev));
>  }
>  
>  static Property x86_iommu_properties[] = {



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type()
  2021-10-20 17:27 ` [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type() Jean-Philippe Brucker
@ 2021-10-21  9:02   ` Eric Auger
  2021-10-21 13:29   ` Igor Mammedov
  1 sibling, 0 replies; 27+ messages in thread
From: Eric Auger @ 2021-10-21  9:02 UTC (permalink / raw)
  To: Jean-Philippe Brucker, mst, imammedo, peter.maydell
  Cc: ehabkost, jasowang, richard.henderson, qemu-devel, peterx,
	shannon.zhaosl, qemu-arm, ani, pbonzini

Hi Jean,

On 10/20/21 7:27 PM, Jean-Philippe Brucker wrote:
> To generate the IOMMU ACPI table, acpi-build.c can use base QEMU types
> instead of a special IommuType value.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Looks good to me

Thanks

Eric

> ---
>  include/hw/i386/x86-iommu.h | 12 ------------
>  hw/i386/acpi-build.c        | 20 +++++++++-----------
>  hw/i386/amd_iommu.c         |  2 --
>  hw/i386/intel_iommu.c       |  3 ---
>  hw/i386/x86-iommu-stub.c    |  5 -----
>  hw/i386/x86-iommu.c         |  5 -----
>  6 files changed, 9 insertions(+), 38 deletions(-)
>
> diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
> index 9de92d33a1..5ba0c056d6 100644
> --- a/include/hw/i386/x86-iommu.h
> +++ b/include/hw/i386/x86-iommu.h
> @@ -33,12 +33,6 @@ OBJECT_DECLARE_TYPE(X86IOMMUState, X86IOMMUClass, X86_IOMMU_DEVICE)
>  typedef struct X86IOMMUIrq X86IOMMUIrq;
>  typedef struct X86IOMMU_MSIMessage X86IOMMU_MSIMessage;
>  
> -typedef enum IommuType {
> -    TYPE_INTEL,
> -    TYPE_AMD,
> -    TYPE_NONE
> -} IommuType;
> -
>  struct X86IOMMUClass {
>      SysBusDeviceClass parent;
>      /* Intel/AMD specific realize() hook */
> @@ -71,7 +65,6 @@ struct X86IOMMUState {
>      OnOffAuto intr_supported;   /* Whether vIOMMU supports IR */
>      bool dt_supported;          /* Whether vIOMMU supports DT */
>      bool pt_supported;          /* Whether vIOMMU supports pass-through */
> -    IommuType type;             /* IOMMU type - AMD/Intel     */
>      QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
>  };
>  
> @@ -140,11 +133,6 @@ struct X86IOMMU_MSIMessage {
>   */
>  X86IOMMUState *x86_iommu_get_default(void);
>  
> -/*
> - * x86_iommu_get_type - get IOMMU type
> - */
> -IommuType x86_iommu_get_type(void);
> -
>  /**
>   * x86_iommu_iec_register_notifier - register IEC (Interrupt Entry
>   *                                   Cache) notifiers
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 81418b7911..ab49e799ff 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2488,6 +2488,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>      PCMachineState *pcms = PC_MACHINE(machine);
>      PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>      X86MachineState *x86ms = X86_MACHINE(machine);
> +    X86IOMMUState *iommu = x86_iommu_get_default();
>      GArray *table_offsets;
>      unsigned facs, dsdt, rsdt, fadt;
>      AcpiPmInfo pm;
> @@ -2604,17 +2605,14 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>          build_mcfg(tables_blob, tables->linker, &mcfg, x86ms->oem_id,
>                     x86ms->oem_table_id);
>      }
> -    if (x86_iommu_get_default()) {
> -        IommuType IOMMUType = x86_iommu_get_type();
> -        if (IOMMUType == TYPE_AMD) {
> -            acpi_add_table(table_offsets, tables_blob);
> -            build_amd_iommu(tables_blob, tables->linker, x86ms->oem_id,
> -                            x86ms->oem_table_id);
> -        } else if (IOMMUType == TYPE_INTEL) {
> -            acpi_add_table(table_offsets, tables_blob);
> -            build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
> -                           x86ms->oem_table_id);
> -        }
> +    if (object_dynamic_cast(OBJECT(iommu), TYPE_AMD_IOMMU_DEVICE)) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_amd_iommu(tables_blob, tables->linker, x86ms->oem_id,
> +                        x86ms->oem_table_id);
> +    } else if (object_dynamic_cast(OBJECT(iommu), TYPE_INTEL_IOMMU_DEVICE)) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
> +                       x86ms->oem_table_id);
>      }
>      if (machine->nvdimms_state->is_enabled) {
>          nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 9242a0d3ed..91fe34ae58 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1538,7 +1538,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>  {
>      int ret = 0;
>      AMDVIState *s = AMD_IOMMU_DEVICE(dev);
> -    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>      MachineState *ms = MACHINE(qdev_get_machine());
>      PCMachineState *pcms = PC_MACHINE(ms);
>      X86MachineState *x86ms = X86_MACHINE(ms);
> @@ -1548,7 +1547,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>                                       amdvi_uint64_equal, g_free, g_free);
>  
>      /* This device should take care of IOMMU PCI properties */
> -    x86_iommu->type = TYPE_AMD;
>      if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) {
>          return;
>      }
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 75f075547f..c27b20090e 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3806,9 +3806,6 @@ static void vtd_realize(DeviceState *dev, Error **errp)
>      X86MachineState *x86ms = X86_MACHINE(ms);
>      PCIBus *bus = pcms->bus;
>      IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
> -    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
> -
> -    x86_iommu->type = TYPE_INTEL;
>  
>      if (!vtd_decide_config(s, errp)) {
>          return;
> diff --git a/hw/i386/x86-iommu-stub.c b/hw/i386/x86-iommu-stub.c
> index c5ba077f9d..781b5ff922 100644
> --- a/hw/i386/x86-iommu-stub.c
> +++ b/hw/i386/x86-iommu-stub.c
> @@ -36,8 +36,3 @@ bool x86_iommu_ir_supported(X86IOMMUState *s)
>  {
>      return false;
>  }
> -
> -IommuType x86_iommu_get_type(void)
> -{
> -    abort();
> -}
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index 86ad03972e..dc968c7a53 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -98,11 +98,6 @@ X86IOMMUState *x86_iommu_get_default(void)
>      return x86_iommu_default;
>  }
>  
> -IommuType x86_iommu_get_type(void)
> -{
> -    return x86_iommu_default->type;
> -}
> -
>  static void x86_iommu_realize(DeviceState *dev, Error **errp)
>  {
>      X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type()
  2021-10-20 17:27 ` [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type() Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
@ 2021-10-21 13:29   ` Igor Mammedov
  1 sibling, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 13:29 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:36 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> To generate the IOMMU ACPI table, acpi-build.c can use base QEMU types
> instead of a special IommuType value.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/i386/x86-iommu.h | 12 ------------
>  hw/i386/acpi-build.c        | 20 +++++++++-----------
>  hw/i386/amd_iommu.c         |  2 --
>  hw/i386/intel_iommu.c       |  3 ---
>  hw/i386/x86-iommu-stub.c    |  5 -----
>  hw/i386/x86-iommu.c         |  5 -----
>  6 files changed, 9 insertions(+), 38 deletions(-)
> 
> diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
> index 9de92d33a1..5ba0c056d6 100644
> --- a/include/hw/i386/x86-iommu.h
> +++ b/include/hw/i386/x86-iommu.h
> @@ -33,12 +33,6 @@ OBJECT_DECLARE_TYPE(X86IOMMUState, X86IOMMUClass, X86_IOMMU_DEVICE)
>  typedef struct X86IOMMUIrq X86IOMMUIrq;
>  typedef struct X86IOMMU_MSIMessage X86IOMMU_MSIMessage;
>  
> -typedef enum IommuType {
> -    TYPE_INTEL,
> -    TYPE_AMD,
> -    TYPE_NONE
> -} IommuType;
> -
>  struct X86IOMMUClass {
>      SysBusDeviceClass parent;
>      /* Intel/AMD specific realize() hook */
> @@ -71,7 +65,6 @@ struct X86IOMMUState {
>      OnOffAuto intr_supported;   /* Whether vIOMMU supports IR */
>      bool dt_supported;          /* Whether vIOMMU supports DT */
>      bool pt_supported;          /* Whether vIOMMU supports pass-through */
> -    IommuType type;             /* IOMMU type - AMD/Intel     */
>      QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
>  };
>  
> @@ -140,11 +133,6 @@ struct X86IOMMU_MSIMessage {
>   */
>  X86IOMMUState *x86_iommu_get_default(void);
>  
> -/*
> - * x86_iommu_get_type - get IOMMU type
> - */
> -IommuType x86_iommu_get_type(void);
> -
>  /**
>   * x86_iommu_iec_register_notifier - register IEC (Interrupt Entry
>   *                                   Cache) notifiers
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 81418b7911..ab49e799ff 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2488,6 +2488,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>      PCMachineState *pcms = PC_MACHINE(machine);
>      PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>      X86MachineState *x86ms = X86_MACHINE(machine);
> +    X86IOMMUState *iommu = x86_iommu_get_default();
>      GArray *table_offsets;
>      unsigned facs, dsdt, rsdt, fadt;
>      AcpiPmInfo pm;
> @@ -2604,17 +2605,14 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>          build_mcfg(tables_blob, tables->linker, &mcfg, x86ms->oem_id,
>                     x86ms->oem_table_id);
>      }
> -    if (x86_iommu_get_default()) {
> -        IommuType IOMMUType = x86_iommu_get_type();
> -        if (IOMMUType == TYPE_AMD) {
> -            acpi_add_table(table_offsets, tables_blob);
> -            build_amd_iommu(tables_blob, tables->linker, x86ms->oem_id,
> -                            x86ms->oem_table_id);
> -        } else if (IOMMUType == TYPE_INTEL) {
> -            acpi_add_table(table_offsets, tables_blob);
> -            build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
> -                           x86ms->oem_table_id);
> -        }
> +    if (object_dynamic_cast(OBJECT(iommu), TYPE_AMD_IOMMU_DEVICE)) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_amd_iommu(tables_blob, tables->linker, x86ms->oem_id,
> +                        x86ms->oem_table_id);
> +    } else if (object_dynamic_cast(OBJECT(iommu), TYPE_INTEL_IOMMU_DEVICE)) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
> +                       x86ms->oem_table_id);
>      }
>      if (machine->nvdimms_state->is_enabled) {
>          nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 9242a0d3ed..91fe34ae58 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1538,7 +1538,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>  {
>      int ret = 0;
>      AMDVIState *s = AMD_IOMMU_DEVICE(dev);
> -    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>      MachineState *ms = MACHINE(qdev_get_machine());
>      PCMachineState *pcms = PC_MACHINE(ms);
>      X86MachineState *x86ms = X86_MACHINE(ms);
> @@ -1548,7 +1547,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>                                       amdvi_uint64_equal, g_free, g_free);
>  
>      /* This device should take care of IOMMU PCI properties */
> -    x86_iommu->type = TYPE_AMD;
>      if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) {
>          return;
>      }
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 75f075547f..c27b20090e 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3806,9 +3806,6 @@ static void vtd_realize(DeviceState *dev, Error **errp)
>      X86MachineState *x86ms = X86_MACHINE(ms);
>      PCIBus *bus = pcms->bus;
>      IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
> -    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
> -
> -    x86_iommu->type = TYPE_INTEL;
>  
>      if (!vtd_decide_config(s, errp)) {
>          return;
> diff --git a/hw/i386/x86-iommu-stub.c b/hw/i386/x86-iommu-stub.c
> index c5ba077f9d..781b5ff922 100644
> --- a/hw/i386/x86-iommu-stub.c
> +++ b/hw/i386/x86-iommu-stub.c
> @@ -36,8 +36,3 @@ bool x86_iommu_ir_supported(X86IOMMUState *s)
>  {
>      return false;
>  }
> -
> -IommuType x86_iommu_get_type(void)
> -{
> -    abort();
> -}
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index 86ad03972e..dc968c7a53 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -98,11 +98,6 @@ X86IOMMUState *x86_iommu_get_default(void)
>      return x86_iommu_default;
>  }
>  
> -IommuType x86_iommu_get_type(void)
> -{
> -    return x86_iommu_default->type;
> -}
> -
>  static void x86_iommu_realize(DeviceState *dev, Error **errp)
>  {
>      X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState
  2021-10-20 17:27 ` [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
@ 2021-10-21 13:34   ` Igor Mammedov
  1 sibling, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 13:34 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:37 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> We're about to support a third vIOMMU for x86, virtio-iommu which
> doesn't inherit X86IOMMUState. Move the IOMMU singleton into
> PCMachineState, so it can be shared between all three vIOMMUs.
> 
> The x86_iommu_get_default() helper is still needed by KVM and IOAPIC to
> fetch the default IRQ-remapping IOMMU. Since virtio-iommu doesn't
> support IRQ remapping, this interface doesn't need to change for the
> moment. We could later replace X86IOMMUState with an "IRQ remapping
> IOMMU" interface if necessary.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/i386/pc.h |  1 +
>  hw/i386/pc.c         | 12 +++++++++++-
>  hw/i386/x86-iommu.c  | 26 ++++++++------------------
>  3 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 11426e26dc..b72e5bf9d1 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -35,6 +35,7 @@ typedef struct PCMachineState {
>      I2CBus *smbus;
>      PFlashCFI01 *flash[2];
>      ISADevice *pcspk;
> +    DeviceState *iommu;
>  
>      /* Configuration options: */
>      uint64_t max_ram_below_4g;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 54e4c00dce..fcbf328e8d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1330,6 +1330,15 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
>                 object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
>          pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
> +        PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +
> +        if (pcms->iommu) {
> +            error_setg(errp, "QEMU does not support multiple vIOMMUs "
> +                       "for x86 yet.");
> +            return;
> +        }
> +        pcms->iommu = dev;
>      }
>  }
>  
> @@ -1384,7 +1393,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> -        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
>          return HOTPLUG_HANDLER(machine);
>      }
>  
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index dc968c7a53..01d11325a6 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -77,25 +77,17 @@ void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *msg_out)
>      msg_out->data = msg.msi_data;
>  }
>  
> -/* Default X86 IOMMU device */
> -static X86IOMMUState *x86_iommu_default = NULL;
> -
> -static void x86_iommu_set_default(X86IOMMUState *x86_iommu)
> +X86IOMMUState *x86_iommu_get_default(void)
>  {
> -    assert(x86_iommu);
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    PCMachineState *pcms =
> +        PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
>  
> -    if (x86_iommu_default) {
> -        error_report("QEMU does not support multiple vIOMMUs "
> -                     "for x86 yet.");
> -        exit(1);
> +    if (pcms &&
> +        object_dynamic_cast(OBJECT(pcms->iommu), TYPE_X86_IOMMU_DEVICE)) {
> +        return X86_IOMMU_DEVICE(pcms->iommu);
>      }
> -
> -    x86_iommu_default = x86_iommu;
> -}
> -
> -X86IOMMUState *x86_iommu_get_default(void)
> -{
> -    return x86_iommu_default;
> +    return NULL;
>  }
>  
>  static void x86_iommu_realize(DeviceState *dev, Error **errp)
> @@ -131,8 +123,6 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
>      if (x86_class->realize) {
>          x86_class->realize(dev, errp);
>      }
> -
> -    x86_iommu_set_default(X86_IOMMU_DEVICE(dev));
>  }
>  
>  static Property x86_iommu_properties[] = {



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device
  2021-10-20 17:27 ` [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
@ 2021-10-21 13:47   ` Igor Mammedov
  2021-10-25 11:23     ` Jean-Philippe Brucker
  0 siblings, 1 reply; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 13:47 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:38 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> Allow instantiating a virtio-iommu device by adding an ACPI Virtual I/O
> Translation table (VIOT), which describes the relation between the
> virtio-iommu and the endpoints it manages.
> 
> Add a hotplug handler for virtio-iommu on x86 and set the necessary
> reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
> region is reserved for MSIs. DMA transactions to this range either
> trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.

shouldn't above be "IO remapping"?

> Although virtio-iommu does not support IRQ remapping it must be informed
> of the reserved region so that it can forward DMA transactions targeting
> this region.
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/acpi-build.c | 10 +++++++++-
>  hw/i386/pc.c         | 16 +++++++++++++++-
>  hw/i386/Kconfig      |  1 +
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ab49e799ff..3ca6cc8118 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -68,9 +68,11 @@
>  #include "qom/qom-qobject.h"
>  #include "hw/i386/amd_iommu.h"
>  #include "hw/i386/intel_iommu.h"
> +#include "hw/virtio/virtio-iommu.h"
>  
>  #include "hw/acpi/ipmi.h"
>  #include "hw/acpi/hmat.h"
> +#include "hw/acpi/viot.h"
>  
>  /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>   * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
> @@ -2488,7 +2490,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>      PCMachineState *pcms = PC_MACHINE(machine);
>      PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>      X86MachineState *x86ms = X86_MACHINE(machine);
> -    X86IOMMUState *iommu = x86_iommu_get_default();
> +    DeviceState *iommu = pcms->iommu;
>      GArray *table_offsets;
>      unsigned facs, dsdt, rsdt, fadt;
>      AcpiPmInfo pm;
> @@ -2613,6 +2615,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>          acpi_add_table(table_offsets, tables_blob);
>          build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
>                         x86ms->oem_table_id);
> +    } else if (object_dynamic_cast(OBJECT(iommu), TYPE_VIRTIO_IOMMU_PCI)) {
> +        PCIDevice *pdev = PCI_DEVICE(iommu);
> +
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_viot(machine, tables_blob, tables->linker, pci_get_bdf(pdev),
> +                   x86ms->oem_id, x86ms->oem_table_id);
>      }
>      if (machine->nvdimms_state->is_enabled) {
>          nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index fcbf328e8d..f47f7866c7 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -83,6 +83,7 @@
>  #include "hw/i386/intel_iommu.h"
>  #include "hw/net/ne2000-isa.h"
>  #include "standard-headers/asm-x86/bootparam.h"
> +#include "hw/virtio/virtio-iommu.h"
>  #include "hw/virtio/virtio-pmem-pci.h"
>  #include "hw/virtio/virtio-mem-pci.h"
>  #include "hw/mem/memory-device.h"
> @@ -1330,7 +1331,19 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
>                 object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
>          pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
> -    } else if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> +        /* Declare the APIC range as the reserved MSI region */
> +        char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
> +                                              VIRTIO_IOMMU_RESV_MEM_T_MSI);
> +
> +        object_property_set_uint(OBJECT(dev), "len-reserved-regions", 1, errp);
> +        object_property_set_str(OBJECT(dev), "reserved-regions[0]",
> +                                resv_prop_str, errp);
> +        g_free(resv_prop_str);
> +    }
> +
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>          PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
>          if (pcms->iommu) {
> @@ -1394,6 +1407,7 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
>          object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE)) {
>          return HOTPLUG_HANDLER(machine);
>      }
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 962d2c981b..d22ac4a4b9 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -59,6 +59,7 @@ config PC_ACPI
>      select ACPI_X86
>      select ACPI_CPU_HOTPLUG
>      select ACPI_MEMORY_HOTPLUG
> +    select ACPI_VIOT
>      select SMBUS_EEPROM
>      select PFLASH_CFI01
>      depends on ACPI_SMBUS



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 05/12] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu
  2021-10-20 17:27 ` [PATCH v5 05/12] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu Jean-Philippe Brucker
@ 2021-10-21 13:49   ` Igor Mammedov
  0 siblings, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 13:49 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:39 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> When a virtio-iommu is instantiated, describe it using the ACPI VIOT
> table.
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt-acpi-build.c | 7 +++++++
>  hw/arm/Kconfig           | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6cec97352b..e26639e1e1 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -55,6 +55,7 @@
>  #include "kvm_arm.h"
>  #include "migration/vmstate.h"
>  #include "hw/acpi/ghes.h"
> +#include "hw/acpi/viot.h"
>  
>  #define ARM_SPI_BASE 32
>  
> @@ -934,6 +935,12 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>      }
>  #endif
>  
> +    if (vms->iommu == VIRT_IOMMU_VIRTIO) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_viot(ms, tables_blob, tables->linker, vms->virtio_iommu_bdf,
> +                   vms->oem_id, vms->oem_table_id);
> +    }
> +
>      /* XSDT is pointed to by RSDP */
>      xsdt = tables_blob->len;
>      build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 2d37d29f02..e652590943 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -27,6 +27,7 @@ config ARM_VIRT
>      select DIMM
>      select ACPI_HW_REDUCED
>      select ACPI_APEI
> +    select ACPI_VIOT
>  
>  config CHEETAH
>      bool



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 06/12] hw/arm/virt: Remove device tree restriction for virtio-iommu
  2021-10-20 17:27 ` [PATCH v5 06/12] hw/arm/virt: Remove device tree restriction " Jean-Philippe Brucker
@ 2021-10-21 13:53   ` Igor Mammedov
  0 siblings, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 13:53 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:40 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> virtio-iommu is now supported with ACPI VIOT as well as device tree.
> Remove the restriction that prevents from instantiating a virtio-iommu
> device under ACPI.
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt.c                | 10 ++--------
>  hw/virtio/virtio-iommu-pci.c | 12 ++----------
>  2 files changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f7f456bf58..3da7a86e37 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2561,16 +2561,10 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>  
>      if (device_is_dynamic_sysbus(mc, dev) ||
> -       (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
> +        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>          return HOTPLUG_HANDLER(machine);
>      }
> -    if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> -        VirtMachineState *vms = VIRT_MACHINE(machine);
> -
> -        if (!vms->bootinfo.firmware_loaded || !virt_is_acpi_enabled(vms)) {
> -            return HOTPLUG_HANDLER(machine);
> -        }
> -    }
>      return NULL;
>  }
>  
> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> index 770c286be7..5c0b87316a 100644
> --- a/hw/virtio/virtio-iommu-pci.c
> +++ b/hw/virtio/virtio-iommu-pci.c
> @@ -48,16 +48,8 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>      VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
>  
>      if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) {
> -        MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> -
> -        error_setg(errp,
> -                   "%s machine fails to create iommu-map device tree bindings",
> -                   mc->name);
> -        error_append_hint(errp,
> -                          "Check your machine implements a hotplug handler "
> -                          "for the virtio-iommu-pci device\n");
> -        error_append_hint(errp, "Check the guest is booted without FW or with "
> -                          "-no-acpi\n");
> +        error_setg(errp, "Check your machine implements a hotplug handler "
> +                         "for the virtio-iommu-pci device");
>          return;
>      }
>      for (int i = 0; i < s->nb_reserved_regions; i++) {



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 07/12] hw/arm/virt: Reject instantiation of multiple IOMMUs
  2021-10-20 17:27 ` [PATCH v5 07/12] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
@ 2021-10-21 13:54   ` Igor Mammedov
  0 siblings, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 13:54 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:41 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> We do not support instantiating multiple IOMMUs. Before adding a
> virtio-iommu, check that no other IOMMU is present. This will detect
> both "iommu=smmuv3" machine parameter and another virtio-iommu instance.
> 
> Fixes: 70e89132c9 ("hw/arm/virt: Add the virtio-iommu device tree mappings")
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3da7a86e37..25db8d4262 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2441,6 +2441,11 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>          hwaddr db_start = 0, db_end = 0;
>          char *resv_prop_str;
>  
> +        if (vms->iommu != VIRT_IOMMU_NONE) {
> +            error_setg(errp, "virt machine does not support multiple IOMMUs");
> +            return;
> +        }
> +
>          switch (vms->msi_controller) {
>          case VIRT_MSI_CTRL_NONE:
>              return;



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files
  2021-10-20 17:27 ` [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
  2021-10-21  9:02   ` Eric Auger
@ 2021-10-21 14:00   ` Igor Mammedov
  1 sibling, 0 replies; 27+ messages in thread
From: Igor Mammedov @ 2021-10-21 14:00 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Wed, 20 Oct 2021 18:27:43 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> Create empty data files and allow updates for the upcoming VIOT tests.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
>  tests/data/acpi/q35/DSDT.viot               | 0
>  tests/data/acpi/q35/VIOT.viot               | 0
>  tests/data/acpi/virt/VIOT                   | 0
>  4 files changed, 3 insertions(+)
>  create mode 100644 tests/data/acpi/q35/DSDT.viot
>  create mode 100644 tests/data/acpi/q35/VIOT.viot
>  create mode 100644 tests/data/acpi/virt/VIOT
> 
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..29b5b1eabc 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,4 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/virt/VIOT",
> +"tests/data/acpi/q35/DSDT.viot",
> +"tests/data/acpi/q35/VIOT.viot",
> diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT
> new file mode 100644
> index 0000000000..e69de29bb2

Acked-by: Igor Mammedov <imammedo@redhat.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device
  2021-10-21 13:47   ` Igor Mammedov
@ 2021-10-25 11:23     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-25 11:23 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, ani, pbonzini,
	eric.auger

On Thu, Oct 21, 2021 at 03:47:09PM +0200, Igor Mammedov wrote:
> On Wed, 20 Oct 2021 18:27:38 +0100
> Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> 
> > Allow instantiating a virtio-iommu device by adding an ACPI Virtual I/O
> > Translation table (VIOT), which describes the relation between the
> > virtio-iommu and the endpoints it manages.
> > 
> > Add a hotplug handler for virtio-iommu on x86 and set the necessary
> > reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
> > region is reserved for MSIs. DMA transactions to this range either
> > trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.
> 
> shouldn't above be "IO remapping"?

No it is IRQ remapping: DMA writes to this address range are interrupt
requests and the IOMMU may either perform interrupt remapping or pass it
to the APIC directly

> 
> > Although virtio-iommu does not support IRQ remapping it must be informed
> > of the reserved region so that it can forward DMA transactions targeting
> > this region.
> > 
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Eric Auger <eric.auger@redhat.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Thanks!
Jean



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 10/12] tests/acpi: add test cases for VIOT
  2021-10-21  9:02   ` Eric Auger
@ 2021-10-26  9:47     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 27+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-26  9:47 UTC (permalink / raw)
  To: Eric Auger
  Cc: peter.maydell, ehabkost, mst, jasowang, richard.henderson,
	qemu-devel, peterx, shannon.zhaosl, qemu-arm, pbonzini, ani,
	imammedo

On Thu, Oct 21, 2021 at 11:02:27AM +0200, Eric Auger wrote:
> Hi Jean,
> 
> On 10/20/21 7:27 PM, Jean-Philippe Brucker wrote:
> > Add two test cases for VIOT, one on the q35 machine and the other on
> > virt. To test complex topologies the q35 test has two PCIe buses that
> > bypass the IOMMU (and are therefore not described by VIOT), and two
> > buses that are translated by virtio-iommu.
> >
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >  tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 4f11d03055..599b155201 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -1403,6 +1403,42 @@ static void test_acpi_virt_tcg(void)
> >      free_test_data(&data);
> >  }
> >  
> > +static void test_acpi_q35_viot(void)
> > +{
> > +    test_data data = {
> > +        .machine = MACHINE_Q35,
> > +        .variant = ".viot",
> > +    };
> > +
> > +    /*
> > +     * To keep things interesting, two buses bypass the IOMMU.
> > +     * VIOT should only describes the other two buses.
> > +     */
> > +    test_acpi_one("-machine default_bus_bypass_iommu=on "
> Just a reminder that we still have a dependency on the
> default_bus_bypass_iommu fix here. Otherwise those tests will fail. So
> the fix needs to be upstreamed along with that series.

Thanks for the reminder, the fix is now queued (for x86, which this patch
relies on) so should be good to go. I'm planning to fix the conflict you
reported and resend only the x86 parts, so they get be merged this cycle

Thanks,
Jean



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2021-10-26  9:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 17:27 [PATCH v5 00/12] virtio-iommu: Add ACPI support Jean-Philippe Brucker
2021-10-20 17:27 ` [PATCH v5 01/12] hw/acpi: Add VIOT table Jean-Philippe Brucker
2021-10-20 17:27 ` [PATCH v5 02/12] hw/i386/pc: Remove x86_iommu_get_type() Jean-Philippe Brucker
2021-10-21  9:02   ` Eric Auger
2021-10-21 13:29   ` Igor Mammedov
2021-10-20 17:27 ` [PATCH v5 03/12] hw/i386/pc: Move IOMMU singleton into PCMachineState Jean-Philippe Brucker
2021-10-21  9:02   ` Eric Auger
2021-10-21 13:34   ` Igor Mammedov
2021-10-20 17:27 ` [PATCH v5 04/12] hw/i386/pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
2021-10-21 13:47   ` Igor Mammedov
2021-10-25 11:23     ` Jean-Philippe Brucker
2021-10-20 17:27 ` [PATCH v5 05/12] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu Jean-Philippe Brucker
2021-10-21 13:49   ` Igor Mammedov
2021-10-20 17:27 ` [PATCH v5 06/12] hw/arm/virt: Remove device tree restriction " Jean-Philippe Brucker
2021-10-21 13:53   ` Igor Mammedov
2021-10-20 17:27 ` [PATCH v5 07/12] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
2021-10-21 13:54   ` Igor Mammedov
2021-10-20 17:27 ` [PATCH v5 08/12] hw/arm/virt: Use object_property_set instead of qdev_prop_set Jean-Philippe Brucker
2021-10-20 17:27 ` [PATCH v5 09/12] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
2021-10-21  9:02   ` Eric Auger
2021-10-21 14:00   ` Igor Mammedov
2021-10-20 17:27 ` [PATCH v5 10/12] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
2021-10-21  9:02   ` Eric Auger
2021-10-26  9:47     ` Jean-Philippe Brucker
2021-10-21  9:02   ` Eric Auger
2021-10-20 17:27 ` [PATCH v5 11/12] tests/acpi: add expected blob for VIOT test on virt machine Jean-Philippe Brucker
2021-10-20 17:27 ` [PATCH v5 12/12] tests/acpi: add expected blobs for VIOT test on q35 machine Jean-Philippe Brucker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).