[RFC PATCH 0/4] hw/arm/virt-acpi-build: Introduce iommu option for pci root bus

[RFC PATCH 0/4] hw/arm/virt-acpi-build: Introduce iommu option for pci root bus

[Wang Xingang]

From: Xingang Wang <wangxingang5@huawei.com>

These patches add support for configure iommu on/off for pci root bus,
including primary bus and pxb root bus. At present, All root bus will go
through iommu when iommu is configured, which is not flexible.

So this add option to enable/disable iommu for primary bus and pxb root bus.
When iommu is enabled for the root bus, devices attached to it will go
through iommu. When iommu is disabled for the root bus, devices will not
go through iommu accordingly.

Xingang Wang (4):
  pci: Add PCI_BUS_IOMMU property
  hw/pci: Add iommu option for pci root bus
  hw/pci: Add pci_root_bus_max_bus
  hw/arm/virt-acpi-build: Add explicit idmap info in IORT table

 hw/arm/virt-acpi-build.c            | 88 +++++++++++++++++++++--------
 hw/arm/virt.c                       | 29 ++++++++++
 hw/pci-bridge/pci_expander_bridge.c |  6 ++
 hw/pci/pci.c                        | 35 +++++++++++-
 include/hw/arm/virt.h               |  1 +
 include/hw/pci/pci.h                |  1 +
 include/hw/pci/pci_bus.h            | 13 +++++
 7 files changed, 149 insertions(+), 24 deletions(-)

-- 
2.19.1

[RFC PATCH 1/4] pci: Add PCI_BUS_IOMMU property

[Wang Xingang]

From: Xingang Wang <wangxingang5@huawei.com>

This property can be useful to check whether this bus is attached to iommu.

Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
---
 include/hw/pci/pci_bus.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 347440d42c..42109e8a06 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -24,6 +24,8 @@ enum PCIBusFlags {
     PCI_BUS_IS_ROOT                                         = 0x0001,
     /* PCIe extended configuration space is accessible on this bus */
     PCI_BUS_EXTENDED_CONFIG_SPACE                           = 0x0002,
+    /* Iommu is enabled on this bus */
+    PCI_BUS_IOMMU                                           = 0x0004,
 };
 
 struct PCIBus {
@@ -63,4 +65,15 @@ static inline bool pci_bus_allows_extended_config_space(PCIBus *bus)
     return !!(bus->flags & PCI_BUS_EXTENDED_CONFIG_SPACE);
 }
 
+static inline bool pci_bus_has_iommu(PCIBus *bus)
+{
+    PCIBus *root_bus = bus;
+
+    while (root_bus && !pci_bus_is_root(root_bus)) {
+        root_bus = pci_get_bus(root_bus->parent_dev);
+    }
+
+    return !!(root_bus->flags & PCI_BUS_IOMMU);
+}
+
 #endif /* QEMU_PCI_BUS_H */
-- 
2.19.1

[RFC PATCH 2/4] hw/pci: Add iommu option for pci root bus

[Wang Xingang]

From: Xingang Wang <wangxingang5@huawei.com>

This add iommu option for pci root bus, including primary bus
and pxb root bus. Default option is set to true, and the option
is valid only if the iommu option for machine is properly set.

Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
---
 hw/arm/virt.c                       | 29 +++++++++++++++++++++++++++++
 hw/pci-bridge/pci_expander_bridge.c |  6 ++++++
 hw/pci/pci.c                        |  2 +-
 include/hw/arm/virt.h               |  1 +
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 371147f3ae..0c9e549759 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -79,6 +79,7 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
 #include "qemu/guest-random.h"
+#include "include/hw/pci/pci_bus.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1232,6 +1233,10 @@ static void create_smmu(const VirtMachineState *vms,
 
     dev = qdev_new("arm-smmuv3");
 
+    if (vms->primary_bus_iommu) {
+        bus->flags |= PCI_BUS_IOMMU;
+    }
+
     object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus),
                              &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
@@ -2305,6 +2310,20 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
     }
 }
 
+static bool virt_get_primary_bus_iommu(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->primary_bus_iommu;
+}
+
+static void virt_set_primary_bus_iommu(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->primary_bus_iommu = value;
+}
+
 static CpuInstanceProperties
 virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 {
@@ -2629,6 +2648,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
                                           "Set the IOMMU type. "
                                           "Valid values are none and smmuv3");
 
