Skip to content

Commit

Permalink
hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ
Browse files Browse the repository at this point in the history 
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>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Message-id: 20240122143537.233498-3-peter.maydell@linaro.org
  • Loading branch information
@pm215
pm215 committed Feb 15, 2024
1 parent 6c1c2e9 commit 1ec896f
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 15 deletions.
20 changes: 14 additions & 6 deletions hw/arm/virt-acpi-build.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
};
Expand Down
60 changes: 51 additions & 9 deletions hw/arm/virt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,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);
Expand Down Expand Up @@ -338,15 +352,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)
Expand Down Expand Up @@ -789,6 +817,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++) {
Expand Down Expand Up @@ -2222,6 +2251,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);

Expand Down Expand Up @@ -3179,8 +3213,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)

Expand Down
2 changes: 2 additions & 0 deletions include/hw/arm/virt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 1ec896f

Please sign in to comment.