Re: [PATCH v7 15/24] hw/arm: virt: cleanly fail on attempt to use the platform vGIC together with ITS

[Mohamed Mediouni <mohamed@unpredictable.fr>]

Note for this specific one (already posted on IRC, but worth keeping in mind for a merge), mail server is misbehaving… (so v8 has been posted partially. And this is the change there)

It has a bug, with the changed commit being at https://github.com/mediouni-m/qemu/commit/863eb69916be4a43ddde62276bb643caf46ef236

The delta between the two:

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index c716130206..8e730731ca 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -961,7 +961,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
        }
    }

-    if (virt_is_its_enabled(vms) && !vms->no_gicv3_with_gicv2m) {
+    if (!(vms->gic_version != VIRT_GIC_VERSION_2 && virt_is_its_enabled(vms))
+     && !vms->no_gicv3_with_gicv2m) {
        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 1904765db3..b5bed029b9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2717,10 +2717,6 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,

bool virt_is_its_enabled(VirtMachineState *vms)
{
-    if (vms->gic_version == VIRT_GIC_VERSION_2) {
-        return false;
-    }
-
    if (vms->its == ON_OFF_AUTO_OFF) {
        return false;
    }



> On 16. Oct 2025, at 18:55, Mohamed Mediouni <mohamed@unpredictable.fr> wrote:
> 
> Switch its to a tristate.
> 
> Windows Hypervisor Platform's vGIC doesn't support ITS.
> Deal with this by reporting to the user and exiting.
> 
> Regular configuration: GICv3 + ITS
> New default configuration with WHPX: GICv3 with GICv2m
> And its=off explicitly for the newest machine version: GICv3 + GICv2m
> 
> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
> ---
> hw/arm/virt-acpi-build.c | 14 +++++------
> hw/arm/virt.c            | 50 ++++++++++++++++++++++++++++++++--------
> include/hw/arm/virt.h    |  4 +++-
> 3 files changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 0a6ec74aa0..c716130206 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -472,7 +472,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 (vms->its) {
> +        if (virt_is_its_enabled(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.
> @@ -483,7 +483,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>            rc_mapping_count += rc_its_idmaps->len;
>        }
>    } else {
> -        if (vms->its) {
> +        if (virt_is_its_enabled(vms)) {
>            nb_nodes = 2; /* RC and ITS */
>            rc_mapping_count = 1; /* Direct map to ITS */
>        } else {
> @@ -498,7 +498,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 (vms->its) {
> +    if (virt_is_its_enabled(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 */;
> @@ -517,7 +517,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>        int smmu_mapping_count, offset_to_id_array;
>        int irq = sdev->irq;
> 
> -        if (vms->its) {
> +        if (virt_is_its_enabled(vms)) {
>            smmu_mapping_count = 1; /* ITS Group node */
>            offset_to_id_array = SMMU_V3_ENTRY_SIZE; /* Just after the header */
>        } else {
> @@ -610,7 +610,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>            }
>        }
> 
> -        if (vms->its) {
> +        if (virt_is_its_enabled(vms)) {
>            /*
>             * Map bypassed (don't go through the SMMU) RIDs (input) to
>             * ITS Group node directly: RC -> ITS.
> @@ -945,7 +945,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>                                          memmap[VIRT_HIGH_GIC_REDIST2].size);
>        }
> 
> -        if (vms->its) {
> +        if (virt_is_its_enabled(vms)) {
>            /*
>             * ACPI spec, Revision 6.0 Errata A
>             * (original 6.0 definition has invalid Length)
> @@ -961,7 +961,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>        }
>    }
> 
> -    if (!(vms->gic_version != VIRT_GIC_VERSION_2 && vms->its) && !vms->no_gicv3_with_gicv2m) {
> +    if (virt_is_its_enabled(vms) && !vms->no_gicv3_with_gicv2m) {
>        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 9121eb37eb..1bebbc265d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -735,7 +735,7 @@ static void create_its(VirtMachineState *vms)
> {
>    DeviceState *dev;
> 
> -    assert(vms->its);
> +    assert(virt_is_its_enabled(vms));
>    if (!kvm_irqchip_in_kernel() && !vms->tcg_its) {
>        /*
>         * Do nothing if ITS is neither supported by the host nor emulated by
> @@ -744,6 +744,15 @@ static void create_its(VirtMachineState *vms)
>        return;
>    }
> 
> +    if (whpx_enabled() && vms->tcg_its) {
> +        /*
> +         * Signal to the user when ITS is neither supported by the host
> +         * nor emulated by the machine.
> +         */
> +        info_report("ITS not supported on WHPX.");
> +        exit(1);
> +    }
> +
>    dev = qdev_new(its_class_name());
> 
>    object_property_set_link(OBJECT(dev), "parent-gicv3", OBJECT(vms->gic),
> @@ -955,7 +964,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
> 
>    fdt_add_gic_node(vms);
> 
> -    if (vms->gic_version != VIRT_GIC_VERSION_2 && vms->its) {
> +    if (vms->gic_version != VIRT_GIC_VERSION_2 && 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);
> @@ -2705,18 +2714,38 @@ static void virt_set_highmem_mmio_size(Object *obj, Visitor *v,
>    extended_memmap[VIRT_HIGH_PCIE_MMIO].size = size;
> }
> 
> -static bool virt_get_its(Object *obj, Error **errp)
> +bool virt_is_its_enabled(VirtMachineState *vms)
> +{
> +    if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +        return false;
> +    }
> +
> +    if (vms->its == ON_OFF_AUTO_OFF) {
> +        return false;
> +    }
> +    if (vms->its == ON_OFF_AUTO_AUTO) {
> +        if (whpx_enabled()) {
> +            return false;
> +        }
> +    }
> +    return true;
> +}
> +
> +static void virt_get_its(Object *obj, Visitor *v, const char *name,
> +                          void *opaque, Error **errp)
> {
>    VirtMachineState *vms = VIRT_MACHINE(obj);
> +    OnOffAuto its = vms->its;
> 
> -    return vms->its;
> +    visit_type_OnOffAuto(v, name, &its, errp);
> }
> 
> -static void virt_set_its(Object *obj, bool value, Error **errp)
> +static void virt_set_its(Object *obj, Visitor *v, const char *name,
> +                          void *opaque, Error **errp)
> {
>    VirtMachineState *vms = VIRT_MACHINE(obj);
> 
> -    vms->its = value;
> +    visit_type_OnOffAuto(v, name, &vms->its, errp);
> }
> 
> static bool virt_get_dtb_randomness(Object *obj, Error **errp)
> @@ -3426,8 +3455,9 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
>                                          "guest CPU which implements the ARM "
>                                          "Memory Tagging Extension");
> 
> -    object_class_property_add_bool(oc, "its", virt_get_its,
> -                                   virt_set_its);
> +    object_class_property_add(oc, "its", "OnOffAuto",
> +        virt_get_its, virt_set_its,
> +        NULL, NULL);
>    object_class_property_set_description(oc, "its",
>                                          "Set on/off to enable/disable "
>                                          "ITS instantiation");
> @@ -3487,8 +3517,8 @@ static void virt_instance_init(Object *obj)
>    vms->highmem_mmio = true;
>    vms->highmem_redists = true;
> 
> -    /* Default allows ITS instantiation */
> -    vms->its = true;
> +    /* Default allows ITS instantiation if available */
> +    vms->its = ON_OFF_AUTO_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 d31348dd61..997dd51678 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -149,7 +149,7 @@ struct VirtMachineState {
>    bool highmem_ecam;
>    bool highmem_mmio;
>    bool highmem_redists;
> -    bool its;
> +    OnOffAuto its;
>    bool tcg_its;
>    bool virt;
>    bool ras;
> @@ -218,4 +218,6 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
>            vms->highmem_redists) ? 2 : 1;
> }
> 
> +bool virt_is_its_enabled(VirtMachineState *vms);
> +
> #endif /* QEMU_ARM_VIRT_H */
> -- 
> 2.50.1 (Apple Git-155)
>