All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ
@ 2024-01-22 14:35 Peter Maydell
  2024-01-22 14:35 ` [PATCH v2 1/3] tests/qtest/bios-tables-test: Allow changes to virt GTDT Peter Maydell
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Peter Maydell @ 2024-01-22 14:35 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: Leif Lindholm, Marcin Juszkiewicz, Ard Biesheuvel, Shannon Zhao,
	Michael S. Tsirkin, Igor Mammedov, Ani Sinha

This patchset wires up the NS EL2 virtual timer IRQ on the virt
board, similarly to what commit 058262e0a8b2 did for the sbsa-ref board.

Version 1 was an RFC patchset, originally sent back in autumn:
https://patchew.org/QEMU/20230919101240.2569334-1-peter.maydell@linaro.org/
The main reason for it being an RFC is that the change, while correct,
triggers a bug in EDK2 guest firmware that makes EDK2 assert on bootup.
Since the RFC, we've upgraded our in-tree version of the EDK2 binaries
to a version that has the fix for that bug, so I think the QEMU side of
these patches is ready to go in now.

To accommodate users who might still be using older EDK2 binaries,
we only expose the IRQ in the DTB and ACPI tables for virt-9.0 and
later machine types.

If you see in the guest:
     ASSERT [ArmTimerDxe] /home/kraxel/projects/qemu/roms/edk2/ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.c(72): PropSize == 36 || PropSize == 48

then your options are:
 * update your EDK2 binaries to edk2-stable202311 or newer
 * use the 'virt-8.2' versioned machine type
 * not use 'virtualization=on'
                   
I'll put something about this into the release notes when this
goes into git. (There are other reasons why you probably want a
newer EDK2 for AArch64 guests, so this is worth flagging up to our
downstream distros who don't take our pre-built firmware binaries.)

changes v1->v2:
 * the change in DTB and ACPI tables is now tied to the machine version
 * handle change of the ARCH_TIMER_*_IRQ values from PPI numbers to INTIDs
 * bump the FADT header to indicate ACPI v6.3, since we might be using
   a 6.3 feature in the GTDT
 * the avocado tests now all pass, because we have updated our copy
   of EDK2 in pc-bios/ to a version which has the fix for the bug
   which would otherwise cause it to assert on bootup
 * patch 2 commit message improved to give details of the EDK2 assert and
   state the options for dealing with it (this will also go into the
   QEMU release notes)

thanks
-- PMM

Peter Maydell (3):
  tests/qtest/bios-tables-test: Allow changes to virt GTDT
  hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ
  tests/qtest/bios-tables-tests: Update virt golden reference

 include/hw/arm/virt.h     |   2 ++
 hw/arm/virt-acpi-build.c  |  20 +++++++++----
 hw/arm/virt.c             |  60 ++++++++++++++++++++++++++++++++------
 tests/data/acpi/virt/FACP | Bin 276 -> 276 bytes
 tests/data/acpi/virt/GTDT | Bin 96 -> 104 bytes
 5 files changed, 67 insertions(+), 15 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] tests/qtest/bios-tables-test: Allow changes to virt GTDT
  2024-01-22 14:35 [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
@ 2024-01-22 14:35 ` Peter Maydell
  2024-01-22 14:35 ` [PATCH v2 2/3] hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ Peter Maydell
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2024-01-22 14:35 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: Leif Lindholm, Marcin Juszkiewicz, Ard Biesheuvel, Shannon Zhao,
	Michael S. Tsirkin, Igor Mammedov, Ani Sinha

Allow changes to the virt GTDT -- we are going to add the IRQ
entry for a new timer to it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf..7a6d4f80214 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/virt/FACP",
+"tests/data/acpi/virt/GTDT",
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ
  2024-01-22 14:35 [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
  2024-01-22 14:35 ` [PATCH v2 1/3] tests/qtest/bios-tables-test: Allow changes to virt GTDT Peter Maydell
@ 2024-01-22 14:35 ` Peter Maydell
  2024-01-22 14:35 ` [PATCH v2 3/3] tests/qtest/bios-tables-tests: Update virt golden reference Peter Maydell
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2024-01-22 14:35 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: Leif Lindholm, Marcin Juszkiewicz, Ard Biesheuvel, Shannon Zhao,
	Michael S. Tsirkin, Igor Mammedov, Ani Sinha

