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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 4 Feb 2015, parth.dixit@xxxxxxxxxx wrote:
> From: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> ---
>  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

I wouldn't introduce a new function for this.

You can call the acpi specific initialization with appropriate #defines
from the implementation of init_xen_time.


>      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();

Spurious change


> 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

This should be in actbl3.h with the other GTDT #defines.


> +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};


I don't think is a good idea to specify GTDT interrupt states in terms
of DT_IRQ_TYPEs. Unless this is some sort of conversion table?


> +/* 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);
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel