[PATCH v5 02/18] hw/arm: virt: add GICv2m for the case when ITS is not available

Mohamed Mediouni posted 18 patches 4 months, 1 week ago
Maintainers: Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Sunil Muthuswamy <sunilmut@microsoft.com>, Mohamed Mediouni <mohamed@unpredictable.fr>, Peter Maydell <peter.maydell@linaro.org>, Shannon Zhao <shannon.zhaosl@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Alexander Graf <agraf@csgraf.de>
There is a newer version of this series
[PATCH v5 02/18] hw/arm: virt: add GICv2m for the case when ITS is not available
Posted by Mohamed Mediouni 4 months, 1 week ago
On Hypervisor.framework for macOS and WHPX for Windows, the provided environment is a GICv3 without ITS.

As such, support a GICv3 w/ GICv2m for that scenario.

Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 hw/arm/virt-acpi-build.c | 4 +++-
 hw/arm/virt.c            | 8 ++++++++
 include/hw/arm/virt.h    | 2 ++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index b01fc4f8ef..969fa3f686 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -848,7 +848,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
             build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
             build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
         }
-    } else {
+    }
+
+    if (!vms->its && !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 ef6be3660f..5951b331f3 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -953,6 +953,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
 
     if (vms->gic_version != VIRT_GIC_VERSION_2 && vms->its) {
         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) {
         create_v2m(vms);
     }
@@ -2402,6 +2404,8 @@ static void machvirt_init(MachineState *machine)
     vms->ns_el2_virt_timer_irq = ns_el2_virt_timer_present() &&
         !vmc->no_ns_el2_virt_timer_irq;
 
+    vms->no_gicv3_with_gicv2m = vmc->no_gicv3_with_gicv2m;
+
     fdt_add_timer_nodes(vms);
     fdt_add_cpu_nodes(vms);
 
@@ -3410,6 +3414,7 @@ static void virt_instance_init(Object *obj)
     vms->its = true;
     /* Allow ITS emulation if the machine version supports it */
     vms->tcg_its = !vmc->no_tcg_its;
+    vms->no_gicv3_with_gicv2m = false;
 
     /* Default disallows iommu instantiation */
     vms->iommu = VIRT_IOMMU_NONE;
@@ -3462,8 +3467,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(10, 1)
 
 static void virt_machine_10_0_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_10_1_options(mc);
     compat_props_add(mc->compat_props, hw_compat_10_0, hw_compat_10_0_len);
+    vmc->no_gicv3_with_gicv2m = true;
 }
 DEFINE_VIRT_MACHINE(10, 0)
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 365a28b082..725ec18fd2 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -131,6 +131,7 @@ struct VirtMachineClass {
     bool no_cpu_topology;
     bool no_tcg_lpa2;
     bool no_ns_el2_virt_timer_irq;
+    bool no_gicv3_with_gicv2m;
     bool no_nested_smmu;
 };
 
@@ -178,6 +179,7 @@ struct VirtMachineState {
     char *oem_id;
     char *oem_table_id;
     bool ns_el2_virt_timer_irq;
+    bool no_gicv3_with_gicv2m;
     CXLState cxl_devices_state;
 };
 
-- 
2.39.5 (Apple Git-154)
Re: [PATCH v5 02/18] hw/arm: virt: add GICv2m for the case when ITS is not available
Posted by Peter Maydell 4 months ago
On Fri, 8 Aug 2025 at 07:54, Mohamed Mediouni <mohamed@unpredictable.fr> wrote:
>
> On Hypervisor.framework for macOS and WHPX for Windows, the provided environment is a GICv3 without ITS.
>
> As such, support a GICv3 w/ GICv2m for that scenario.
>
> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

This breaks the bios-tables-test which is run as part of "make check",
because the ACPI tables generated no longer match the golden reference
output. If the ACPI table change here is intentional then the comment
at the top of tests/qtest/bios-tables-test.c describes the process
and sequence of patches you need to update the reference versions.


$ make -C build/arm-clang/ -j6 && (cd build/arm-clang/ &&
QTEST_QEMU_BINARY=./qemu-system-aarch64
./tests/qtest/bios-tables-test)
make: Entering directory
'/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang'
[1/28] Generating qemu-version.h with a custom command (wrapped by
meson to capture output)
[2/8] Compiling C object libsystem.a.p/hw_pci-host_gpex-acpi.c.o
[3/8] Compiling C object libqemu-arm-softmmu.a.p/hw_arm_virt-acpi-build.c.o
[4/8] Compiling C object libqemu-aarch64-softmmu.a.p/hw_arm_virt-acpi-build.c.o
[5/8] Compiling C object libqemu-arm-softmmu.a.p/hw_arm_virt.c.o
[6/8] Compiling C object libqemu-aarch64-softmmu.a.p/hw_arm_virt.c.o
[7/8] Linking target qemu-system-arm
[8/8] Linking target qemu-system-aarch64
make: Leaving directory
'/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang'
TAP version 13
# random seed: R02Sbd44a41112c6e2df4635231a2a1f3510
# starting QEMU: exec ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-2147871.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-2147871.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine none
-accel qtest
# starting QEMU: exec ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-2147871.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-2147871.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine none
-accel qtest
1..12
# Start of aarch64 tests
# Start of acpi tests
# starting QEMU: exec ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-2147871.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-2147871.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine virt
-accel tcg -nodefaults -nographic -drive
if=pflash,format=raw,file=pc-bios/edk2-aarch64-code.fd,readonly=on
-drive if=pflash,format=raw,file=pc-bios/edk2-arm-vars.fd,snapshot=on
-cdrom tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2
-cpu cortex-a57 -machine ras=on -smbios
type=4,max-speed=2900,current-speed=2700 -accel qtest
acpi-test: Warning! APIC binary file mismatch. Actual
[aml:/tmp/aml-0EHGB3], Expected
[aml:tests/data/acpi/aarch64/virt/APIC].
See source file tests/qtest/bios-tables-test.c for instructions on how
to update expected files.
acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-48FGB3.dsl,
aml:/tmp/aml-0EHGB3], Expected [asl:/tmp/asl-D5NHB3.dsl,
aml:tests/data/acpi/aarch64/virt/APIC].
**
ERROR:../../tests/qtest/bios-tables-test.c:554:test_acpi_asl:
assertion failed: (all_tables_match)
not ok /aarch64/acpi/virt -
ERROR:../../tests/qtest/bios-tables-test.c:554:test_acpi_asl:
assertion failed: (all_tables_match)
Bail out!
Aborted (core dumped)


thanks
-- PMM
Re: [PATCH v5 02/18] hw/arm: virt: add GICv2m for the case when ITS is not available
Posted by Peter Maydell 4 months ago
On Fri, 8 Aug 2025 at 07:54, Mohamed Mediouni <mohamed@unpredictable.fr> wrote:
>
> On Hypervisor.framework for macOS and WHPX for Windows, the provided environment is a GICv3 without ITS.
>
> As such, support a GICv3 w/ GICv2m for that scenario.
>
> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>

We should also update the docs in docs/system/arm/virt.rst.
These are already incorrect in that we never updated them
when we implemented the ITS:

- An MSI controller (GICv2M or ITS). GICv2M is selected by default along
  with GICv2. ITS is selected by default with GICv3 (>= virt-2.7). Note
  that ITS is not modeled in TCG mode.

so we should update that bullet point to match current reality.
(If it is looking a bit big then we can have the bullet point
be just "An MSI controller (GICv2M or ITS)" and move the explanation
of when we go with GICv2M vs ITS to a paragraph below the
bullet list, in the same way we have for the explanation about
the second NS UART.

-- PMM