Armv8.1+ CPUs have the Virtual Host Extension (VHE) which adds a
non-secure EL2 virtual timer.  We implemented the timer itself in the
CPU model, but never wired up its IRQ line to the GIC.

Wire up the IRQ line (this is always safe whether the CPU has the
interrupt or not, since it always creates the outbound IRQ line).
Report it to the guest via dtb and ACPI if the CPU has the feature.

The DTB binding is documented in the kernel's
Documentation/devicetree/bindings/timer/arm\,arch_timer.yaml
and the ACPI table entries are documented in the ACPI specification
version 6.3 or later.

Because the IRQ line ACPI binding is new in 6.3, we need to bump the
FADT table rev to show that we might be using 6.3 features.

Note that exposing this IRQ in the DTB will trigger a bug in EDK2
versions prior to edk2-stable202311, for users who use the virt board
with 'virtualization=on' to enable EL2 emulation and are booting an
EDK2 guest BIOS, if that EDK2 has assertions enabled.  The effect is
that EDK2 will assert on bootup:

 ASSERT [ArmTimerDxe] /home/kraxel/projects/qemu/roms/edk2/ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.c(72): PropSize == 36 || PropSize == 48

If you see that assertion you should do one of:
 * update your EDK2 binaries to edk2-stable202311 or newer
 * use the 'virt-8.2' versioned machine type
 * not use 'virtualization=on'

(The versions shipped with QEMU itself have the fix.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/virt.h    |  2 ++
 hw/arm/virt-acpi-build.c | 20 ++++++++++----
 hw/arm/virt.c            | 60 ++++++++++++++++++++++++++++++++++------
 3 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index f69239850e6..bb486d36b14 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -130,6 +130,7 @@ struct VirtMachineClass {
     /* Machines < 6.2 have no support for describing cpu topology to guest */
     bool no_cpu_topology;
     bool no_tcg_lpa2;
+    bool no_ns_el2_virt_timer_irq;
 };
 
 struct VirtMachineState {
@@ -173,6 +174,7 @@ struct VirtMachineState {
     PCIBus *bus;
     char *oem_id;
     char *oem_table_id;
+    bool ns_el2_virt_timer_irq;
 };
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a22a2f43a56..903ddf4a361 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -533,8 +533,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 }
 
 /*
- * ACPI spec, Revision 5.1
- * 5.2.24 Generic Timer Description Table (GTDT)
+ * ACPI spec, Revision 6.5
+ * 5.2.25 Generic Timer Description Table (GTDT)
  */
 static void
 build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
@@ -548,7 +548,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     uint32_t irqflags = vmc->claim_edge_triggered_timers ?
         1 : /* Interrupt is Edge triggered */
         0;  /* Interrupt is Level triggered  */
-    AcpiTable table = { .sig = "GTDT", .rev = 2, .oem_id = vms->oem_id,
+    AcpiTable table = { .sig = "GTDT", .rev = 3, .oem_id = vms->oem_id,
                         .oem_table_id = vms->oem_table_id };
 
     acpi_table_begin(&table, table_data);
@@ -584,7 +584,15 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     build_append_int_noprefix(table_data, 0, 4);
     /* Platform Timer Offset */
     build_append_int_noprefix(table_data, 0, 4);
-
+    if (vms->ns_el2_virt_timer_irq) {
+        /* Virtual EL2 Timer GSIV */
+        build_append_int_noprefix(table_data, ARCH_TIMER_NS_EL2_VIRT_IRQ, 4);
+        /* Virtual EL2 Timer Flags */
+        build_append_int_noprefix(table_data, irqflags, 4);
+    } else {
+        build_append_int_noprefix(table_data, 0, 4);
+        build_append_int_noprefix(table_data, 0, 4);
+    }
     acpi_table_end(linker, &table);
 }
 
@@ -771,10 +779,10 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
                             VirtMachineState *vms, unsigned dsdt_tbl_offset)
 {
-    /* ACPI v6.0 */
+    /* ACPI v6.3 */
     AcpiFadtData fadt = {
         .rev = 6,
-        .minor_ver = 0,
+        .minor_ver = 3,
         .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
         .xdsdt_tbl_offset = &dsdt_tbl_offset,
     };
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5cbc69dff83..fa7c233346e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -218,6 +218,20 @@ static void create_randomness(MachineState *ms, const char *node)
     qemu_fdt_setprop(ms->fdt, node, "rng-seed", seed.rng, sizeof(seed.rng));
 }
 