+    object_class_property_add_bool(oc, "primary_bus_iommu",
+                                  virt_get_primary_bus_iommu,
+                                  virt_set_primary_bus_iommu);
+    object_class_property_set_description(oc, "primary_bus_iommu",
+                                          "Set on/off to enable/disable "
+                                          "iommu for primary bus");
+
     object_class_property_add_bool(oc, "ras", virt_get_ras,
                                    virt_set_ras);
     object_class_property_set_description(oc, "ras",
@@ -2696,6 +2722,9 @@ static void virt_instance_init(Object *obj)
     /* Default disallows iommu instantiation */
     vms->iommu = VIRT_IOMMU_NONE;
 
+    /* Iommu is enabled by default for primary bus */
+    vms->primary_bus_iommu = true;
+
     /* Default disallows RAS instantiation */
     vms->ras = false;
 
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index aedded1064..7971ce9bd9 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -57,6 +57,7 @@ struct PXBDev {
 
     uint8_t bus_nr;
     uint16_t numa_node;
+    bool iommu;
 };
 
 static PXBDev *convert_to_pxb(PCIDevice *dev)
@@ -254,6 +255,10 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
     bus->address_space_io = pci_get_bus(dev)->address_space_io;
     bus->map_irq = pxb_map_irq_fn;
 
+    if (pxb->iommu) {
+       bus->flags |= PCI_BUS_IOMMU;
+    }
+
     PCI_HOST_BRIDGE(ds)->bus = bus;
 
     pxb_register_bus(dev, bus, &local_err);
@@ -301,6 +306,7 @@ static Property pxb_dev_properties[] = {
     /* Note: 0 is not a legal PXB bus number. */
     DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
     DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, NUMA_NODE_UNASSIGNED),
+    DEFINE_PROP_BOOL("iommu", PXBDev, iommu, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a9ebef8a35..dc969989c9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2712,7 +2712,7 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
 
         iommu_bus = parent_bus;
     }
-    if (iommu_bus && iommu_bus->iommu_fn) {
+    if (pci_bus_has_iommu(bus) && iommu_bus && iommu_bus->iommu_fn) {
         return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
     }
     return &address_space_memory;
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index ee9a93101e..babe829486 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -147,6 +147,7 @@ struct VirtMachineState {
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
+    bool primary_bus_iommu;
     VirtMSIControllerType msi_controller;
     uint16_t virtio_iommu_bdf;
     struct arm_boot_info bootinfo;
-- 
2.19.1

[RFC PATCH 3/4] hw/pci: Add pci_root_bus_max_bus

[Wang Xingang]

From: Xingang Wang <wangxingang5@huawei.com>

This helps to find max bus number of a root bus.

Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
---
 hw/pci/pci.c         | 33 +++++++++++++++++++++++++++++++++
 include/hw/pci/pci.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index dc969989c9..ed92ce0971 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -516,6 +516,39 @@ int pci_bus_num(PCIBus *s)
     return PCI_BUS_GET_CLASS(s)->bus_num(s);
 }
 
+int pci_root_bus_max_bus(PCIBus *bus)
+{
+    PCIHostState *host;
+    int max_bus = 0;
+    int type;
+    int devfn;
+
+    if (!pci_bus_is_root(bus)) {
+        return 0;
+    }
+
+    host = PCI_HOST_BRIDGE(BUS(bus)->parent);
+    max_bus = pci_bus_num(host->bus);
+
+    for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
+        PCIDevice *dev = host->bus->devices[devfn];
+
+        if (!dev) {
+            continue;
+        }
+
+        type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+        if (type == PCI_HEADER_TYPE_BRIDGE) {
+            uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS];
+            if (subordinate > max_bus) {
+                max_bus = subordinate;
+            }
+        }
+    }
+
+    return max_bus;
+}
+
 int pci_bus_numa_node(PCIBus *bus)
 {
     return PCI_BUS_GET_CLASS(bus)->numa_node(bus);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 1bc231480f..238b91817a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -449,6 +449,7 @@ static inline PCIBus *pci_get_bus(const PCIDevice *dev)
     return PCI_BUS(qdev_get_parent_bus(DEVICE(dev)));
 }
 int pci_bus_num(PCIBus *s);
