Currently, the ITS Group nodes in the IORT table and the GIC ITS Struct
in the MADT table are always generated, even if GIC ITS is not available
on the machine.
This commit fixes it by not generating the ITS Group nodes, not mapping
any other node to them, and not advertising the GIC ITS in the MADT
table, when GIC ITS is not available on the machine.
Since the fix changes the MADT and IORT tables, add the blobs for the
"its=off" test to the allow list and update them in the next commit.
This commit also renames the smmu_idmaps and its_idmaps variables in
build_iort() to rc_smmu_idmaps and rc_its_idmaps, respectively, to make
it clearer which nodes are involved in the mappings associated with
these variables.
Reported-by: Udo Steinberg <udo@hypervisor.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2886
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Co-authored-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/virt-acpi-build.c | 142 ++++++++++++--------
tests/qtest/bios-tables-test-allowed-diff.h | 2 +
2 files changed, 90 insertions(+), 54 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 068383f982..eff0d698df 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -267,7 +267,7 @@ static int iort_idmap_compare(gconstpointer a, gconstpointer b)
}
/* Compute ID ranges (RIDs) from RC that are directed to the ITS Group node */
-static void create_its_idmaps(GArray *its_idmaps, GArray *smmu_idmaps)
+static void create_rc_its_idmaps(GArray *its_idmaps, GArray *smmu_idmaps)
{
AcpiIortIdMapping *idmap;
AcpiIortIdMapping next_range = {0};
@@ -314,8 +314,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
int i, nb_nodes, rc_mapping_count;
size_t node_size, smmu_offset = 0;
uint32_t id = 0;
- GArray *smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
- GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
+ GArray *rc_smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
+ GArray *rc_its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
.oem_table_id = vms->oem_table_id };
@@ -324,22 +324,38 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
if (vms->iommu == VIRT_IOMMU_SMMUV3) {
object_child_foreach_recursive(object_get_root(),
- iort_host_bridges, smmu_idmaps);
+ iort_host_bridges, rc_smmu_idmaps);
/* Sort the smmu idmap by input_base */
- g_array_sort(smmu_idmaps, iort_idmap_compare);
+ g_array_sort(rc_smmu_idmaps, iort_idmap_compare);
/*
* Knowing the ID ranges from the RC to the SMMU, it's possible to
* determine the ID ranges from RC that are directed to the ITS.
*/
- create_its_idmaps(its_idmaps, smmu_idmaps);
+ create_rc_its_idmaps(rc_its_idmaps, rc_smmu_idmaps);
- nb_nodes = 3; /* RC, ITS, SMMUv3 */
- rc_mapping_count = smmu_idmaps->len + its_idmaps->len;
+ nb_nodes = 2; /* RC and SMMUv3 */
+ rc_mapping_count = rc_smmu_idmaps->len;
+
+ if (vms->its) {
+ /*
+ * Knowing the ID ranges from the RC to the SMMU, it's possible to
+ * determine the ID ranges from RC that go directly to ITS.
+ */
+ create_rc_its_idmaps(rc_its_idmaps, rc_smmu_idmaps);
+
+ nb_nodes++; /* ITS */
+ rc_mapping_count += rc_its_idmaps->len;
+ }
} else {
- nb_nodes = 2; /* RC, ITS */
- rc_mapping_count = 1;
+ if (vms->its) {
+ nb_nodes = 2; /* RC and ITS */
+ rc_mapping_count = 1; /* Direct map to ITS */
+ } else {
+ nb_nodes = 1; /* RC only */
+ rc_mapping_count = 0; /* No output mapping */
+ }
}
/* Number of IORT Nodes */
build_append_int_noprefix(table_data, nb_nodes, 4);
@@ -348,31 +364,43 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
build_append_int_noprefix(table_data, IORT_NODE_OFFSET, 4);
build_append_int_noprefix(table_data, 0, 4); /* Reserved */
- /* Table 12 ITS Group Format */
- build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type */
- node_size = 20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
- build_append_int_noprefix(table_data, node_size, 2); /* Length */
- build_append_int_noprefix(table_data, 1, 1); /* Revision */
- build_append_int_noprefix(table_data, id++, 4); /* Identifier */
- build_append_int_noprefix(table_data, 0, 4); /* Number of ID mappings */
- build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array */
- build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
- /* GIC ITS Identifier Array */
- build_append_int_noprefix(table_data, 0 /* MADT translation_id */, 4);
+ if (vms->its) {
+ /* Table 12 ITS Group Format */
+ build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type */
+ node_size = 20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
+ build_append_int_noprefix(table_data, node_size, 2); /* Length */
+ build_append_int_noprefix(table_data, 1, 1); /* Revision */
+ build_append_int_noprefix(table_data, id++, 4); /* Identifier */
+ build_append_int_noprefix(table_data, 0, 4); /* Number of ID mappings */
+ build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array */
+ build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
+ /* GIC ITS Identifier Array */
+ build_append_int_noprefix(table_data, 0 /* MADT translation_id */, 4);
+ }
if (vms->iommu == VIRT_IOMMU_SMMUV3) {
int irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
-
+ int smmu_mapping_count, offset_to_id_array;
+
+ if (vms->its) {
+ smmu_mapping_count = 1; /* ITS Group node */
+ offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
+ } else {
+ smmu_mapping_count = 0; /* No ID mappings */
+ offset_to_id_array = 0; /* No ID mappings array */
+ }
smmu_offset = table_data->len - table.table_offset;
/* Table 9 SMMUv3 Format */
build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type */
- node_size = SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
+ node_size = SMMU_V3_ENTRY_SIZE +
+ (ID_MAPPING_ENTRY_SIZE * smmu_mapping_count);
build_append_int_noprefix(table_data, node_size, 2); /* Length */
build_append_int_noprefix(table_data, 4, 1); /* Revision */
build_append_int_noprefix(table_data, id++, 4); /* Identifier */
- build_append_int_noprefix(table_data, 1, 4); /* Number of ID mappings */
+ /* Number of ID mappings */
+ build_append_int_noprefix(table_data, smmu_mapping_count, 4);
/* Reference to ID Array */
- build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
+ build_append_int_noprefix(table_data, offset_to_id_array, 4);
/* Base address */
build_append_int_noprefix(table_data, vms->memmap[VIRT_SMMU].base, 8);
/* Flags */
@@ -388,9 +416,11 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
build_append_int_noprefix(table_data, 0, 4); /* Proximity domain */
/* DeviceID mapping index (ignored since interrupts are GSIV based) */
build_append_int_noprefix(table_data, 0, 4);
-
- /* Output IORT node is the ITS Group node (the first node) */
- build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
+ /* Array of ID mappings */
+ if (smmu_mapping_count) {
+ /* Output IORT node is the ITS Group node (the first node). */
+ build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
+ }
}
/* Table 17 Root Complex Node */
@@ -431,24 +461,26 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
*
* N.B.: The mapping from SMMUv3 to ITS Group node (SMMUv3 -> ITS) is
* defined in the SMMUv3 table, where all SMMUv3 IDs are mapped to the
- * ITS Group node.
+ * ITS Group node, if ITS is available.
*/
- for (i = 0; i < smmu_idmaps->len; i++) {
- range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
+ for (i = 0; i < rc_smmu_idmaps->len; i++) {
+ range = &g_array_index(rc_smmu_idmaps, AcpiIortIdMapping, i);
/* Output IORT node is the SMMUv3 node. */
build_iort_id_mapping(table_data, range->input_base,
range->id_count, smmu_offset);
}
- /*
- * Map bypassed (don't go throught the SMMU) RIDs (input) to ITS Group
- * node directly: RC -> ITS.
- */
- for (i = 0; i < its_idmaps->len; i++) {
- range = &g_array_index(its_idmaps, AcpiIortIdMapping, i);
- /* Output IORT node is the ITS Group node (the first node). */
- build_iort_id_mapping(table_data, range->input_base,
- range->id_count, IORT_NODE_OFFSET);
+ if (vms->its) {
+ /*
+ * Map bypassed (don't go throught the SMMU) RIDs (input) to ITS Group
+ * node directly: RC -> ITS.
+ */
+ for (i = 0; i < rc_its_idmaps->len; i++) {
+ range = &g_array_index(rc_its_idmaps, AcpiIortIdMapping, i);
+ /* Output IORT node is the ITS Group node (the first node). */
+ build_iort_id_mapping(table_data, range->input_base,
+ range->id_count, IORT_NODE_OFFSET);
+ }
}
} else {
/*
@@ -460,8 +492,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
acpi_table_end(linker, &table);
- g_array_free(smmu_idmaps, true);
- g_array_free(its_idmaps, true);
+ g_array_free(rc_smmu_idmaps, true);
+ g_array_free(rc_its_idmaps, true);
}
/*
@@ -769,18 +801,20 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
memmap[VIRT_HIGH_GIC_REDIST2].size);
}
- /*
- * ACPI spec, Revision 6.0 Errata A
- * (original 6.0 definition has invalid Length)
- * 5.2.12.18 GIC ITS Structure
- */
- build_append_int_noprefix(table_data, 0xF, 1); /* Type */
- build_append_int_noprefix(table_data, 20, 1); /* Length */
- build_append_int_noprefix(table_data, 0, 2); /* Reserved */
- build_append_int_noprefix(table_data, 0, 4); /* GIC ITS ID */
- /* Physical Base Address */
- build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
- build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+ if (vms->its) {
+ /*
+ * ACPI spec, Revision 6.0 Errata A
+ * (original 6.0 definition has invalid Length)
+ * 5.2.12.18 GIC ITS Structure
+ */
+ build_append_int_noprefix(table_data, 0xF, 1); /* Type */
+ build_append_int_noprefix(table_data, 20, 1); /* Length */
+ build_append_int_noprefix(table_data, 0, 2); /* Reserved */
+ build_append_int_noprefix(table_data, 0, 4); /* GIC ITS ID */
+ /* Physical Base Address */
+ build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
+ build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+ }
} else {
const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..a88198d5c2 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/aarch64/virt/APIC.its_off",
+"tests/data/acpi/aarch64/virt/IORT.its_off",
--
2.34.1
Hi Gustavo,
> -----Original Message-----
> From: qemu-devel-
> bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org <qemu-
> devel-bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org> On
> Behalf Of Gustavo Romero
> Sent: Saturday, June 28, 2025 8:57 PM
> To: qemu-devel@nongnu.org; eric.auger@redhat.com; philmd@linaro.org;
> mst@redhat.com
> Cc: qemu-arm@nongnu.org; alex.bennee@linaro.org;
> gustavo.romero@linaro.org; udo@hypervisor.org;
> ajones@ventanamicro.com; peter.maydell@linaro.org;
> imammedo@redhat.com; anisinha@redhat.com
> Subject: [PATCH v6 8/9] hw/arm/virt-acpi-build: Fix ACPI IORT and MADT
> tables when its=off
>
> Currently, the ITS Group nodes in the IORT table and the GIC ITS Struct
> in the MADT table are always generated, even if GIC ITS is not available
> on the machine.
>
> This commit fixes it by not generating the ITS Group nodes, not mapping
> any other node to them, and not advertising the GIC ITS in the MADT
> table, when GIC ITS is not available on the machine.
>
> Since the fix changes the MADT and IORT tables, add the blobs for the
> "its=off" test to the allow list and update them in the next commit.
>
> This commit also renames the smmu_idmaps and its_idmaps variables in
> build_iort() to rc_smmu_idmaps and rc_its_idmaps, respectively, to make
> it clearer which nodes are involved in the mappings associated with
> these variables.
>
> Reported-by: Udo Steinberg <udo@hypervisor.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2886
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Co-authored-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/virt-acpi-build.c | 142 ++++++++++++--------
> tests/qtest/bios-tables-test-allowed-diff.h | 2 +
> 2 files changed, 90 insertions(+), 54 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 068383f982..eff0d698df 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -267,7 +267,7 @@ static int iort_idmap_compare(gconstpointer a,
> gconstpointer b)
> }
>
> /* Compute ID ranges (RIDs) from RC that are directed to the ITS Group
> node */
> -static void create_its_idmaps(GArray *its_idmaps, GArray *smmu_idmaps)
> +static void create_rc_its_idmaps(GArray *its_idmaps, GArray
> *smmu_idmaps)
> {
> AcpiIortIdMapping *idmap;
> AcpiIortIdMapping next_range = {0};
> @@ -314,8 +314,8 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> int i, nb_nodes, rc_mapping_count;
> size_t node_size, smmu_offset = 0;
> uint32_t id = 0;
> - GArray *smmu_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> - GArray *its_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> + GArray *rc_smmu_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> + GArray *rc_its_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
>
> AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
> .oem_table_id = vms->oem_table_id };
> @@ -324,22 +324,38 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
>
> if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> object_child_foreach_recursive(object_get_root(),
> - iort_host_bridges, smmu_idmaps);
> + iort_host_bridges, rc_smmu_idmaps);
>
> /* Sort the smmu idmap by input_base */
> - g_array_sort(smmu_idmaps, iort_idmap_compare);
> + g_array_sort(rc_smmu_idmaps, iort_idmap_compare);
>
> /*
> * Knowing the ID ranges from the RC to the SMMU, it's possible to
> * determine the ID ranges from RC that are directed to the ITS.
> */
> - create_its_idmaps(its_idmaps, smmu_idmaps);
> + create_rc_its_idmaps(rc_its_idmaps, rc_smmu_idmaps);
Hmm...not sure why we still need the above now as this is being moved down
for vms->its is set case.
I had a look at v5, which seems to be doing the right thing.
https://lore.kernel.org/qemu-devel/20250623135749.691137-9-gustavo.romero@linaro.org/
Or am I missing something here?
Please let me know.
Thanks,
Shameer
>
> - nb_nodes = 3; /* RC, ITS, SMMUv3 */
> - rc_mapping_count = smmu_idmaps->len + its_idmaps->len;
> + nb_nodes = 2; /* RC and SMMUv3 */
> + rc_mapping_count = rc_smmu_idmaps->len;
> +
> + if (vms->its) {
> + /*
> + * Knowing the ID ranges from the RC to the SMMU, it's possible to
> + * determine the ID ranges from RC that go directly to ITS.
> + */
> + create_rc_its_idmaps(rc_its_idmaps, rc_smmu_idmaps);
> +
> + nb_nodes++; /* ITS */
> + rc_mapping_count += rc_its_idmaps->len;
> + }
> } else {
> - nb_nodes = 2; /* RC, ITS */
> - rc_mapping_count = 1;
> + if (vms->its) {
> + nb_nodes = 2; /* RC and ITS */
> + rc_mapping_count = 1; /* Direct map to ITS */
> + } else {
> + nb_nodes = 1; /* RC only */
> + rc_mapping_count = 0; /* No output mapping */
> + }
> }
> /* Number of IORT Nodes */
> build_append_int_noprefix(table_data, nb_nodes, 4);
> @@ -348,31 +364,43 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> build_append_int_noprefix(table_data, IORT_NODE_OFFSET, 4);
> build_append_int_noprefix(table_data, 0, 4); /* Reserved */
>
> - /* Table 12 ITS Group Format */
> - build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type */
> - node_size = 20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
> - build_append_int_noprefix(table_data, node_size, 2); /* Length */
> - build_append_int_noprefix(table_data, 1, 1); /* Revision */
> - build_append_int_noprefix(table_data, id++, 4); /* Identifier */
> - build_append_int_noprefix(table_data, 0, 4); /* Number of ID mappings
> */
> - build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array */
> - build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
> - /* GIC ITS Identifier Array */
> - build_append_int_noprefix(table_data, 0 /* MADT translation_id */, 4);
> + if (vms->its) {
> + /* Table 12 ITS Group Format */
> + build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type
> */
> + node_size = 20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
> + build_append_int_noprefix(table_data, node_size, 2); /* Length */
> + build_append_int_noprefix(table_data, 1, 1); /* Revision */
> + build_append_int_noprefix(table_data, id++, 4); /* Identifier */
> + build_append_int_noprefix(table_data, 0, 4); /* Number of ID
> mappings */
> + build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array
> */
> + build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
> + /* GIC ITS Identifier Array */
> + build_append_int_noprefix(table_data, 0 /* MADT translation_id */,
> 4);
> + }
>
> if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> int irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
> -
> + int smmu_mapping_count, offset_to_id_array;
> +
> + if (vms->its) {
> + smmu_mapping_count = 1; /* ITS Group node */
> + offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the
> header */
> + } else {
> + smmu_mapping_count = 0; /* No ID mappings */
> + offset_to_id_array = 0; /* No ID mappings array */
> + }
> smmu_offset = table_data->len - table.table_offset;
> /* Table 9 SMMUv3 Format */
> build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type
> */
> - node_size = SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
> + node_size = SMMU_V3_ENTRY_SIZE +
> + (ID_MAPPING_ENTRY_SIZE * smmu_mapping_count);
> build_append_int_noprefix(table_data, node_size, 2); /* Length */
> build_append_int_noprefix(table_data, 4, 1); /* Revision */
> build_append_int_noprefix(table_data, id++, 4); /* Identifier */
> - build_append_int_noprefix(table_data, 1, 4); /* Number of ID
> mappings */
> + /* Number of ID mappings */
> + build_append_int_noprefix(table_data, smmu_mapping_count, 4);
> /* Reference to ID Array */
> - build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
> + build_append_int_noprefix(table_data, offset_to_id_array, 4);
> /* Base address */
> build_append_int_noprefix(table_data, vms-
> >memmap[VIRT_SMMU].base, 8);
> /* Flags */
> @@ -388,9 +416,11 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> build_append_int_noprefix(table_data, 0, 4); /* Proximity domain */
> /* DeviceID mapping index (ignored since interrupts are GSIV based)
> */
> build_append_int_noprefix(table_data, 0, 4);
> -
> - /* Output IORT node is the ITS Group node (the first node) */
> - build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
> + /* Array of ID mappings */
> + if (smmu_mapping_count) {
> + /* Output IORT node is the ITS Group node (the first node). */
> + build_iort_id_mapping(table_data, 0, 0x10000,
> IORT_NODE_OFFSET);
> + }
> }
>
> /* Table 17 Root Complex Node */
> @@ -431,24 +461,26 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> *
> * N.B.: The mapping from SMMUv3 to ITS Group node (SMMUv3 ->
> ITS) is
> * defined in the SMMUv3 table, where all SMMUv3 IDs are mapped to
> the
> - * ITS Group node.
> + * ITS Group node, if ITS is available.
> */
> - for (i = 0; i < smmu_idmaps->len; i++) {
> - range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
> + for (i = 0; i < rc_smmu_idmaps->len; i++) {
> + range = &g_array_index(rc_smmu_idmaps, AcpiIortIdMapping, i);
> /* Output IORT node is the SMMUv3 node. */
> build_iort_id_mapping(table_data, range->input_base,
> range->id_count, smmu_offset);
> }
>
> - /*
> - * Map bypassed (don't go throught the SMMU) RIDs (input) to ITS
> Group
> - * node directly: RC -> ITS.
> - */
> - for (i = 0; i < its_idmaps->len; i++) {
> - range = &g_array_index(its_idmaps, AcpiIortIdMapping, i);
> - /* Output IORT node is the ITS Group node (the first node). */
> - build_iort_id_mapping(table_data, range->input_base,
> - range->id_count, IORT_NODE_OFFSET);
> + if (vms->its) {
> + /*
> + * Map bypassed (don't go throught the SMMU) RIDs (input) to ITS
> Group
> + * node directly: RC -> ITS.
> + */
> + for (i = 0; i < rc_its_idmaps->len; i++) {
> + range = &g_array_index(rc_its_idmaps, AcpiIortIdMapping, i);
> + /* Output IORT node is the ITS Group node (the first node). */
> + build_iort_id_mapping(table_data, range->input_base,
> + range->id_count, IORT_NODE_OFFSET);
> + }
> }
> } else {
> /*
> @@ -460,8 +492,8 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> }
>
> acpi_table_end(linker, &table);
> - g_array_free(smmu_idmaps, true);
> - g_array_free(its_idmaps, true);
> + g_array_free(rc_smmu_idmaps, true);
> + g_array_free(rc_its_idmaps, true);
> }
>
> /*
> @@ -769,18 +801,20 @@ build_madt(GArray *table_data, BIOSLinker
> *linker, VirtMachineState *vms)
> memmap[VIRT_HIGH_GIC_REDIST2].size);
> }
>
> - /*
> - * ACPI spec, Revision 6.0 Errata A
> - * (original 6.0 definition has invalid Length)
> - * 5.2.12.18 GIC ITS Structure
> - */
> - build_append_int_noprefix(table_data, 0xF, 1); /* Type */
> - build_append_int_noprefix(table_data, 20, 1); /* Length */
> - build_append_int_noprefix(table_data, 0, 2); /* Reserved */
> - build_append_int_noprefix(table_data, 0, 4); /* GIC ITS ID */
> - /* Physical Base Address */
> - build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base,
> 8);
> - build_append_int_noprefix(table_data, 0, 4); /* Reserved */
> + if (vms->its) {
> + /*
> + * ACPI spec, Revision 6.0 Errata A
> + * (original 6.0 definition has invalid Length)
> + * 5.2.12.18 GIC ITS Structure
> + */
> + build_append_int_noprefix(table_data, 0xF, 1); /* Type */
> + build_append_int_noprefix(table_data, 20, 1); /* Length */
> + build_append_int_noprefix(table_data, 0, 2); /* Reserved */
> + build_append_int_noprefix(table_data, 0, 4); /* GIC ITS ID */
> + /* Physical Base Address */
> + build_append_int_noprefix(table_data,
> memmap[VIRT_GIC_ITS].base, 8);
> + build_append_int_noprefix(table_data, 0, 4); /* Reserved */
> + }
> } else {
> const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] +
> ARM_SPI_BASE;
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-
> tables-test-allowed-diff.h
> index dfb8523c8b..a88198d5c2 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,3 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/aarch64/virt/APIC.its_off",
> +"tests/data/acpi/aarch64/virt/IORT.its_off",
> --
> 2.34.1
>
> -----Original Message-----
> From: Shameerali Kolothum Thodi
> Sent: Wednesday, July 2, 2025 9:34 AM
> To: 'Gustavo Romero' <gustavo.romero@linaro.org>; qemu-
> devel@nongnu.org; eric.auger@redhat.com; philmd@linaro.org;
> mst@redhat.com
> Cc: qemu-arm@nongnu.org; alex.bennee@linaro.org; udo@hypervisor.org;
> ajones@ventanamicro.com; peter.maydell@linaro.org;
> imammedo@redhat.com; anisinha@redhat.com
> Subject: RE: [PATCH v6 8/9] hw/arm/virt-acpi-build: Fix ACPI IORT and MADT
> tables when its=off
>
> Hi Gustavo,
>
> > -----Original Message-----
> > From: qemu-devel-
> > bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org <qemu-
> > devel-bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org>
> On
> > Behalf Of Gustavo Romero
> > Sent: Saturday, June 28, 2025 8:57 PM
> > To: qemu-devel@nongnu.org; eric.auger@redhat.com;
> philmd@linaro.org;
> > mst@redhat.com
> > Cc: qemu-arm@nongnu.org; alex.bennee@linaro.org;
> > gustavo.romero@linaro.org; udo@hypervisor.org;
> > ajones@ventanamicro.com; peter.maydell@linaro.org;
> > imammedo@redhat.com; anisinha@redhat.com
> > Subject: [PATCH v6 8/9] hw/arm/virt-acpi-build: Fix ACPI IORT and MADT
> > tables when its=off
> >
> > Currently, the ITS Group nodes in the IORT table and the GIC ITS Struct
> > in the MADT table are always generated, even if GIC ITS is not available
> > on the machine.
> >
> > This commit fixes it by not generating the ITS Group nodes, not mapping
> > any other node to them, and not advertising the GIC ITS in the MADT
> > table, when GIC ITS is not available on the machine.
> >
> > Since the fix changes the MADT and IORT tables, add the blobs for the
> > "its=off" test to the allow list and update them in the next commit.
> >
> > This commit also renames the smmu_idmaps and its_idmaps variables in
> > build_iort() to rc_smmu_idmaps and rc_its_idmaps, respectively, to make
> > it clearer which nodes are involved in the mappings associated with
> > these variables.
> >
> > Reported-by: Udo Steinberg <udo@hypervisor.org>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2886
> > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> > Co-authored-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > hw/arm/virt-acpi-build.c | 142 ++++++++++++--------
> > tests/qtest/bios-tables-test-allowed-diff.h | 2 +
> > 2 files changed, 90 insertions(+), 54 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 068383f982..eff0d698df 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -267,7 +267,7 @@ static int iort_idmap_compare(gconstpointer a,
> > gconstpointer b)
> > }
> >
> > /* Compute ID ranges (RIDs) from RC that are directed to the ITS Group
> > node */
> > -static void create_its_idmaps(GArray *its_idmaps, GArray
> *smmu_idmaps)
> > +static void create_rc_its_idmaps(GArray *its_idmaps, GArray
> > *smmu_idmaps)
> > {
> > AcpiIortIdMapping *idmap;
> > AcpiIortIdMapping next_range = {0};
> > @@ -314,8 +314,8 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> > VirtMachineState *vms)
> > int i, nb_nodes, rc_mapping_count;
> > size_t node_size, smmu_offset = 0;
> > uint32_t id = 0;
> > - GArray *smmu_idmaps = g_array_new(false, true,
> > sizeof(AcpiIortIdMapping));
> > - GArray *its_idmaps = g_array_new(false, true,
> > sizeof(AcpiIortIdMapping));
> > + GArray *rc_smmu_idmaps = g_array_new(false, true,
> > sizeof(AcpiIortIdMapping));
> > + GArray *rc_its_idmaps = g_array_new(false, true,
> > sizeof(AcpiIortIdMapping));
> >
> > AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
> > .oem_table_id = vms->oem_table_id };
> > @@ -324,22 +324,38 @@ build_iort(GArray *table_data, BIOSLinker
> *linker,
> > VirtMachineState *vms)
> >
> > if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> > object_child_foreach_recursive(object_get_root(),
> > - iort_host_bridges, smmu_idmaps);
> > + iort_host_bridges, rc_smmu_idmaps);
> >
> > /* Sort the smmu idmap by input_base */
> > - g_array_sort(smmu_idmaps, iort_idmap_compare);
> > + g_array_sort(rc_smmu_idmaps, iort_idmap_compare);
> >
> > /*
> > * Knowing the ID ranges from the RC to the SMMU, it's possible to
> > * determine the ID ranges from RC that are directed to the ITS.
> > */
> > - create_its_idmaps(its_idmaps, smmu_idmaps);
> > + create_rc_its_idmaps(rc_its_idmaps, rc_smmu_idmaps);
>
> Hmm...not sure why we still need the above now as this is being moved
> down
> for vms->its is set case.
>
> I had a look at v5, which seems to be doing the right thing.
> https://lore.kernel.org/qemu-devel/20250623135749.691137-9-
> gustavo.romero@linaro.org/
>
> Or am I missing something here?
I have included a fix for the above in my SMMUv3 dev series here,
https://lore.kernel.org/qemu-devel/20250703084643.85740-2-shameerali.kolothum.thodi@huawei.com/
Please take a look and let me know if it doesn't make sense.
Thanks,
Shameer
© 2016 - 2025 Red Hat, Inc.