+/*
+ * The CPU object always exposes the NS EL2 virt timer IRQ line,
+ * but we don't want to advertise it to the guest in the dtb or ACPI
+ * table unless it's really going to do something.
+ */
+static bool ns_el2_virt_timer_present(void)
+{
+    ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
+    CPUARMState *env = &cpu->env;
+
+    return arm_feature(env, ARM_FEATURE_AARCH64) &&
+        arm_feature(env, ARM_FEATURE_EL2) && cpu_isar_feature(aa64_vh, cpu);
+}
+
 static void create_fdt(VirtMachineState *vms)
 {
     MachineState *ms = MACHINE(vms);
@@ -335,15 +349,29 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
                                 "arm,armv7-timer");
     }
     qemu_fdt_setprop(ms->fdt, "/timer", "always-on", NULL, 0);
-    qemu_fdt_setprop_cells(ms->fdt, "/timer", "interrupts",
-                           GIC_FDT_IRQ_TYPE_PPI,
-                           INTID_TO_PPI(ARCH_TIMER_S_EL1_IRQ), irqflags,
-                           GIC_FDT_IRQ_TYPE_PPI,
-                           INTID_TO_PPI(ARCH_TIMER_NS_EL1_IRQ), irqflags,
-                           GIC_FDT_IRQ_TYPE_PPI,
-                           INTID_TO_PPI(ARCH_TIMER_VIRT_IRQ), irqflags,
-                           GIC_FDT_IRQ_TYPE_PPI,
-                           INTID_TO_PPI(ARCH_TIMER_NS_EL2_IRQ), irqflags);
+    if (vms->ns_el2_virt_timer_irq) {
+        qemu_fdt_setprop_cells(ms->fdt, "/timer", "interrupts",
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_S_EL1_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_NS_EL1_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_VIRT_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_NS_EL2_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_NS_EL2_VIRT_IRQ), irqflags);
+    } else {
+        qemu_fdt_setprop_cells(ms->fdt, "/timer", "interrupts",
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_S_EL1_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_NS_EL1_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_VIRT_IRQ), irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI,
+                               INTID_TO_PPI(ARCH_TIMER_NS_EL2_IRQ), irqflags);
+    }
 }
 
 static void fdt_add_cpu_nodes(const VirtMachineState *vms)
