diff mbox

[Xen-devel,RFC,16/35] ARM64 / ACPI: Parse GTDT to initialize timer

Message ID 1423058539-26403-17-git-send-email-parth.dixit@linaro.org
State New
Headers show

Commit Message

Parth Dixit Feb. 4, 2015, 2:02 p.m. UTC
From: Naresh Bhat <naresh.bhat@linaro.org>

Parse GTDT (Generic Timer Descriptor Table) to initialize timer.
Using the information presented by GTDT to initialize the arch
timer (not momery-mapped).

Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
---
 xen/arch/arm/setup.c   |  6 +++++
 xen/arch/arm/time.c    | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/time.h |  1 +
 3 files changed, 73 insertions(+)

Comments

Julien Grall Feb. 4, 2015, 9:51 p.m. UTC | #1
Hi Parth,

On 04/02/2015 14:02, parth.dixit@linaro.org wrote:
> From: Naresh Bhat <naresh.bhat@linaro.org>
>
> Parse GTDT (Generic Timer Descriptor Table) to initialize timer.
> Using the information presented by GTDT to initialize the arch
> timer (not momery-mapped).

memory-mapped

> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> ---
>   xen/arch/arm/setup.c   |  6 +++++
>   xen/arch/arm/time.c    | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   xen/include/xen/time.h |  1 +
>   3 files changed, 73 insertions(+)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 569b2da..af9f429 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -775,7 +775,12 @@ void __init start_xen(unsigned long boot_phys_offset,
>       smp_init_cpus();
>       cpus = smp_get_max_cpus();
>
> +/* Comment for now take it after GIC initialization */
> +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM_64)
> +   init_xen_acpi_time();
> +#else
>       init_xen_time();
> +#endif

This is clearly wrong. A same Xen binary should run on both ACPI and DT 
when ACPI is enabled.

Also, I would take care of the different between ACPI and DT in 
init_xen_time, rather than ifdefining here. It will make the code more 
cleaner.

>       gic_init();
>
> @@ -789,6 +794,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>       xsm_dt_init();
>
>       init_maintenance_interrupt();
> +

Spurious change.

>       init_timer_interrupt();
>
>       timer_init();
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index 0add494..0d4c26d 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -29,6 +29,7 @@
>   #include <xen/time.h>
>   #include <xen/sched.h>
>   #include <xen/event.h>
> +#include <xen/acpi.h>
>   #include <asm/system.h>
>   #include <asm/time.h>
>   #include <asm/gic.h>
> @@ -239,6 +240,71 @@ struct tm wallclock_time(uint64_t *ns)
>       return (struct tm) { 0 };
>   }
>
> +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI)

#ifdef CONFIG_ACPI

> +#define ACPI_GTDT_INTR_MASK 0x3
> +
> +static int GTDT_INTRL_TAB[] ={ DT_IRQ_TYPE_LEVEL_HIGH, DT_IRQ_TYPE_EDGE_RISING,
> +                          DT_IRQ_TYPE_LEVEL_LOW ,DT_IRQ_TYPE_EDGE_FALLING};
> +
> +/* Initialize per-processor generic timer */
> +static int __init arch_timer_acpi_init(struct acpi_table_header *table)
> +{
> +    int res;
> +    int type;
> +    struct acpi_table_gtdt *gtdt;
> +
> +    gtdt = (struct acpi_table_gtdt *)table;
> +
> +    /* Initialize all the generic timer IRQ variable from GTDT table */
> +
> +    type = GTDT_INTRL_TAB[gtdt->non_secure_el1_flags & ACPI_GTDT_INTR_MASK];
> +    acpi_set_irq(gtdt->non_secure_el1_interrupt, type);
> +    timer_irq[TIMER_PHYS_NONSECURE_PPI] = gtdt->non_secure_el1_interrupt;
> +
> +    type = GTDT_INTRL_TAB[gtdt->secure_el1_flags & ACPI_GTDT_INTR_MASK];
> +    acpi_set_irq(gtdt->secure_el1_interrupt, type);
> +    timer_irq[TIMER_PHYS_SECURE_PPI] = gtdt->secure_el1_interrupt;
> +
> +    type = GTDT_INTRL_TAB[gtdt->non_secure_el2_flags & ACPI_GTDT_INTR_MASK];
> +    acpi_set_irq(gtdt->non_secure_el2_interrupt, type);
> +    timer_irq[TIMER_HYP_PPI] = gtdt->non_secure_el2_interrupt;
> +
> +    type = GTDT_INTRL_TAB[gtdt->virtual_timer_flags & ACPI_GTDT_INTR_MASK];
> +    acpi_set_irq(gtdt->virtual_timer_interrupt, type);
> +    timer_irq[TIMER_VIRT_PPI] = gtdt->virtual_timer_interrupt;
> +
> +    printk("Generic Timer IRQ from ACPI GTDT: phys=%u hyp=%u virt=%u\n",
> +           timer_irq[TIMER_PHYS_NONSECURE_PPI],
> +           timer_irq[TIMER_HYP_PPI],
> +           timer_irq[TIMER_VIRT_PPI]);
> +
> +    res = platform_init_time();

The platform code is DT-centrict. Although, most of the code below is 
similar to the Timer DT initialization. Please factor it.

> +    if ( res )
> +        printk("Timer: Cannot initialize platform timer");
> +
> +    /* Check that this CPU supports the Generic Timer interface */
> +    if ( !cpu_has_gentimer )
> +        printk("CPU does not support the Generic Timer v1 interface");
> +
> +    cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
> +
> +    boot_count = READ_SYSREG64(CNTPCT_EL0);
> +
> +    printk("Using generic timer at %lu KHz\n", cpu_khz);
> +
> +    return 0;
> +}
> +
> +int __init init_xen_acpi_time(void)
> +{
> +   /* Initialize all the generic timers presented in GTDT */
> +   acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
> +   return 0;
> +}
> +
> +#endif
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/include/xen/time.h b/xen/include/xen/time.h
> index 709501f..4598a0c 100644
> --- a/xen/include/xen/time.h
> +++ b/xen/include/xen/time.h
> @@ -11,6 +11,7 @@
>   #include <xen/types.h>
>   #include <public/xen.h>
>
> +extern int init_xen_acpi_time(void);