+int pci_root_bus_max_bus(PCIBus *bus);
 static inline int pci_dev_bus_num(const PCIDevice *dev)
 {
     return pci_bus_num(pci_get_bus(dev));
-- 
2.19.1

[RFC PATCH 4/4] hw/arm/virt-acpi-build: Add explicit idmap info in IORT table

[Wang Xingang]

From: Xingang Wang <wangxingang5@huawei.com>

The idmap of smmuv3 and root complex covers the whole RID space for now,
this patch add explicit idmap info according to root bus number range.
This add smmuv3 idmap for certain bus which has enabled the iommu property.

Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
---
 hw/arm/virt-acpi-build.c | 88 +++++++++++++++++++++++++++++-----------
 1 file changed, 65 insertions(+), 23 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f9c9df916c..8bb8b251d0 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -54,6 +54,7 @@
 #include "kvm_arm.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/ghes.h"
+#include "hw/pci/pci_bus.h"
 
 #define ARM_SPI_BASE 32
 
@@ -247,9 +248,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     AcpiIortSmmu3 *smmu;
     size_t node_size, iort_node_offset, iort_length, smmu_offset = 0;
     AcpiIortRC *rc;
+    PCIBus *bus = vms->bus;
+    GArray *root_bus_array;
+    size_t root_bus_count = 0;
+    size_t root_bus_smmu_count = 0;
+    int bus_num, max_bus, index;
+
+    root_bus_array = g_array_new(false, true, sizeof(PCIBus *));
 
     iort = acpi_data_push(table_data, sizeof(*iort));
 
+    g_array_append_val(root_bus_array, bus);
+    root_bus_count++;
+    if (vms->iommu == VIRT_IOMMU_SMMUV3 && pci_bus_has_iommu(bus)) {
+        root_bus_smmu_count++;
+    }
+
+    QLIST_FOREACH(bus, &bus->child, sibling) {
+
+        if (!pci_bus_is_root(bus)) continue;
+
+        g_array_append_val(root_bus_array, bus);
+        root_bus_count++;
+
+        if (vms->iommu == VIRT_IOMMU_SMMUV3 && pci_bus_has_iommu(bus)) {
+            root_bus_smmu_count++;
+        }
+    }
+
     if (vms->iommu == VIRT_IOMMU_SMMUV3) {
         nb_nodes = 3; /* RC, ITS, SMMUv3 */
     } else {
@@ -280,13 +306,13 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 
         /* SMMUv3 node */
         smmu_offset = iort_node_offset + node_size;
-        node_size = sizeof(*smmu) + sizeof(*idmap);
+        node_size = sizeof(*smmu) + sizeof(*idmap) * root_bus_smmu_count;
         iort_length += node_size;
         smmu = acpi_data_push(table_data, node_size);
 
         smmu->type = ACPI_IORT_NODE_SMMU_V3;
         smmu->length = cpu_to_le16(node_size);
-        smmu->mapping_count = cpu_to_le32(1);
+        smmu->mapping_count = cpu_to_le32(root_bus_smmu_count);
         smmu->mapping_offset = cpu_to_le32(sizeof(*smmu));
         smmu->base_address = cpu_to_le64(vms->memmap[VIRT_SMMU].base);
         smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
@@ -295,23 +321,32 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         smmu->gerr_gsiv = cpu_to_le32(irq + 2);
         smmu->sync_gsiv = cpu_to_le32(irq + 3);
 
-        /* Identity RID mapping covering the whole input RID range */
-        idmap = &smmu->id_mapping_array[0];
-        idmap->input_base = 0;
-        idmap->id_count = cpu_to_le32(0xFFFF);
-        idmap->output_base = 0;
-        /* output IORT node is the ITS group node (the first node) */
-        idmap->output_reference = cpu_to_le32(iort_node_offset);
+        index = 0;
+        for (int i = 0; i < root_bus_count; i++) {
+            bus = g_array_index(root_bus_array, PCIBus *, i);
+
+            if (!pci_bus_has_iommu(bus)) continue;
+
+            bus_num = pci_bus_num(bus);
+            max_bus = pci_root_bus_max_bus(bus);
+
+            idmap = &smmu->id_mapping_array[index++];
+            idmap->input_base = cpu_to_le32(bus_num << 8);
+            idmap->id_count = cpu_to_le32((max_bus - bus_num + 1) << 8);
+            idmap->output_base = cpu_to_le32(bus_num << 8);
+            /* output IORT node is the ITS group node (the first node) */
+            idmap->output_reference = cpu_to_le32(iort_node_offset);
+        }
     }
 
     /* Root Complex Node */
-    node_size = sizeof(*rc) + sizeof(*idmap);
+    node_size = sizeof(*rc) + sizeof(*idmap) * root_bus_count;
     iort_length += node_size;
     rc = acpi_data_push(table_data, node_size);
 
     rc->type = ACPI_IORT_NODE_PCI_ROOT_COMPLEX;
     rc->length = cpu_to_le16(node_size);
-    rc->mapping_count = cpu_to_le32(1);
+    rc->mapping_count = cpu_to_le32(root_bus_count);
     rc->mapping_offset = cpu_to_le32(sizeof(*rc));
 
     /* fully coherent device */
@@ -319,18 +354,23 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     rc->memory_properties.memory_flags = 0x3; /* CCA = CPM = DCAS = 1 */
     rc->pci_segment_number = 0; /* MCFG pci_segment */
 
-    /* Identity RID mapping covering the whole input RID range */
-    idmap = &rc->id_mapping_array[0];
-    idmap->input_base = 0;
-    idmap->id_count = cpu_to_le32(0xFFFF);
-    idmap->output_base = 0;
-
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
-        /* output IORT node is the smmuv3 node */
-        idmap->output_reference = cpu_to_le32(smmu_offset);
-    } else {
-        /* output IORT node is the ITS group node (the first node) */
-        idmap->output_reference = cpu_to_le32(iort_node_offset);
+    for (int i = 0; i < root_bus_count; i++) {
+        bus = g_array_index(root_bus_array, PCIBus *, i);
+        bus_num = pci_bus_num(bus);
+        max_bus = pci_root_bus_max_bus(bus);
+
+        idmap = &rc->id_mapping_array[i];
+        idmap->input_base = cpu_to_le32(bus_num << 8);
+        idmap->id_count = cpu_to_le32((max_bus - bus_num + 1) << 8);
+        idmap->output_base = cpu_to_le32(bus_num << 8);
+
+        if (vms->iommu == VIRT_IOMMU_SMMUV3 && pci_bus_has_iommu(bus)) {
+            /* output IORT node is the smmuv3 node */
+            idmap->output_reference = cpu_to_le32(smmu_offset);
+        } else {
+            /* output IORT node is the ITS group node (the first node) */
+            idmap->output_reference = cpu_to_le32(iort_node_offset);
+        }
     }
 
     /*
@@ -343,6 +383,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     build_header(linker, table_data, (void *)(table_data->data + iort_start),
                  "IORT", table_data->len - iort_start, 0, vms->oem_id,
                  vms->oem_table_id);
+
+    g_array_free(root_bus_array, true);
 }
 
 static void
-- 
2.19.1

Re: [RFC PATCH 0/4] hw/arm/virt-acpi-build: Introduce iommu option for pci root bus

[no-reply]

Patchew URL: https://patchew.org/QEMU/1614329353-2124-1-git-send-email-wangxingang5@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1614329353-2124-1-git-send-email-wangxingang5@huawei.com
Subject: [RFC PATCH 0/4] hw/arm/virt-acpi-build: Introduce iommu option for pci root bus

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1614329353-2124-1-git-send-email-wangxingang5@huawei.com -> patchew/1614329353-2124-1-git-send-email-wangxingang5@huawei.com
Switched to a new branch 'test'
baed0b1 hw/arm/virt-acpi-build: Add explicit idmap info in IORT table
bf70832 hw/pci: Add pci_root_bus_max_bus
ddfb0cc hw/pci: Add iommu option for pci root bus
ab317df pci: Add PCI_BUS_IOMMU property

=== OUTPUT BEGIN ===
1/4 Checking commit ab317dff3b76 (pci: Add PCI_BUS_IOMMU property)
2/4 Checking commit ddfb0ccbed8b (hw/pci: Add iommu option for pci root bus)
ERROR: suspect code indent for conditional statements (4, 7)
#105: FILE: hw/pci-bridge/pci_expander_bridge.c:258:
+    if (pxb->iommu) {
+       bus->flags |= PCI_BUS_IOMMU;

total: 1 errors, 0 warnings, 98 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/4 Checking commit bf70832f4742 (hw/pci: Add pci_root_bus_max_bus)
4/4 Checking commit baed0b179ccc (hw/arm/virt-acpi-build: Add explicit idmap info in IORT table)
ERROR: trailing statements should be on next line
#52: FILE: hw/arm/virt-acpi-build.c:269:
+        if (!pci_bus_is_root(bus)) continue;

ERROR: braces {} are necessary for all arms of this statement
#52: FILE: hw/arm/virt-acpi-build.c:269:
+        if (!pci_bus_is_root(bus)) continue;
[...]

ERROR: trailing statements should be on next line
#96: FILE: hw/arm/virt-acpi-build.c:328:
+            if (!pci_bus_has_iommu(bus)) continue;

ERROR: braces {} are necessary for all arms of this statement
#96: FILE: hw/arm/virt-acpi-build.c:328:
+            if (!pci_bus_has_iommu(bus)) continue;
[...]

total: 4 errors, 0 warnings, 139 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1614329353-2124-1-git-send-email-wangxingang5@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com