@@ -786,6 +814,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
             [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
             [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
             [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
+            [GTIMER_HYPVIRT] = ARCH_TIMER_NS_EL2_VIRT_IRQ,
         };
 
         for (unsigned irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
@@ -2221,6 +2250,11 @@ static void machvirt_init(MachineState *machine)
         qdev_realize(DEVICE(cpuobj), NULL, &error_fatal);
         object_unref(cpuobj);
     }
+
+    /* Now we've created the CPUs we can see if they have the hypvirt timer */
+    vms->ns_el2_virt_timer_irq = ns_el2_virt_timer_present() &&
+        !vmc->no_ns_el2_virt_timer_irq;
+
     fdt_add_timer_nodes(vms);
     fdt_add_cpu_nodes(vms);
 
@@ -3178,8 +3212,16 @@ DEFINE_VIRT_MACHINE_AS_LATEST(9, 0)
 
 static void virt_machine_8_2_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_9_0_options(mc);
     compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len);
+    /*
+     * Don't expose NS_EL2_VIRT timer IRQ in DTB on ACPI on 8.2 and
+     * earlier machines. (Exposing it tickles a bug in older EDK2
+     * guest BIOS binaries.)
+     */
+    vmc->no_ns_el2_virt_timer_irq = true;
 }
 DEFINE_VIRT_MACHINE(8, 2)
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] tests/qtest/bios-tables-tests: Update virt golden reference
  2024-01-22 14:35 [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
  2024-01-22 14:35 ` [PATCH v2 1/3] tests/qtest/bios-tables-test: Allow changes to virt GTDT Peter Maydell
  2024-01-22 14:35 ` [PATCH v2 2/3] hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ Peter Maydell
@ 2024-01-22 14:35 ` Peter Maydell
  2024-02-01 13:33 ` [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
  2024-02-14  8:55 ` Ard Biesheuvel
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2024-01-22 14:35 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: Leif Lindholm, Marcin Juszkiewicz, Ard Biesheuvel, Shannon Zhao,
	Michael S. Tsirkin, Igor Mammedov, Ani Sinha

Update the virt golden reference files to say that the FACP is ACPI
v6.3, and the GTDT table is a revision 3 table with space for the
virtual EL2 timer.

Diffs from iasl:

@@ -1,32 +1,32 @@
 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20200925 (64-bit version)
  * Copyright (c) 2000 - 2020 Intel Corporation
  *
- * Disassembly of tests/data/acpi/virt/FACP, Mon Jan 22 13:48:40 2024
+ * Disassembly of /tmp/aml-W8RZH2, Mon Jan 22 13:48:40 2024
  *
  * ACPI Data Table [FACP]
  *
  * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue
  */

 [000h 0000   4]                    Signature : "FACP"    [Fixed ACPI Description Table (FADT)]
 [004h 0004   4]                 Table Length : 00000114
 [008h 0008   1]                     Revision : 06
-[009h 0009   1]                     Checksum : 15
+[009h 0009   1]                     Checksum : 12
 [00Ah 0010   6]                       Oem ID : "BOCHS "
 [010h 0016   8]                 Oem Table ID : "BXPC    "
 [018h 0024   4]                 Oem Revision : 00000001
 [01Ch 0028   4]              Asl Compiler ID : "BXPC"
 [020h 0032   4]        Asl Compiler Revision : 00000001

 [024h 0036   4]                 FACS Address : 00000000
 [028h 0040   4]                 DSDT Address : 00000000
 [02Ch 0044   1]                        Model : 00
 [02Dh 0045   1]                   PM Profile : 00 [Unspecified]
 [02Eh 0046   2]                SCI Interrupt : 0000
 [030h 0048   4]             SMI Command Port : 00000000
 [034h 0052   1]            ACPI Enable Value : 00
 [035h 0053   1]           ACPI Disable Value : 00
 [036h 0054   1]               S4BIOS Command : 00
 [037h 0055   1]              P-State Control : 00
@@ -86,33 +86,33 @@
      Use APIC Physical Destination Mode (V4) : 0
                        Hardware Reduced (V5) : 1
                       Low Power S0 Idle (V5) : 0

 [074h 0116  12]               Reset Register : [Generic Address Structure]
 [074h 0116   1]                     Space ID : 00 [SystemMemory]
 [075h 0117   1]                    Bit Width : 00
 [076h 0118   1]                   Bit Offset : 00
 [077h 0119   1]         Encoded Access Width : 00 [Undefined/Legacy]
 [078h 0120   8]                      Address : 0000000000000000

 [080h 0128   1]         Value to cause reset : 00
 [081h 0129   2]    ARM Flags (decoded below) : 0003
                               PSCI Compliant : 1
                        Must use HVC for PSCI : 1

-[083h 0131   1]          FADT Minor Revision : 00
+[083h 0131   1]          FADT Minor Revision : 03
 [084h 0132   8]                 FACS Address : 0000000000000000
 [08Ch 0140   8]                 DSDT Address : 0000000000000000
 [094h 0148  12]             PM1A Event Block : [Generic Address Structure]
 [094h 0148   1]                     Space ID : 00 [SystemMemory]
 [095h 0149   1]                    Bit Width : 00
 [096h 0150   1]                   Bit Offset : 00
 [097h 0151   1]         Encoded Access Width : 00 [Undefined/Legacy]
 [098h 0152   8]                      Address : 0000000000000000

 [0A0h 0160  12]             PM1B Event Block : [Generic Address Structure]
 [0A0h 0160   1]                     Space ID : 00 [SystemMemory]
 [0A1h 0161   1]                    Bit Width : 00
 [0A2h 0162   1]                   Bit Offset : 00
 [0A3h 0163   1]         Encoded Access Width : 00 [Undefined/Legacy]
 [0A4h 0164   8]                      Address : 0000000000000000

@@ -164,34 +164,34 @@
 [0F5h 0245   1]                    Bit Width : 00
 [0F6h 0246   1]                   Bit Offset : 00
 [0F7h 0247   1]         Encoded Access Width : 00 [Undefined/Legacy]
 [0F8h 0248   8]                      Address : 0000000000000000

 [100h 0256  12]        Sleep Status Register : [Generic Address Structure]
 [100h 0256   1]                     Space ID : 00 [SystemMemory]
 [101h 0257   1]                    Bit Width : 00
 [102h 0258   1]                   Bit Offset : 00
 [103h 0259   1]         Encoded Access Width : 00 [Undefined/Legacy]
 [104h 0260   8]                      Address : 0000000000000000

 [10Ch 0268   8]                Hypervisor ID : 00000000554D4551


 Raw Table Data: Length 276 (0x114)

-    0000: 46 41 43 50 14 01 00 00 06 15 42 4F 43 48 53 20  // FACP......BOCHS
+    0000: 46 41 43 50 14 01 00 00 06 12 42 4F 43 48 53 20  // FACP......BOCHS
     0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPC    ....BXPC
     0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0070: 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
-    0080: 00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
+    0080: 00 03 00 03 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     00A0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     00B0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     00C0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     00D0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     00E0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     00F0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
     0100: 00 00 00 00 00 00 00 00 00 00 00 00 51 45 4D 55  // ............QEMU
     0110: 00 00 00 00                                      // ....


@@ -1,32 +1,32 @@
 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20200925 (64-bit version)
  * Copyright (c) 2000 - 2020 Intel Corporation
  *
- * Disassembly of tests/data/acpi/virt/GTDT, Mon Jan 22 13:48:40 2024
+ * Disassembly of /tmp/aml-XDSZH2, Mon Jan 22 13:48:40 2024
  *
  * ACPI Data Table [GTDT]
  *
  * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue
  */

 [000h 0000   4]                    Signature : "GTDT"    [Generic Timer Description Table]
-[004h 0004   4]                 Table Length : 00000060
-[008h 0008   1]                     Revision : 02
-[009h 0009   1]                     Checksum : 9C
+[004h 0004   4]                 Table Length : 00000068
+[008h 0008   1]                     Revision : 03
+[009h 0009   1]                     Checksum : 93
 [00Ah 0010   6]                       Oem ID : "BOCHS "
 [010h 0016   8]                 Oem Table ID : "BXPC    "
 [018h 0024   4]                 Oem Revision : 00000001
 [01Ch 0028   4]              Asl Compiler ID : "BXPC"
 [020h 0032   4]        Asl Compiler Revision : 00000001

 [024h 0036   8]        Counter Block Address : FFFFFFFFFFFFFFFF
 [02Ch 0044   4]                     Reserved : 00000000

 [030h 0048   4]         Secure EL1 Interrupt : 0000001D
 [034h 0052   4]    EL1 Flags (decoded below) : 00000000
                                 Trigger Mode : 0
                                     Polarity : 0
                                    Always On : 0

 [038h 0056   4]     Non-Secure EL1 Interrupt : 0000001E
@@ -37,25 +37,28 @@

 [040h 0064   4]      Virtual Timer Interrupt : 0000001B
 [044h 0068   4]     VT Flags (decoded below) : 00000000
                                 Trigger Mode : 0
                                     Polarity : 0
                                    Always On : 0

 [048h 0072   4]     Non-Secure EL2 Interrupt : 0000001A
 [04Ch 0076   4]   NEL2 Flags (decoded below) : 00000000
                                 Trigger Mode : 0
                                     Polarity : 0
                                    Always On : 0
 [050h 0080   8]   Counter Read Block Address : FFFFFFFFFFFFFFFF

 [058h 0088   4]         Platform Timer Count : 00000000
 [05Ch 0092   4]        Platform Timer Offset : 00000000
+[060h 0096   4]       Virtual EL2 Timer GSIV : 00000000
+[064h 0100   4]      Virtual EL2 Timer Flags : 00000000

-Raw Table Data: Length 96 (0x60)
+Raw Table Data: Length 104 (0x68)

-    0000: 47 54 44 54 60 00 00 00 02 9C 42 4F 43 48 53 20  // GTDT`.....BOCHS
+    0000: 47 54 44 54 68 00 00 00 03 93 42 4F 43 48 53 20  // GTDTh.....BOCHS
     0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPC    ....BXPC
     0020: 01 00 00 00 FF FF FF FF FF FF FF FF 00 00 00 00  // ................
     0030: 1D 00 00 00 00 00 00 00 1E 00 00 00 04 00 00 00  // ................
     0040: 1B 00 00 00 00 00 00 00 1A 00 00 00 00 00 00 00  // ................
     0050: FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00  // ................
+    0060: 00 00 00 00 00 00 00 00                          // ........

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   2 --
 tests/data/acpi/virt/FACP                   | Bin 276 -> 276 bytes
 tests/data/acpi/virt/GTDT                   | Bin 96 -> 104 bytes
 3 files changed, 2 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 7a6d4f80214..dfb8523c8bf 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/FACP",
-"tests/data/acpi/virt/GTDT",
diff --git a/tests/data/acpi/virt/FACP b/tests/data/acpi/virt/FACP
index ac05c35a69451519bd1152c54d1e741af36390f5..da0c3644cc4536a0a0141603ed470bd11492b678 100644
GIT binary patch
delta 25
gcmbQjG=+)F&CxkPgpq-PO=u!l<;2F$$vli407<0<)c^nh

delta 28
kcmbQjG=+)F&CxkPgpq-PO>`nx<-|!<6Akz$^DuG%0AAS!ssI20

diff --git a/tests/data/acpi/virt/GTDT b/tests/data/acpi/virt/GTDT
index 6f8cb9b8f30b55f4c93fe515982621e3db50feb2..7f330e04d144f9cc22eef06127ecc19abf9e8009 100644
GIT binary patch
delta 25
bcmYeu;BpUf3CUn!U|^m+kt>V?$N&QXMtB4L

delta 16
Xcmc~u;BpUf2}xjJU|^avkt+-UB60)u

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ
  2024-01-22 14:35 [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
                   ` (2 preceding siblings ...)
  2024-01-22 14:35 ` [PATCH v2 3/3] tests/qtest/bios-tables-tests: Update virt golden reference Peter Maydell
@ 2024-02-01 13:33 ` Peter Maydell
  2024-02-14  8:55 ` Ard Biesheuvel
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2024-02-01 13:33 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: Leif Lindholm, Marcin Juszkiewicz, Ard Biesheuvel, Shannon Zhao,
	Michael S. Tsirkin, Igor Mammedov, Ani Sinha

Ping for code review, please?

thanks
-- PMM

On Mon, 22 Jan 2024 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset wires up the NS EL2 virtual timer IRQ on the virt
> board, similarly to what commit 058262e0a8b2 did for the sbsa-ref board.
>
> Version 1 was an RFC patchset, originally sent back in autumn:
> https://patchew.org/QEMU/20230919101240.2569334-1-peter.maydell@linaro.org/
> The main reason for it being an RFC is that the change, while correct,
> triggers a bug in EDK2 guest firmware that makes EDK2 assert on bootup.
> Since the RFC, we've upgraded our in-tree version of the EDK2 binaries
> to a version that has the fix for that bug, so I think the QEMU side of
> these patches is ready to go in now.
>
> To accommodate users who might still be using older EDK2 binaries,
> we only expose the IRQ in the DTB and ACPI tables for virt-9.0 and
> later machine types.
>
> If you see in the guest:
>      ASSERT [ArmTimerDxe] /home/kraxel/projects/qemu/roms/edk2/ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.c(72): PropSize == 36 || PropSize == 48
>
> then your options are:
>  * update your EDK2 binaries to edk2-stable202311 or newer
>  * use the 'virt-8.2' versioned machine type
>  * not use 'virtualization=on'
>
> I'll put something about this into the release notes when this
> goes into git. (There are other reasons why you probably want a
> newer EDK2 for AArch64 guests, so this is worth flagging up to our
> downstream distros who don't take our pre-built firmware binaries.)
>
> changes v1->v2:
>  * the change in DTB and ACPI tables is now tied to the machine version
>  * handle change of the ARCH_TIMER_*_IRQ values from PPI numbers to INTIDs
>  * bump the FADT header to indicate ACPI v6.3, since we might be using
>    a 6.3 feature in the GTDT
>  * the avocado tests now all pass, because we have updated our copy
>    of EDK2 in pc-bios/ to a version which has the fix for the bug
>    which would otherwise cause it to assert on bootup
>  * patch 2 commit message improved to give details of the EDK2 assert and
>    state the options for dealing with it (this will also go into the
>    QEMU release notes)
>
> thanks
> -- PMM
>
> Peter Maydell (3):
>   tests/qtest/bios-tables-test: Allow changes to virt GTDT
>   hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ
>   tests/qtest/bios-tables-tests: Update virt golden reference
>
>  include/hw/arm/virt.h     |   2 ++
>  hw/arm/virt-acpi-build.c  |  20 +++++++++----
>  hw/arm/virt.c             |  60 ++++++++++++++++++++++++++++++++------
>  tests/data/acpi/virt/FACP | Bin 276 -> 276 bytes
>  tests/data/acpi/virt/GTDT | Bin 96 -> 104 bytes
>  5 files changed, 67 insertions(+), 15 deletions(-)
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ
  2024-01-22 14:35 [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
                   ` (3 preceding siblings ...)
  2024-02-01 13:33 ` [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
@ 2024-02-14  8:55 ` Ard Biesheuvel
  4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2024-02-14  8:55 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel, Leif Lindholm, Marcin Juszkiewicz,
	Shannon Zhao, Michael S. Tsirkin, Igor Mammedov, Ani Sinha

