On Tue, 19 Sept 2023 at 12:12, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> 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.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
As mentioned in reply to the cover letter, this needs the hunk below
to avoid using ACPI 6.3 features while claiming compatibility with
ACPI 6.0
With that added,
Reviewed-by: Ard Biesheuvel <a...@kernel.org>
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -811,10 +811,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,
};
> ---
> include/hw/arm/virt.h | 2 ++
> hw/arm/virt-acpi-build.c | 16 ++++++++++++----
> hw/arm/virt.c | 29 ++++++++++++++++++++++++++++-
> 3 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index e1ddbea96be..79b1f9b737d 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -49,6 +49,7 @@
> #define ARCH_TIMER_S_EL1_IRQ 13
> #define ARCH_TIMER_NS_EL1_IRQ 14
> #define ARCH_TIMER_NS_EL2_IRQ 10
> +#define ARCH_TIMER_NS_EL2_VIRT_IRQ 12
>
> #define VIRTUAL_PMU_IRQ 7
>
> @@ -183,6 +184,7 @@ struct VirtMachineState {
> PCIBus *bus;
> char *oem_id;
> char *oem_table_id;
> + bool ns_el2_virt_timer_present;
> };
>
> #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 6b674231c27..7bc120a0f13 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -573,8 +573,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)
> @@ -588,7 +588,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);
> @@ -624,7 +624,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_present) {
> + /* Virtual EL2 Timer GSIV */
> + build_append_int_noprefix(table_data, ARCH_TIMER_NS_EL2_VIRT_IRQ +
> 16, 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);
> }
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8ad78b23c24..4df7cd0a366 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -248,6 +248,19 @@ 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_EL2) && cpu_isar_feature(aa64_vh,
> cpu);
> +}
> +
> static void create_fdt(VirtMachineState *vms)
> {
> MachineState *ms = MACHINE(vms);
> @@ -365,11 +378,20 @@ 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",
> + if (vms->ns_el2_virt_timer_present) {
> + qemu_fdt_setprop_cells(ms->fdt, "/timer", "interrupts",
> + GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
> + GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> + GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_VIRT_IRQ, irqflags,
> + GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL2_IRQ, irqflags,
> + GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL2_VIRT_IRQ,
> irqflags);
> + } else {
> + qemu_fdt_setprop_cells(ms->fdt, "/timer", "interrupts",
> GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
> GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_VIRT_IRQ, irqflags,
> GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL2_IRQ,
> irqflags);
> + }
> }
>
> static void fdt_add_cpu_nodes(const VirtMachineState *vms)
> @@ -810,6 +832,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 (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
> @@ -2249,6 +2272,10 @@ 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_present = ns_el2_virt_timer_present();
> +
> fdt_add_timer_nodes(vms);
> fdt_add_cpu_nodes(vms);
>
> --
> 2.34.1
>
