Introduce a -M msi= argument to be able to control MSI-X support independently
from ITS, as part of supporting GICv3 + GICv2m platforms.
Remove vms->its as it's no longer needed after that change.
Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
---
hw/arm/virt-acpi-build.c | 24 ++++++---
hw/arm/virt.c | 108 +++++++++++++++++++++++++++++++--------
include/hw/arm/virt.h | 4 +-
3 files changed, 108 insertions(+), 28 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 86024a1a73..187dd4e272 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -440,6 +440,17 @@ static void create_rc_its_idmaps(GArray *its_idmaps, GArray *smmuv3_devs)
}
}
+/*
+ * In the prior Qemu ACPI table handling, GICv2 configurations
+ * had vms->its=1... That's broken.
+ *
+ * Match that assumption to match the existing ACPI tables that
+ * have been shipping for quite a while.
+ */
+static int is_gicv2_acpi_workaround_needed(VirtMachineState *vms) {
+ return vms->gic_version == 2;
+}
+
/*
* Input Output Remapping Table (IORT)
* Conforms to "IO Remapping Table System Software on ARM Platforms",
@@ -473,7 +484,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
nb_nodes = num_smmus + 1; /* RC and SMMUv3 */
rc_mapping_count = rc_smmu_idmaps_len;
- if (virt_is_its_enabled(vms)) {
+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
/*
* 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.
@@ -484,7 +495,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
rc_mapping_count += rc_its_idmaps->len;
}
} else {
- if (virt_is_its_enabled(vms)) {
+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
nb_nodes = 2; /* RC and ITS */
rc_mapping_count = 1; /* Direct map to ITS */
} else {
@@ -499,7 +510,7 @@ 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 */
- if (virt_is_its_enabled(vms)) {
+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
/* 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 */;
@@ -518,7 +529,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
int smmu_mapping_count, offset_to_id_array;
int irq = sdev->irq;
- if (virt_is_its_enabled(vms)) {
+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
smmu_mapping_count = 1; /* ITS Group node */
offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
} else {
@@ -611,7 +622,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
}
- if (virt_is_its_enabled(vms)) {
+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
/*
* Map bypassed (don't go through the SMMU) RIDs (input) to
* ITS Group node directly: RC -> ITS.
@@ -962,8 +973,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
}
- if (!(vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms))
- && !vms->no_gicv3_with_gicv2m) {
+ if (virt_is_gicv2m_enabled(vms)) {
const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
/* 5.2.12.16 GIC MSI Frame Structure */
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 553c7f62cc..0e84ccd82c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -966,12 +966,12 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
fdt_add_gic_node(vms);
- if (vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms)) {
+ if (virt_is_its_enabled(vms)) {
create_its(vms);
- } else if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->no_gicv3_with_gicv2m) {
- create_v2m(vms);
- } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
+ } else if (virt_is_gicv2m_enabled(vms)) {
create_v2m(vms);
+ } else {
+ vms->msi_controller = VIRT_MSI_CTRL_NONE;
}
}
@@ -2710,32 +2710,93 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,
bool virt_is_its_enabled(VirtMachineState *vms)
{
- if (vms->its == ON_OFF_AUTO_OFF) {
+ switch (vms->msi_controller) {
+ case VIRT_MSI_CTRL_NONE:
return false;
- }
- if (vms->its == ON_OFF_AUTO_AUTO) {
- if (whpx_enabled()) {
+ case VIRT_MSI_CTRL_ITS:
+ return true;
+ case VIRT_MSI_CTRL_GICV2M:
+ return false;
+ case VIRT_MSI_CTRL_AUTO:
+ if (whpx_enabled() && whpx_irqchip_in_kernel()) {
+ return false;
+ }
+ if (vms->gic_version == VIRT_GIC_VERSION_2) {
return false;
}
+ return true;
}
- return true;
}
-static void virt_get_its(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
+bool virt_is_gicv2m_enabled(VirtMachineState *vms)
+{
+ switch (vms->msi_controller) {
+ case VIRT_MSI_CTRL_NONE:
+ return false;
+ default:
+ return !virt_is_its_enabled(vms);
+ }
+}
+
+static char *virt_get_msi(Object *obj, Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(obj);
+ const char *val;
+
+ switch (vms->msi_controller) {
+ case VIRT_MSI_CTRL_NONE:
+ val = "off";
+ break;
+ case VIRT_MSI_CTRL_ITS:
+ val = "its";
+ break;
+ case VIRT_MSI_CTRL_GICV2M:
+ val = "gicv2m";
+ break;
+ case VIRT_MSI_CTRL_AUTO:
+ val = "auto";
+ break;
+ }
+ return g_strdup(val);
+}
+
+static void virt_set_msi(Object *obj, const char *value, Error **errp)
{
+ ERRP_GUARD();
VirtMachineState *vms = VIRT_MACHINE(obj);
- OnOffAuto its = vms->its;
- visit_type_OnOffAuto(v, name, &its, errp);
+ if (!strcmp(value, "auto")) {
+ vms->msi_controller = VIRT_MSI_CTRL_AUTO; /* Will be overriden later */
+ } else if (!strcmp(value, "its")) {
+ vms->msi_controller = VIRT_MSI_CTRL_ITS;
+ } else if (!strcmp(value, "gicv2m")) {
+ vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
+ } else if (!strcmp(value, "none")) {
+ vms->msi_controller = VIRT_MSI_CTRL_NONE;
+ } else {
+ error_setg(errp, "Invalid msi value");
+ error_append_hint(errp, "Valid values are auto, gicv2m, its, off\n");
+ }
}
-static void virt_set_its(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
+static bool virt_get_its(Object *obj, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(obj);
- visit_type_OnOffAuto(v, name, &vms->its, errp);
+ return virt_is_its_enabled(vms);
+}
+
+static void virt_set_its(Object *obj, bool value, Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(obj);
+
+ if (value) {
+ vms->msi_controller = VIRT_MSI_CTRL_ITS;
+ } else if (vms->no_gicv3_with_gicv2m) {
+ vms->msi_controller = VIRT_MSI_CTRL_NONE;
+ } else {
+ vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
+ }
}
static bool virt_get_dtb_randomness(Object *obj, Error **errp)
@@ -3062,6 +3123,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
db_start = base_memmap[VIRT_GIC_V2M].base;
db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
break;
+ case VIRT_MSI_CTRL_AUTO:
+ g_assert_not_reached();
}
resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
db_start, db_end,
@@ -3452,13 +3515,18 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
"guest CPU which implements the ARM "
"Memory Tagging Extension");
- object_class_property_add(oc, "its", "OnOffAuto",
- virt_get_its, virt_set_its,
- NULL, NULL);
+ object_class_property_add_bool(oc, "its", virt_get_its,
+ virt_set_its);
object_class_property_set_description(oc, "its",
"Set on/off to enable/disable "
"ITS instantiation");
+ object_class_property_add_str(oc, "msi", virt_get_msi,
+ virt_set_msi);
+ object_class_property_set_description(oc, "msi",
+ "Set MSI settings. "
+ "Valid values are auto/gicv2m/its/off");
+
object_class_property_add_bool(oc, "dtb-randomness",
virt_get_dtb_randomness,
virt_set_dtb_randomness);
@@ -3515,7 +3583,7 @@ static void virt_instance_init(Object *obj)
vms->highmem_redists = true;
/* Default allows ITS instantiation if available */
- vms->its = ON_OFF_AUTO_AUTO;
+ vms->msi_controller = VIRT_MSI_CTRL_AUTO;
/* Allow ITS emulation if the machine version supports it */
vms->tcg_its = !vmc->no_tcg_its;
vms->no_gicv3_with_gicv2m = false;
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 394b70c62e..ff43bcb739 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -101,6 +101,8 @@ typedef enum VirtIOMMUType {
typedef enum VirtMSIControllerType {
VIRT_MSI_CTRL_NONE,
+ /* This value is overriden at runtime.*/
+ VIRT_MSI_CTRL_AUTO,
VIRT_MSI_CTRL_GICV2M,
VIRT_MSI_CTRL_ITS,
} VirtMSIControllerType;
@@ -147,7 +149,6 @@ struct VirtMachineState {
bool highmem_ecam;
bool highmem_mmio;
bool highmem_redists;
- OnOffAuto its;
bool tcg_its;
bool virt;
bool ras;
@@ -217,5 +218,6 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
}
bool virt_is_its_enabled(VirtMachineState *vms);
+bool virt_is_gicv2m_enabled(VirtMachineState *vms);
#endif /* QEMU_ARM_VIRT_H */
--
2.50.1 (Apple Git-155)
Am 14. Januar 2026 13:41:24 UTC schrieb Mohamed Mediouni <mohamed@unpredictable.fr>:
>Introduce a -M msi= argument to be able to control MSI-X support independently
>from ITS, as part of supporting GICv3 + GICv2m platforms.
>
>Remove vms->its as it's no longer needed after that change.
>
>Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>---
> hw/arm/virt-acpi-build.c | 24 ++++++---
> hw/arm/virt.c | 108 +++++++++++++++++++++++++++++++--------
> include/hw/arm/virt.h | 4 +-
> 3 files changed, 108 insertions(+), 28 deletions(-)
>
>diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>index 86024a1a73..187dd4e272 100644
>--- a/hw/arm/virt-acpi-build.c
>+++ b/hw/arm/virt-acpi-build.c
>@@ -440,6 +440,17 @@ static void create_rc_its_idmaps(GArray *its_idmaps, GArray *smmuv3_devs)
> }
> }
>
>+/*
>+ * In the prior Qemu ACPI table handling, GICv2 configurations
>+ * had vms->its=1... That's broken.
>+ *
>+ * Match that assumption to match the existing ACPI tables that
>+ * have been shipping for quite a while.
>+ */
>+static int is_gicv2_acpi_workaround_needed(VirtMachineState *vms) {
>+ return vms->gic_version == 2;
>+}
>+
> /*
> * Input Output Remapping Table (IORT)
> * Conforms to "IO Remapping Table System Software on ARM Platforms",
>@@ -473,7 +484,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> nb_nodes = num_smmus + 1; /* RC and SMMUv3 */
> rc_mapping_count = rc_smmu_idmaps_len;
>
>- if (virt_is_its_enabled(vms)) {
>+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
> /*
> * 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.
>@@ -484,7 +495,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> rc_mapping_count += rc_its_idmaps->len;
> }
> } else {
>- if (virt_is_its_enabled(vms)) {
>+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
> nb_nodes = 2; /* RC and ITS */
> rc_mapping_count = 1; /* Direct map to ITS */
> } else {
>@@ -499,7 +510,7 @@ 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 */
>
>- if (virt_is_its_enabled(vms)) {
>+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
> /* 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 */;
>@@ -518,7 +529,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> int smmu_mapping_count, offset_to_id_array;
> int irq = sdev->irq;
>
>- if (virt_is_its_enabled(vms)) {
>+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
> smmu_mapping_count = 1; /* ITS Group node */
> offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
> } else {
>@@ -611,7 +622,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> }
> }
>
>- if (virt_is_its_enabled(vms)) {
>+ if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
> /*
> * Map bypassed (don't go through the SMMU) RIDs (input) to
> * ITS Group node directly: RC -> ITS.
>@@ -962,8 +973,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> }
> }
>
>- if (!(vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms))
>- && !vms->no_gicv3_with_gicv2m) {
>+ if (virt_is_gicv2m_enabled(vms)) {
> const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>
> /* 5.2.12.16 GIC MSI Frame Structure */
>diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>index 553c7f62cc..0e84ccd82c 100644
>--- a/hw/arm/virt.c
>+++ b/hw/arm/virt.c
>@@ -966,12 +966,12 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>
> fdt_add_gic_node(vms);
>
>- if (vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms)) {
>+ if (virt_is_its_enabled(vms)) {
> create_its(vms);
>- } else if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->no_gicv3_with_gicv2m) {
>- create_v2m(vms);
>- } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
>+ } else if (virt_is_gicv2m_enabled(vms)) {
> create_v2m(vms);
>+ } else {
>+ vms->msi_controller = VIRT_MSI_CTRL_NONE;
> }
> }
>
>@@ -2710,32 +2710,93 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,
>
> bool virt_is_its_enabled(VirtMachineState *vms)
> {
>- if (vms->its == ON_OFF_AUTO_OFF) {
>+ switch (vms->msi_controller) {
>+ case VIRT_MSI_CTRL_NONE:
> return false;
>- }
>- if (vms->its == ON_OFF_AUTO_AUTO) {
>- if (whpx_enabled()) {
>+ case VIRT_MSI_CTRL_ITS:
>+ return true;
>+ case VIRT_MSI_CTRL_GICV2M:
>+ return false;
>+ case VIRT_MSI_CTRL_AUTO:
>+ if (whpx_enabled() && whpx_irqchip_in_kernel()) {
>+ return false;
>+ }
>+ if (vms->gic_version == VIRT_GIC_VERSION_2) {
> return false;
> }
>+ return true;
> }
>- return true;
With the last return statement removed, I get a spurious warning from MSYS2/x86_64 GCC 15.2: "control reaches end of non-void function".
> }
>
>-static void virt_get_its(Object *obj, Visitor *v, const char *name,
>- void *opaque, Error **errp)
>+bool virt_is_gicv2m_enabled(VirtMachineState *vms)
>+{
>+ switch (vms->msi_controller) {
>+ case VIRT_MSI_CTRL_NONE:
>+ return false;
>+ default:
>+ return !virt_is_its_enabled(vms);
>+ }
>+}
>+
>+static char *virt_get_msi(Object *obj, Error **errp)
>+{
>+ VirtMachineState *vms = VIRT_MACHINE(obj);
>+ const char *val;
>+
>+ switch (vms->msi_controller) {
>+ case VIRT_MSI_CTRL_NONE:
>+ val = "off";
>+ break;
>+ case VIRT_MSI_CTRL_ITS:
>+ val = "its";
>+ break;
>+ case VIRT_MSI_CTRL_GICV2M:
>+ val = "gicv2m";
>+ break;
>+ case VIRT_MSI_CTRL_AUTO:
>+ val = "auto";
>+ break;
>+ }
Similar to the above I get a spurious warning: "'val' may be used uninitialized in this function".
Apparently the compiler doesn't notice that both switch statements handle all enums. I wonder why it can figure that out in other cases...
Other than that the series works for me!
Best regards,
Bernhard
>+ return g_strdup(val);
>+}
>+
>+static void virt_set_msi(Object *obj, const char *value, Error **errp)
> {
>+ ERRP_GUARD();
> VirtMachineState *vms = VIRT_MACHINE(obj);
>- OnOffAuto its = vms->its;
>
>- visit_type_OnOffAuto(v, name, &its, errp);
>+ if (!strcmp(value, "auto")) {
>+ vms->msi_controller = VIRT_MSI_CTRL_AUTO; /* Will be overriden later */
>+ } else if (!strcmp(value, "its")) {
>+ vms->msi_controller = VIRT_MSI_CTRL_ITS;
>+ } else if (!strcmp(value, "gicv2m")) {
>+ vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>+ } else if (!strcmp(value, "none")) {
>+ vms->msi_controller = VIRT_MSI_CTRL_NONE;
>+ } else {
>+ error_setg(errp, "Invalid msi value");
>+ error_append_hint(errp, "Valid values are auto, gicv2m, its, off\n");
>+ }
> }
>
>-static void virt_set_its(Object *obj, Visitor *v, const char *name,
>- void *opaque, Error **errp)
>+static bool virt_get_its(Object *obj, Error **errp)
> {
> VirtMachineState *vms = VIRT_MACHINE(obj);
>
>- visit_type_OnOffAuto(v, name, &vms->its, errp);
>+ return virt_is_its_enabled(vms);
>+}
>+
>+static void virt_set_its(Object *obj, bool value, Error **errp)
>+{
>+ VirtMachineState *vms = VIRT_MACHINE(obj);
>+
>+ if (value) {
>+ vms->msi_controller = VIRT_MSI_CTRL_ITS;
>+ } else if (vms->no_gicv3_with_gicv2m) {
>+ vms->msi_controller = VIRT_MSI_CTRL_NONE;
>+ } else {
>+ vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>+ }
> }
>
> static bool virt_get_dtb_randomness(Object *obj, Error **errp)
>@@ -3062,6 +3123,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> db_start = base_memmap[VIRT_GIC_V2M].base;
> db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
> break;
>+ case VIRT_MSI_CTRL_AUTO:
>+ g_assert_not_reached();
> }
> resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
> db_start, db_end,
>@@ -3452,13 +3515,18 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
> "guest CPU which implements the ARM "
> "Memory Tagging Extension");
>
>- object_class_property_add(oc, "its", "OnOffAuto",
>- virt_get_its, virt_set_its,
>- NULL, NULL);
>+ object_class_property_add_bool(oc, "its", virt_get_its,
>+ virt_set_its);
> object_class_property_set_description(oc, "its",
> "Set on/off to enable/disable "
> "ITS instantiation");
>
>+ object_class_property_add_str(oc, "msi", virt_get_msi,
>+ virt_set_msi);
>+ object_class_property_set_description(oc, "msi",
>+ "Set MSI settings. "
>+ "Valid values are auto/gicv2m/its/off");
>+
> object_class_property_add_bool(oc, "dtb-randomness",
> virt_get_dtb_randomness,
> virt_set_dtb_randomness);
>@@ -3515,7 +3583,7 @@ static void virt_instance_init(Object *obj)
> vms->highmem_redists = true;
>
> /* Default allows ITS instantiation if available */
>- vms->its = ON_OFF_AUTO_AUTO;
>+ vms->msi_controller = VIRT_MSI_CTRL_AUTO;
> /* Allow ITS emulation if the machine version supports it */
> vms->tcg_its = !vmc->no_tcg_its;
> vms->no_gicv3_with_gicv2m = false;
>diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>index 394b70c62e..ff43bcb739 100644
>--- a/include/hw/arm/virt.h
>+++ b/include/hw/arm/virt.h
>@@ -101,6 +101,8 @@ typedef enum VirtIOMMUType {
>
> typedef enum VirtMSIControllerType {
> VIRT_MSI_CTRL_NONE,
>+ /* This value is overriden at runtime.*/
>+ VIRT_MSI_CTRL_AUTO,
> VIRT_MSI_CTRL_GICV2M,
> VIRT_MSI_CTRL_ITS,
> } VirtMSIControllerType;
>@@ -147,7 +149,6 @@ struct VirtMachineState {
> bool highmem_ecam;
> bool highmem_mmio;
> bool highmem_redists;
>- OnOffAuto its;
> bool tcg_its;
> bool virt;
> bool ras;
>@@ -217,5 +218,6 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
> }
>
> bool virt_is_its_enabled(VirtMachineState *vms);
>+bool virt_is_gicv2m_enabled(VirtMachineState *vms);
>
> #endif /* QEMU_ARM_VIRT_H */
> On 14. Jan 2026, at 21:51, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 14. Januar 2026 13:41:24 UTC schrieb Mohamed Mediouni <mohamed@unpredictable.fr <mailto:mohamed@unpredictable.fr>>:
>> Introduce a -M msi= argument to be able to control MSI-X support independently
>> from ITS, as part of supporting GICv3 + GICv2m platforms.
>>
>> Remove vms->its as it's no longer needed after that change.
>>
>> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>> ---
>> hw/arm/virt-acpi-build.c | 24 ++++++---
>> hw/arm/virt.c | 108 +++++++++++++++++++++++++++++++--------
>> include/hw/arm/virt.h | 4 +-
>> 3 files changed, 108 insertions(+), 28 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 86024a1a73..187dd4e272 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -440,6 +440,17 @@ static void create_rc_its_idmaps(GArray *its_idmaps, GArray *smmuv3_devs)
>> }
>> }
>>
>> +/*
>> + * In the prior Qemu ACPI table handling, GICv2 configurations
>> + * had vms->its=1... That's broken.
>> + *
>> + * Match that assumption to match the existing ACPI tables that
>> + * have been shipping for quite a while.
>> + */
>> +static int is_gicv2_acpi_workaround_needed(VirtMachineState *vms) {
>> + return vms->gic_version == 2;
>> +}
>> +
>> /*
>> * Input Output Remapping Table (IORT)
>> * Conforms to "IO Remapping Table System Software on ARM Platforms",
>> @@ -473,7 +484,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> nb_nodes = num_smmus + 1; /* RC and SMMUv3 */
>> rc_mapping_count = rc_smmu_idmaps_len;
>>
>> - if (virt_is_its_enabled(vms)) {
>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>> /*
>> * 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.
>> @@ -484,7 +495,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> rc_mapping_count += rc_its_idmaps->len;
>> }
>> } else {
>> - if (virt_is_its_enabled(vms)) {
>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>> nb_nodes = 2; /* RC and ITS */
>> rc_mapping_count = 1; /* Direct map to ITS */
>> } else {
>> @@ -499,7 +510,7 @@ 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 */
>>
>> - if (virt_is_its_enabled(vms)) {
>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>> /* 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 */;
>> @@ -518,7 +529,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> int smmu_mapping_count, offset_to_id_array;
>> int irq = sdev->irq;
>>
>> - if (virt_is_its_enabled(vms)) {
>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>> smmu_mapping_count = 1; /* ITS Group node */
>> offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
>> } else {
>> @@ -611,7 +622,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> }
>> }
>>
>> - if (virt_is_its_enabled(vms)) {
>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>> /*
>> * Map bypassed (don't go through the SMMU) RIDs (input) to
>> * ITS Group node directly: RC -> ITS.
>> @@ -962,8 +973,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> }
>> }
>>
>> - if (!(vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms))
>> - && !vms->no_gicv3_with_gicv2m) {
>> + if (virt_is_gicv2m_enabled(vms)) {
>> const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>>
>> /* 5.2.12.16 GIC MSI Frame Structure */
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 553c7f62cc..0e84ccd82c 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -966,12 +966,12 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>>
>> fdt_add_gic_node(vms);
>>
>> - if (vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms)) {
>> + if (virt_is_its_enabled(vms)) {
>> create_its(vms);
>> - } else if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->no_gicv3_with_gicv2m) {
>> - create_v2m(vms);
>> - } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
>> + } else if (virt_is_gicv2m_enabled(vms)) {
>> create_v2m(vms);
>> + } else {
>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>> }
>> }
>>
>> @@ -2710,32 +2710,93 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,
>>
>> bool virt_is_its_enabled(VirtMachineState *vms)
>> {
>> - if (vms->its == ON_OFF_AUTO_OFF) {
>> + switch (vms->msi_controller) {
>> + case VIRT_MSI_CTRL_NONE:
>> return false;
>> - }
>> - if (vms->its == ON_OFF_AUTO_AUTO) {
>> - if (whpx_enabled()) {
>> + case VIRT_MSI_CTRL_ITS:
>> + return true;
>> + case VIRT_MSI_CTRL_GICV2M:
>> + return false;
>> + case VIRT_MSI_CTRL_AUTO:
>> + if (whpx_enabled() && whpx_irqchip_in_kernel()) {
>> + return false;
>> + }
>> + if (vms->gic_version == VIRT_GIC_VERSION_2) {
>> return false;
>> }
>> + return true;
>> }
>> - return true;
>
> With the last return statement removed, I get a spurious warning from MSYS2/x86_64 GCC 15.2: "control reaches end of non-void function".
>
>> }
>>
>> -static void virt_get_its(Object *obj, Visitor *v, const char *name,
>> - void *opaque, Error **errp)
>> +bool virt_is_gicv2m_enabled(VirtMachineState *vms)
>> +{
>> + switch (vms->msi_controller) {
>> + case VIRT_MSI_CTRL_NONE:
>> + return false;
>> + default:
>> + return !virt_is_its_enabled(vms);
>> + }
>> +}
>> +
>> +static char *virt_get_msi(Object *obj, Error **errp)
>> +{
>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>> + const char *val;
>> +
>> + switch (vms->msi_controller) {
>> + case VIRT_MSI_CTRL_NONE:
>> + val = "off";
>> + break;
>> + case VIRT_MSI_CTRL_ITS:
>> + val = "its";
>> + break;
>> + case VIRT_MSI_CTRL_GICV2M:
>> + val = "gicv2m";
>> + break;
>> + case VIRT_MSI_CTRL_AUTO:
>> + val = "auto";
>> + break;
>> + }
>
> Similar to the above I get a spurious warning: "'val' may be used uninitialized in this function".
>
> Apparently the compiler doesn't notice that both switch statements handle all enums. I wonder why it can figure that out in other cases…
>
Hello,
I didn’t see this on Clang but I do see this on GCC. Will see what I can do…
Apparently it’s https://stackoverflow.com/questions/33607284/control-reaches-end-of-non-void-function-with-fully-handled-case-switch-over-a and the best way to go seems to be to add an unreachable assert
> Other than that the series works for me!
>
> Best regards,
> Bernhard
>
>> + return g_strdup(val);
>> +}
>> +
>> +static void virt_set_msi(Object *obj, const char *value, Error **errp)
>> {
>> + ERRP_GUARD();
>> VirtMachineState *vms = VIRT_MACHINE(obj);
>> - OnOffAuto its = vms->its;
>>
>> - visit_type_OnOffAuto(v, name, &its, errp);
>> + if (!strcmp(value, "auto")) {
>> + vms->msi_controller = VIRT_MSI_CTRL_AUTO; /* Will be overriden later */
>> + } else if (!strcmp(value, "its")) {
>> + vms->msi_controller = VIRT_MSI_CTRL_ITS;
>> + } else if (!strcmp(value, "gicv2m")) {
>> + vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>> + } else if (!strcmp(value, "none")) {
>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>> + } else {
>> + error_setg(errp, "Invalid msi value");
>> + error_append_hint(errp, "Valid values are auto, gicv2m, its, off\n");
>> + }
>> }
>>
>> -static void virt_set_its(Object *obj, Visitor *v, const char *name,
>> - void *opaque, Error **errp)
>> +static bool virt_get_its(Object *obj, Error **errp)
>> {
>> VirtMachineState *vms = VIRT_MACHINE(obj);
>>
>> - visit_type_OnOffAuto(v, name, &vms->its, errp);
>> + return virt_is_its_enabled(vms);
>> +}
>> +
>> +static void virt_set_its(Object *obj, bool value, Error **errp)
>> +{
>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> + if (value) {
>> + vms->msi_controller = VIRT_MSI_CTRL_ITS;
>> + } else if (vms->no_gicv3_with_gicv2m) {
>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>> + } else {
>> + vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>> + }
>> }
>>
>> static bool virt_get_dtb_randomness(Object *obj, Error **errp)
>> @@ -3062,6 +3123,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> db_start = base_memmap[VIRT_GIC_V2M].base;
>> db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
>> break;
>> + case VIRT_MSI_CTRL_AUTO:
>> + g_assert_not_reached();
>> }
>> resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
>> db_start, db_end,
>> @@ -3452,13 +3515,18 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
>> "guest CPU which implements the ARM "
>> "Memory Tagging Extension");
>>
>> - object_class_property_add(oc, "its", "OnOffAuto",
>> - virt_get_its, virt_set_its,
>> - NULL, NULL);
>> + object_class_property_add_bool(oc, "its", virt_get_its,
>> + virt_set_its);
>> object_class_property_set_description(oc, "its",
>> "Set on/off to enable/disable "
>> "ITS instantiation");
>>
>> + object_class_property_add_str(oc, "msi", virt_get_msi,
>> + virt_set_msi);
>> + object_class_property_set_description(oc, "msi",
>> + "Set MSI settings. "
>> + "Valid values are auto/gicv2m/its/off");
>> +
>> object_class_property_add_bool(oc, "dtb-randomness",
>> virt_get_dtb_randomness,
>> virt_set_dtb_randomness);
>> @@ -3515,7 +3583,7 @@ static void virt_instance_init(Object *obj)
>> vms->highmem_redists = true;
>>
>> /* Default allows ITS instantiation if available */
>> - vms->its = ON_OFF_AUTO_AUTO;
>> + vms->msi_controller = VIRT_MSI_CTRL_AUTO;
>> /* Allow ITS emulation if the machine version supports it */
>> vms->tcg_its = !vmc->no_tcg_its;
>> vms->no_gicv3_with_gicv2m = false;
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index 394b70c62e..ff43bcb739 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -101,6 +101,8 @@ typedef enum VirtIOMMUType {
>>
>> typedef enum VirtMSIControllerType {
>> VIRT_MSI_CTRL_NONE,
>> + /* This value is overriden at runtime.*/
>> + VIRT_MSI_CTRL_AUTO,
>> VIRT_MSI_CTRL_GICV2M,
>> VIRT_MSI_CTRL_ITS,
>> } VirtMSIControllerType;
>> @@ -147,7 +149,6 @@ struct VirtMachineState {
>> bool highmem_ecam;
>> bool highmem_mmio;
>> bool highmem_redists;
>> - OnOffAuto its;
>> bool tcg_its;
>> bool virt;
>> bool ras;
>> @@ -217,5 +218,6 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
>> }
>>
>> bool virt_is_its_enabled(VirtMachineState *vms);
>> +bool virt_is_gicv2m_enabled(VirtMachineState *vms);
>>
>> #endif /* QEMU_ARM_VIRT_H */
> On 15. Jan 2026, at 00:08, Mohamed Mediouni <mohamed@unpredictable.fr> wrote:
>
>
>
>> On 14. Jan 2026, at 21:51, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>>
>>
>> Am 14. Januar 2026 13:41:24 UTC schrieb Mohamed Mediouni <mohamed@unpredictable.fr <mailto:mohamed@unpredictable.fr>>:
>>> Introduce a -M msi= argument to be able to control MSI-X support independently
>>> from ITS, as part of supporting GICv3 + GICv2m platforms.
>>>
>>> Remove vms->its as it's no longer needed after that change.
>>>
>>> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>>> ---
>>> hw/arm/virt-acpi-build.c | 24 ++++++---
>>> hw/arm/virt.c | 108 +++++++++++++++++++++++++++++++--------
>>> include/hw/arm/virt.h | 4 +-
>>> 3 files changed, 108 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>> index 86024a1a73..187dd4e272 100644
>>> --- a/hw/arm/virt-acpi-build.c
>>> +++ b/hw/arm/virt-acpi-build.c
>>> @@ -440,6 +440,17 @@ static void create_rc_its_idmaps(GArray *its_idmaps, GArray *smmuv3_devs)
>>> }
>>> }
>>>
>>> +/*
>>> + * In the prior Qemu ACPI table handling, GICv2 configurations
>>> + * had vms->its=1... That's broken.
>>> + *
>>> + * Match that assumption to match the existing ACPI tables that
>>> + * have been shipping for quite a while.
>>> + */
>>> +static int is_gicv2_acpi_workaround_needed(VirtMachineState *vms) {
>>> + return vms->gic_version == 2;
>>> +}
>>> +
>>> /*
>>> * Input Output Remapping Table (IORT)
>>> * Conforms to "IO Remapping Table System Software on ARM Platforms",
>>> @@ -473,7 +484,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>> nb_nodes = num_smmus + 1; /* RC and SMMUv3 */
>>> rc_mapping_count = rc_smmu_idmaps_len;
>>>
>>> - if (virt_is_its_enabled(vms)) {
>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>> /*
>>> * 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.
>>> @@ -484,7 +495,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>> rc_mapping_count += rc_its_idmaps->len;
>>> }
>>> } else {
>>> - if (virt_is_its_enabled(vms)) {
>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>> nb_nodes = 2; /* RC and ITS */
>>> rc_mapping_count = 1; /* Direct map to ITS */
>>> } else {
>>> @@ -499,7 +510,7 @@ 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 */
>>>
>>> - if (virt_is_its_enabled(vms)) {
>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>> /* 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 */;
>>> @@ -518,7 +529,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>> int smmu_mapping_count, offset_to_id_array;
>>> int irq = sdev->irq;
>>>
>>> - if (virt_is_its_enabled(vms)) {
>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>> smmu_mapping_count = 1; /* ITS Group node */
>>> offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
>>> } else {
>>> @@ -611,7 +622,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>> }
>>> }
>>>
>>> - if (virt_is_its_enabled(vms)) {
>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>> /*
>>> * Map bypassed (don't go through the SMMU) RIDs (input) to
>>> * ITS Group node directly: RC -> ITS.
>>> @@ -962,8 +973,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>> }
>>> }
>>>
>>> - if (!(vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms))
>>> - && !vms->no_gicv3_with_gicv2m) {
>>> + if (virt_is_gicv2m_enabled(vms)) {
>>> const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>>>
>>> /* 5.2.12.16 GIC MSI Frame Structure */
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 553c7f62cc..0e84ccd82c 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -966,12 +966,12 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>>>
>>> fdt_add_gic_node(vms);
>>>
>>> - if (vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms)) {
>>> + if (virt_is_its_enabled(vms)) {
>>> create_its(vms);
>>> - } else if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->no_gicv3_with_gicv2m) {
>>> - create_v2m(vms);
>>> - } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
>>> + } else if (virt_is_gicv2m_enabled(vms)) {
>>> create_v2m(vms);
>>> + } else {
>>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>>> }
>>> }
>>>
>>> @@ -2710,32 +2710,93 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,
>>>
>>> bool virt_is_its_enabled(VirtMachineState *vms)
>>> {
>>> - if (vms->its == ON_OFF_AUTO_OFF) {
>>> + switch (vms->msi_controller) {
>>> + case VIRT_MSI_CTRL_NONE:
>>> return false;
>>> - }
>>> - if (vms->its == ON_OFF_AUTO_AUTO) {
>>> - if (whpx_enabled()) {
>>> + case VIRT_MSI_CTRL_ITS:
>>> + return true;
>>> + case VIRT_MSI_CTRL_GICV2M:
>>> + return false;
>>> + case VIRT_MSI_CTRL_AUTO:
>>> + if (whpx_enabled() && whpx_irqchip_in_kernel()) {
>>> + return false;
>>> + }
>>> + if (vms->gic_version == VIRT_GIC_VERSION_2) {
>>> return false;
>>> }
>>> + return true;
>>> }
>>> - return true;
>>
>> With the last return statement removed, I get a spurious warning from MSYS2/x86_64 GCC 15.2: "control reaches end of non-void function".
>>
>>> }
>>>
>>> -static void virt_get_its(Object *obj, Visitor *v, const char *name,
>>> - void *opaque, Error **errp)
>>> +bool virt_is_gicv2m_enabled(VirtMachineState *vms)
>>> +{
>>> + switch (vms->msi_controller) {
>>> + case VIRT_MSI_CTRL_NONE:
>>> + return false;
>>> + default:
>>> + return !virt_is_its_enabled(vms);
>>> + }
>>> +}
>>> +
>>> +static char *virt_get_msi(Object *obj, Error **errp)
>>> +{
>>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>>> + const char *val;
>>> +
>>> + switch (vms->msi_controller) {
>>> + case VIRT_MSI_CTRL_NONE:
>>> + val = "off";
>>> + break;
>>> + case VIRT_MSI_CTRL_ITS:
>>> + val = "its";
>>> + break;
>>> + case VIRT_MSI_CTRL_GICV2M:
>>> + val = "gicv2m";
>>> + break;
>>> + case VIRT_MSI_CTRL_AUTO:
>>> + val = "auto";
>>> + break;
>>> + }
>>
>> Similar to the above I get a spurious warning: "'val' may be used uninitialized in this function".
>>
>> Apparently the compiler doesn't notice that both switch statements handle all enums. I wonder why it can figure that out in other cases…
>>
> Hello,
>
> I didn’t see this on Clang but I do see this on GCC. Will see what I can do…
>
> Apparently it’s https://stackoverflow.com/questions/33607284/control-reaches-end-of-non-void-function-with-fully-handled-case-switch-over-a and the best way to go seems to be to add an unreachable assert
>
There seems to be a long-standing GCC bug filed for this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950
>> Other than that the series works for me!
>>
>> Best regards,
>> Bernhard
>>
>>> + return g_strdup(val);
>>> +}
>>> +
>>> +static void virt_set_msi(Object *obj, const char *value, Error **errp)
>>> {
>>> + ERRP_GUARD();
>>> VirtMachineState *vms = VIRT_MACHINE(obj);
>>> - OnOffAuto its = vms->its;
>>>
>>> - visit_type_OnOffAuto(v, name, &its, errp);
>>> + if (!strcmp(value, "auto")) {
>>> + vms->msi_controller = VIRT_MSI_CTRL_AUTO; /* Will be overriden later */
>>> + } else if (!strcmp(value, "its")) {
>>> + vms->msi_controller = VIRT_MSI_CTRL_ITS;
>>> + } else if (!strcmp(value, "gicv2m")) {
>>> + vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>>> + } else if (!strcmp(value, "none")) {
>>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>>> + } else {
>>> + error_setg(errp, "Invalid msi value");
>>> + error_append_hint(errp, "Valid values are auto, gicv2m, its, off\n");
>>> + }
>>> }
>>>
>>> -static void virt_set_its(Object *obj, Visitor *v, const char *name,
>>> - void *opaque, Error **errp)
>>> +static bool virt_get_its(Object *obj, Error **errp)
>>> {
>>> VirtMachineState *vms = VIRT_MACHINE(obj);
>>>
>>> - visit_type_OnOffAuto(v, name, &vms->its, errp);
>>> + return virt_is_its_enabled(vms);
>>> +}
>>> +
>>> +static void virt_set_its(Object *obj, bool value, Error **errp)
>>> +{
>>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>>> +
>>> + if (value) {
>>> + vms->msi_controller = VIRT_MSI_CTRL_ITS;
>>> + } else if (vms->no_gicv3_with_gicv2m) {
>>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>>> + } else {
>>> + vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>>> + }
>>> }
>>>
>>> static bool virt_get_dtb_randomness(Object *obj, Error **errp)
>>> @@ -3062,6 +3123,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>> db_start = base_memmap[VIRT_GIC_V2M].base;
>>> db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
>>> break;
>>> + case VIRT_MSI_CTRL_AUTO:
>>> + g_assert_not_reached();
>>> }
>>> resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
>>> db_start, db_end,
>>> @@ -3452,13 +3515,18 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
>>> "guest CPU which implements the ARM "
>>> "Memory Tagging Extension");
>>>
>>> - object_class_property_add(oc, "its", "OnOffAuto",
>>> - virt_get_its, virt_set_its,
>>> - NULL, NULL);
>>> + object_class_property_add_bool(oc, "its", virt_get_its,
>>> + virt_set_its);
>>> object_class_property_set_description(oc, "its",
>>> "Set on/off to enable/disable "
>>> "ITS instantiation");
>>>
>>> + object_class_property_add_str(oc, "msi", virt_get_msi,
>>> + virt_set_msi);
>>> + object_class_property_set_description(oc, "msi",
>>> + "Set MSI settings. "
>>> + "Valid values are auto/gicv2m/its/off");
>>> +
>>> object_class_property_add_bool(oc, "dtb-randomness",
>>> virt_get_dtb_randomness,
>>> virt_set_dtb_randomness);
>>> @@ -3515,7 +3583,7 @@ static void virt_instance_init(Object *obj)
>>> vms->highmem_redists = true;
>>>
>>> /* Default allows ITS instantiation if available */
>>> - vms->its = ON_OFF_AUTO_AUTO;
>>> + vms->msi_controller = VIRT_MSI_CTRL_AUTO;
>>> /* Allow ITS emulation if the machine version supports it */
>>> vms->tcg_its = !vmc->no_tcg_its;
>>> vms->no_gicv3_with_gicv2m = false;
>>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>>> index 394b70c62e..ff43bcb739 100644
>>> --- a/include/hw/arm/virt.h
>>> +++ b/include/hw/arm/virt.h
>>> @@ -101,6 +101,8 @@ typedef enum VirtIOMMUType {
>>>
>>> typedef enum VirtMSIControllerType {
>>> VIRT_MSI_CTRL_NONE,
>>> + /* This value is overriden at runtime.*/
>>> + VIRT_MSI_CTRL_AUTO,
>>> VIRT_MSI_CTRL_GICV2M,
>>> VIRT_MSI_CTRL_ITS,
>>> } VirtMSIControllerType;
>>> @@ -147,7 +149,6 @@ struct VirtMachineState {
>>> bool highmem_ecam;
>>> bool highmem_mmio;
>>> bool highmem_redists;
>>> - OnOffAuto its;
>>> bool tcg_its;
>>> bool virt;
>>> bool ras;
>>> @@ -217,5 +218,6 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
>>> }
>>>
>>> bool virt_is_its_enabled(VirtMachineState *vms);
>>> +bool virt_is_gicv2m_enabled(VirtMachineState *vms);
>>>
>>> #endif /* QEMU_ARM_VIRT_H */
Am 14. Januar 2026 23:12:40 UTC schrieb Mohamed Mediouni <mohamed@unpredictable.fr>:
>
>
>> On 15. Jan 2026, at 00:08, Mohamed Mediouni <mohamed@unpredictable.fr> wrote:
>>
>>
>>
>>> On 14. Jan 2026, at 21:51, Bernhard Beschow <shentey@gmail.com> wrote:
>>>
>>>
>>>
>>> Am 14. Januar 2026 13:41:24 UTC schrieb Mohamed Mediouni <mohamed@unpredictable.fr <mailto:mohamed@unpredictable.fr>>:
>>>> Introduce a -M msi= argument to be able to control MSI-X support independently
>>>> from ITS, as part of supporting GICv3 + GICv2m platforms.
>>>>
>>>> Remove vms->its as it's no longer needed after that change.
>>>>
>>>> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>>>> ---
>>>> hw/arm/virt-acpi-build.c | 24 ++++++---
>>>> hw/arm/virt.c | 108 +++++++++++++++++++++++++++++++--------
>>>> include/hw/arm/virt.h | 4 +-
>>>> 3 files changed, 108 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>>> index 86024a1a73..187dd4e272 100644
>>>> --- a/hw/arm/virt-acpi-build.c
>>>> +++ b/hw/arm/virt-acpi-build.c
>>>> @@ -440,6 +440,17 @@ static void create_rc_its_idmaps(GArray *its_idmaps, GArray *smmuv3_devs)
>>>> }
>>>> }
>>>>
>>>> +/*
>>>> + * In the prior Qemu ACPI table handling, GICv2 configurations
>>>> + * had vms->its=1... That's broken.
>>>> + *
>>>> + * Match that assumption to match the existing ACPI tables that
>>>> + * have been shipping for quite a while.
>>>> + */
>>>> +static int is_gicv2_acpi_workaround_needed(VirtMachineState *vms) {
>>>> + return vms->gic_version == 2;
>>>> +}
>>>> +
>>>> /*
>>>> * Input Output Remapping Table (IORT)
>>>> * Conforms to "IO Remapping Table System Software on ARM Platforms",
>>>> @@ -473,7 +484,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>>> nb_nodes = num_smmus + 1; /* RC and SMMUv3 */
>>>> rc_mapping_count = rc_smmu_idmaps_len;
>>>>
>>>> - if (virt_is_its_enabled(vms)) {
>>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>>> /*
>>>> * 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.
>>>> @@ -484,7 +495,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>>> rc_mapping_count += rc_its_idmaps->len;
>>>> }
>>>> } else {
>>>> - if (virt_is_its_enabled(vms)) {
>>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>>> nb_nodes = 2; /* RC and ITS */
>>>> rc_mapping_count = 1; /* Direct map to ITS */
>>>> } else {
>>>> @@ -499,7 +510,7 @@ 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 */
>>>>
>>>> - if (virt_is_its_enabled(vms)) {
>>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>>> /* 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 */;
>>>> @@ -518,7 +529,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>>> int smmu_mapping_count, offset_to_id_array;
>>>> int irq = sdev->irq;
>>>>
>>>> - if (virt_is_its_enabled(vms)) {
>>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>>> smmu_mapping_count = 1; /* ITS Group node */
>>>> offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
>>>> } else {
>>>> @@ -611,7 +622,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>>> }
>>>> }
>>>>
>>>> - if (virt_is_its_enabled(vms)) {
>>>> + if (virt_is_its_enabled(vms) || is_gicv2_acpi_workaround_needed(vms)) {
>>>> /*
>>>> * Map bypassed (don't go through the SMMU) RIDs (input) to
>>>> * ITS Group node directly: RC -> ITS.
>>>> @@ -962,8 +973,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>>> }
>>>> }
>>>>
>>>> - if (!(vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms))
>>>> - && !vms->no_gicv3_with_gicv2m) {
>>>> + if (virt_is_gicv2m_enabled(vms)) {
>>>> const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>>>>
>>>> /* 5.2.12.16 GIC MSI Frame Structure */
>>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>>> index 553c7f62cc..0e84ccd82c 100644
>>>> --- a/hw/arm/virt.c
>>>> +++ b/hw/arm/virt.c
>>>> @@ -966,12 +966,12 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>>>>
>>>> fdt_add_gic_node(vms);
>>>>
>>>> - if (vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms)) {
>>>> + if (virt_is_its_enabled(vms)) {
>>>> create_its(vms);
>>>> - } else if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->no_gicv3_with_gicv2m) {
>>>> - create_v2m(vms);
>>>> - } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
>>>> + } else if (virt_is_gicv2m_enabled(vms)) {
>>>> create_v2m(vms);
>>>> + } else {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>>>> }
>>>> }
>>>>
>>>> @@ -2710,32 +2710,93 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,
>>>>
>>>> bool virt_is_its_enabled(VirtMachineState *vms)
>>>> {
>>>> - if (vms->its == ON_OFF_AUTO_OFF) {
>>>> + switch (vms->msi_controller) {
>>>> + case VIRT_MSI_CTRL_NONE:
>>>> return false;
>>>> - }
>>>> - if (vms->its == ON_OFF_AUTO_AUTO) {
>>>> - if (whpx_enabled()) {
>>>> + case VIRT_MSI_CTRL_ITS:
>>>> + return true;
>>>> + case VIRT_MSI_CTRL_GICV2M:
>>>> + return false;
>>>> + case VIRT_MSI_CTRL_AUTO:
>>>> + if (whpx_enabled() && whpx_irqchip_in_kernel()) {
>>>> + return false;
>>>> + }
>>>> + if (vms->gic_version == VIRT_GIC_VERSION_2) {
>>>> return false;
>>>> }
>>>> + return true;
>>>> }
>>>> - return true;
>>>
>>> With the last return statement removed, I get a spurious warning from MSYS2/x86_64 GCC 15.2: "control reaches end of non-void function".
>>>
>>>> }
>>>>
>>>> -static void virt_get_its(Object *obj, Visitor *v, const char *name,
>>>> - void *opaque, Error **errp)
>>>> +bool virt_is_gicv2m_enabled(VirtMachineState *vms)
>>>> +{
>>>> + switch (vms->msi_controller) {
>>>> + case VIRT_MSI_CTRL_NONE:
>>>> + return false;
>>>> + default:
>>>> + return !virt_is_its_enabled(vms);
>>>> + }
>>>> +}
>>>> +
>>>> +static char *virt_get_msi(Object *obj, Error **errp)
>>>> +{
>>>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>>>> + const char *val;
>>>> +
>>>> + switch (vms->msi_controller) {
>>>> + case VIRT_MSI_CTRL_NONE:
>>>> + val = "off";
>>>> + break;
>>>> + case VIRT_MSI_CTRL_ITS:
>>>> + val = "its";
>>>> + break;
>>>> + case VIRT_MSI_CTRL_GICV2M:
>>>> + val = "gicv2m";
>>>> + break;
>>>> + case VIRT_MSI_CTRL_AUTO:
>>>> + val = "auto";
>>>> + break;
>>>> + }
>>>
>>> Similar to the above I get a spurious warning: "'val' may be used uninitialized in this function".
>>>
>>> Apparently the compiler doesn't notice that both switch statements handle all enums. I wonder why it can figure that out in other cases…
>>>
>> Hello,
>>
>> I didn’t see this on Clang but I do see this on GCC. Will see what I can do…
>>
>> Apparently it’s https://stackoverflow.com/questions/33607284/control-reaches-end-of-non-void-function-with-fully-handled-case-switch-over-a and the best way to go seems to be to add an unreachable assert
>>
>
>There seems to be a long-standing GCC bug filed for this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950
Yeah, seems like a case for language lawyers...
>
>>> Other than that the series works for me!
>>>
>>> Best regards,
>>> Bernhard
>>>
>>>> + return g_strdup(val);
>>>> +}
>>>> +
>>>> +static void virt_set_msi(Object *obj, const char *value, Error **errp)
>>>> {
>>>> + ERRP_GUARD();
>>>> VirtMachineState *vms = VIRT_MACHINE(obj);
>>>> - OnOffAuto its = vms->its;
>>>>
>>>> - visit_type_OnOffAuto(v, name, &its, errp);
>>>> + if (!strcmp(value, "auto")) {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_AUTO; /* Will be overriden later */
>>>> + } else if (!strcmp(value, "its")) {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_ITS;
>>>> + } else if (!strcmp(value, "gicv2m")) {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>>>> + } else if (!strcmp(value, "none")) {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>>>> + } else {
>>>> + error_setg(errp, "Invalid msi value");
>>>> + error_append_hint(errp, "Valid values are auto, gicv2m, its, off\n");
>>>> + }
>>>> }
>>>>
>>>> -static void virt_set_its(Object *obj, Visitor *v, const char *name,
>>>> - void *opaque, Error **errp)
>>>> +static bool virt_get_its(Object *obj, Error **errp)
>>>> {
>>>> VirtMachineState *vms = VIRT_MACHINE(obj);
>>>>
>>>> - visit_type_OnOffAuto(v, name, &vms->its, errp);
>>>> + return virt_is_its_enabled(vms);
>>>> +}
>>>> +
>>>> +static void virt_set_its(Object *obj, bool value, Error **errp)
>>>> +{
>>>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>>>> +
>>>> + if (value) {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_ITS;
>>>> + } else if (vms->no_gicv3_with_gicv2m) {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_NONE;
>>>> + } else {
>>>> + vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>>>> + }
>>>> }
>>>>
>>>> static bool virt_get_dtb_randomness(Object *obj, Error **errp)
>>>> @@ -3062,6 +3123,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>>> db_start = base_memmap[VIRT_GIC_V2M].base;
>>>> db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
>>>> break;
>>>> + case VIRT_MSI_CTRL_AUTO:
>>>> + g_assert_not_reached();
>>>> }
>>>> resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
>>>> db_start, db_end,
>>>> @@ -3452,13 +3515,18 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
>>>> "guest CPU which implements the ARM "
>>>> "Memory Tagging Extension");
>>>>
>>>> - object_class_property_add(oc, "its", "OnOffAuto",
>>>> - virt_get_its, virt_set_its,
>>>> - NULL, NULL);
>>>> + object_class_property_add_bool(oc, "its", virt_get_its,
>>>> + virt_set_its);
>>>> object_class_property_set_description(oc, "its",
>>>> "Set on/off to enable/disable "
>>>> "ITS instantiation");
>>>>
>>>> + object_class_property_add_str(oc, "msi", virt_get_msi,
>>>> + virt_set_msi);
>>>> + object_class_property_set_description(oc, "msi",
>>>> + "Set MSI settings. "
>>>> + "Valid values are auto/gicv2m/its/off");
>>>> +
>>>> object_class_property_add_bool(oc, "dtb-randomness",
>>>> virt_get_dtb_randomness,
>>>> virt_set_dtb_randomness);
>>>> @@ -3515,7 +3583,7 @@ static void virt_instance_init(Object *obj)
>>>> vms->highmem_redists = true;
>>>>
>>>> /* Default allows ITS instantiation if available */
>>>> - vms->its = ON_OFF_AUTO_AUTO;
>>>> + vms->msi_controller = VIRT_MSI_CTRL_AUTO;
>>>> /* Allow ITS emulation if the machine version supports it */
>>>> vms->tcg_its = !vmc->no_tcg_its;
>>>> vms->no_gicv3_with_gicv2m = false;
>>>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>>>> index 394b70c62e..ff43bcb739 100644
>>>> --- a/include/hw/arm/virt.h
>>>> +++ b/include/hw/arm/virt.h
>>>> @@ -101,6 +101,8 @@ typedef enum VirtIOMMUType {
>>>>
>>>> typedef enum VirtMSIControllerType {
>>>> VIRT_MSI_CTRL_NONE,
>>>> + /* This value is overriden at runtime.*/
>>>> + VIRT_MSI_CTRL_AUTO,
>>>> VIRT_MSI_CTRL_GICV2M,
>>>> VIRT_MSI_CTRL_ITS,
>>>> } VirtMSIControllerType;
>>>> @@ -147,7 +149,6 @@ struct VirtMachineState {
>>>> bool highmem_ecam;
>>>> bool highmem_mmio;
>>>> bool highmem_redists;
>>>> - OnOffAuto its;
>>>> bool tcg_its;
>>>> bool virt;
>>>> bool ras;
>>>> @@ -217,5 +218,6 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
>>>> }
>>>>
>>>> bool virt_is_its_enabled(VirtMachineState *vms);
>>>> +bool virt_is_gicv2m_enabled(VirtMachineState *vms);
>>>>
>>>> #endif /* QEMU_ARM_VIRT_H */
>
© 2016 - 2026 Red Hat, Inc.