On Mon, 22 Jan 2024 at 15:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset wires up the NS EL2 virtual timer IRQ on the virt
> board, similarly to what commit 058262e0a8b2 did for the sbsa-ref board.
>
> Version 1 was an RFC patchset, originally sent back in autumn:
> https://patchew.org/QEMU/20230919101240.2569334-1-peter.maydell@linaro.org/
> The main reason for it being an RFC is that the change, while correct,
> triggers a bug in EDK2 guest firmware that makes EDK2 assert on bootup.
> Since the RFC, we've upgraded our in-tree version of the EDK2 binaries
> to a version that has the fix for that bug, so I think the QEMU side of
> these patches is ready to go in now.
>
> To accommodate users who might still be using older EDK2 binaries,
> we only expose the IRQ in the DTB and ACPI tables for virt-9.0 and
> later machine types.
>
> If you see in the guest:
>      ASSERT [ArmTimerDxe] /home/kraxel/projects/qemu/roms/edk2/ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.c(72): PropSize == 36 || PropSize == 48
>
> then your options are:
>  * update your EDK2 binaries to edk2-stable202311 or newer
>  * use the 'virt-8.2' versioned machine type
>  * not use 'virtualization=on'
>
> I'll put something about this into the release notes when this
> goes into git. (There are other reasons why you probably want a
> newer EDK2 for AArch64 guests, so this is worth flagging up to our
> downstream distros who don't take our pre-built firmware binaries.)
>
> changes v1->v2:
>  * the change in DTB and ACPI tables is now tied to the machine version
>  * handle change of the ARCH_TIMER_*_IRQ values from PPI numbers to INTIDs
>  * bump the FADT header to indicate ACPI v6.3, since we might be using
>    a 6.3 feature in the GTDT
>  * the avocado tests now all pass, because we have updated our copy
>    of EDK2 in pc-bios/ to a version which has the fix for the bug
>    which would otherwise cause it to assert on bootup
>  * patch 2 commit message improved to give details of the EDK2 assert and
>    state the options for dealing with it (this will also go into the
>    QEMU release notes)
>
> thanks
> -- PMM
>
> Peter Maydell (3):
>   tests/qtest/bios-tables-test: Allow changes to virt GTDT
>   hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ
>   tests/qtest/bios-tables-tests: Update virt golden reference
>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-02-14  8:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 14:35 [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
2024-01-22 14:35 ` [PATCH v2 1/3] tests/qtest/bios-tables-test: Allow changes to virt GTDT Peter Maydell
2024-01-22 14:35 ` [PATCH v2 2/3] hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ Peter Maydell
2024-01-22 14:35 ` [PATCH v2 3/3] tests/qtest/bios-tables-tests: Update virt golden reference Peter Maydell
2024-02-01 13:33 ` [PATCH v2 0/3] virt: wire up NS EL2 virtual timer IRQ Peter Maydell
2024-02-14  8:55 ` Ard Biesheuvel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.