xen/arm: vtimer: Don't read/use the secure physical timer interrupt for ACPI

Per ACPI 6.5 section 5.2.25 ("Generic Timer Description Table (GTDT)"),
the fields "Secure EL1 Timer GSIV/Flags" are optional and an OS running
in non-secure world is meant to ignore the values.

However, Xen is trying to reserve the value. The ACPI tables for Graviton
2 metal instances will provide the value 0 which is not a correct PPI
(PPIs start at 16) and would have in fact been already reserved by Xen
as this is an SGI. Xen will hit the BUG() and panic().

For the Device-Tree case, I couldn't find a statement suggesting
that the secure physical timer interrupt  is ignored. In fact, I have
found some code in Linux using it as a fallback. That said, it should
never be used.

As I am not aware of any issue when booting using Device-Tree, the
physical timer interrupt is only ignored for ACPI.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 04b07096b165c9b6a11b8976ecbd1644a1ad392f..09cae8138ef21e36e341383cb775894dd09acce3 100644 (file)
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -78,10 +78,6 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *header)
     irq_set_type(gtdt->non_secure_el1_interrupt, irq_type);
     timer_irq[TIMER_PHYS_NONSECURE_PPI] = gtdt->non_secure_el1_interrupt;
 
-    irq_type = acpi_get_timer_irq_type(gtdt->secure_el1_flags);
-    irq_set_type(gtdt->secure_el1_interrupt, irq_type);
-    timer_irq[TIMER_PHYS_SECURE_PPI] = gtdt->secure_el1_interrupt;
-
     irq_type = acpi_get_timer_irq_type(gtdt->virtual_timer_flags);
     irq_set_type(gtdt->virtual_timer_interrupt, irq_type);
     timer_irq[TIMER_VIRT_PPI] = gtdt->virtual_timer_interrupt;

diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index c54360e20266d0e257b661892dcdaab5d7b9d013..d2124b17552155c9bb50e4af60f6b62763878ff3 100644 (file)
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -8,6 +8,7 @@
  * Copyright (c) 2011 Citrix Systems.
  */
 
+#include <xen/acpi.h>
 #include <xen/lib.h>
 #include <xen/perfc.h>
 #include <xen/sched.h>
@@ -61,10 +62,22 @@ int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
 
     config->clock_frequency = timer_dt_clock_frequency;
 
-    /* At this stage vgic_reserve_virq can't fail */
+    /*
+     * Per the ACPI specification, providing a secure EL1 timer
+     * interrupt is optional and will be ignored by non-secure OS.
+     * Therefore don't reserve the interrupt number for the HW domain
+     * and ACPI.
+     *
+     * Note that we should still reserve it when using the Device-Tree
+     * because the interrupt is not optional. That said, we are not
+     * expecting any OS to use it when running on top of Xen.
+     *
+     * At this stage vgic_reserve_virq() is not meant to fail.
+     */
     if ( is_hardware_domain(d) )
     {
-        if ( !vgic_reserve_virq(d, timer_get_irq(TIMER_PHYS_SECURE_PPI)) )
+        if ( acpi_disabled &&
+             !vgic_reserve_virq(d, timer_get_irq(TIMER_PHYS_SECURE_PPI)) )
             BUG();
 
         if ( !vgic_reserve_virq(d, timer_get_irq(TIMER_PHYS_NONSECURE_PPI)) )