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

[patchbot@xxxxxxx]

commit 6432228fb5808150d3c5c14affb3d46af81b3878
Author:     Julien Grall <jgrall@xxxxxxxxxx>
AuthorDate: Thu Oct 5 17:52:41 2023 +0100
Commit:     Julien Grall <jgrall@xxxxxxxxxx>
CommitDate: Mon Oct 16 10:36:16 2023 +0100

    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@xxxxxxxxxx>
    Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
    Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
    Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>
---
 xen/arch/arm/time.c   |  4 ----
 xen/arch/arm/vtimer.c | 17 +++++++++++++++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 04b07096b1..09cae8138e 100644
--- 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 c54360e202..d2124b1755 100644
--- 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)) )
--
generated by git-patchbot for /home/xen/git/xen.git#master