This is ARM specific, please move the prototype in asm-arm/time.h.

>   extern int init_xen_time(void);
>   extern void cstate_restore_tsc(void);
>
>

Regards,
Julien Grall Feb. 5, 2015, 2:26 p.m. UTC | #2
Hi Ian,

On 05/02/2015 19:39, Ian Campbell wrote:
> On Wed, 2015-02-04 at 21:51 +0000, Julien Grall wrote:
> Perhaps platform_* should all grow an "ASSERT(!in acpi mode)" (or
> "ASSERT(in dt mode)") to help enforce this.

That would be good. We could also do the same for the device tree to 
catch any usage of DT when ACPI is enabled.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 569b2da..af9f429 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -775,7 +775,12 @@  void __init start_xen(unsigned long boot_phys_offset,
     smp_init_cpus();
     cpus = smp_get_max_cpus();
 
+/* Comment for now take it after GIC initialization */
+#if defined(CONFIG_ACPI) && defined(CONFIG_ARM_64)
+   init_xen_acpi_time();
+#else
     init_xen_time();
+#endif
 
     gic_init();
 
@@ -789,6 +794,7 @@  void __init start_xen(unsigned long boot_phys_offset,
     xsm_dt_init();
 
     init_maintenance_interrupt();
+
     init_timer_interrupt();
 
     timer_init();
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 0add494..0d4c26d 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -29,6 +29,7 @@ 
 #include <xen/time.h>
 #include <xen/sched.h>
 #include <xen/event.h>
+#include <xen/acpi.h>
 #include <asm/system.h>
 #include <asm/time.h>
 #include <asm/gic.h>
@@ -239,6 +240,71 @@  struct tm wallclock_time(uint64_t *ns)
     return (struct tm) { 0 };
 }
 
+#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI)
+
+#define ACPI_GTDT_INTR_MASK 0x3
+
+static int GTDT_INTRL_TAB[] ={ DT_IRQ_TYPE_LEVEL_HIGH, DT_IRQ_TYPE_EDGE_RISING,
+                          DT_IRQ_TYPE_LEVEL_LOW ,DT_IRQ_TYPE_EDGE_FALLING};
+
+/* Initialize per-processor generic timer */
+static int __init arch_timer_acpi_init(struct acpi_table_header *table)
+{
+    int res;
+    int type;
+    struct acpi_table_gtdt *gtdt;
+
+    gtdt = (struct acpi_table_gtdt *)table;
+
+    /* Initialize all the generic timer IRQ variable from GTDT table */
+
+    type = GTDT_INTRL_TAB[gtdt->non_secure_el1_flags & ACPI_GTDT_INTR_MASK];
+    acpi_set_irq(gtdt->non_secure_el1_interrupt, type);
+    timer_irq[TIMER_PHYS_NONSECURE_PPI] = gtdt->non_secure_el1_interrupt;
+
+    type = GTDT_INTRL_TAB[gtdt->secure_el1_flags & ACPI_GTDT_INTR_MASK];
+    acpi_set_irq(gtdt->secure_el1_interrupt, type);
+    timer_irq[TIMER_PHYS_SECURE_PPI] = gtdt->secure_el1_interrupt;
+
+    type = GTDT_INTRL_TAB[gtdt->non_secure_el2_flags & ACPI_GTDT_INTR_MASK];
+    acpi_set_irq(gtdt->non_secure_el2_interrupt, type);
+    timer_irq[TIMER_HYP_PPI] = gtdt->non_secure_el2_interrupt;
+
+    type = GTDT_INTRL_TAB[gtdt->virtual_timer_flags & ACPI_GTDT_INTR_MASK];
+    acpi_set_irq(gtdt->virtual_timer_interrupt, type);
+    timer_irq[TIMER_VIRT_PPI] = gtdt->virtual_timer_interrupt;
+
+    printk("Generic Timer IRQ from ACPI GTDT: phys=%u hyp=%u virt=%u\n",
+           timer_irq[TIMER_PHYS_NONSECURE_PPI],
+           timer_irq[TIMER_HYP_PPI],
+           timer_irq[TIMER_VIRT_PPI]);
+
+    res = platform_init_time();
+    if ( res )
+        printk("Timer: Cannot initialize platform timer");
+
+    /* Check that this CPU supports the Generic Timer interface */
+    if ( !cpu_has_gentimer )
+        printk("CPU does not support the Generic Timer v1 interface");
+
+    cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
+
+    boot_count = READ_SYSREG64(CNTPCT_EL0);
+
+    printk("Using generic timer at %lu KHz\n", cpu_khz);
+
+    return 0;
+}
+
+int __init init_xen_acpi_time(void)
+{
+   /* Initialize all the generic timers presented in GTDT */
+   acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
+   return 0;
+}
+
+#endif
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/time.h b/xen/include/xen/time.h
index 709501f..4598a0c 100644
--- a/xen/include/xen/time.h
+++ b/xen/include/xen/time.h
@@ -11,6 +11,7 @@ 
 #include <xen/types.h>
 #include <public/xen.h>
 
+extern int init_xen_acpi_time(void);
 extern int init_xen_time(void);
 extern void cstate_restore_tsc(void);