On Mon, Jan 25, 2016 at 03:53:36PM +0000, Marc Zyngier wrote:
> With the ARMv8.1 VHE, the kernel can run in HYP mode, and thus
> use the HYP timer instead of the normal guest timer in a mostly
> transparent way, except for the interrupt line.
> 
> This patch reworks the arch timer code to allow the selection of
> the HYP PPI, possibly falling back to the guest timer if not
> available.
> 
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
>  drivers/clocksource/arm_arch_timer.c | 96 
> ++++++++++++++++++++++--------------
>  1 file changed, 59 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c 
> b/drivers/clocksource/arm_arch_timer.c
> index c64d543..ffe9d1c 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -67,7 +67,7 @@ static int arch_timer_ppi[MAX_TIMER_PPI];
>  
>  static struct clock_event_device __percpu *arch_timer_evt;
>  
> -static bool arch_timer_use_virtual = true;
> +static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
>  static bool arch_timer_c3stop;
>  static bool arch_timer_mem_use_virtual;
>  
> @@ -263,14 +263,20 @@ static void __arch_timer_setup(unsigned type,
>               clk->name = "arch_sys_timer";
>               clk->rating = 450;
>               clk->cpumask = cpumask_of(smp_processor_id());
> -             if (arch_timer_use_virtual) {
> -                     clk->irq = arch_timer_ppi[VIRT_PPI];
> +             clk->irq = arch_timer_ppi[arch_timer_uses_ppi];
> +             switch (arch_timer_uses_ppi) {
> +             case VIRT_PPI:
>                       clk->set_state_shutdown = arch_timer_shutdown_virt;
>                       clk->set_next_event = arch_timer_set_next_event_virt;
> -             } else {
> -                     clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
> +                     break;
> +             case PHYS_SECURE_PPI:
> +             case PHYS_NONSECURE_PPI:
> +             case HYP_PPI:
>                       clk->set_state_shutdown = arch_timer_shutdown_phys;
>                       clk->set_next_event = arch_timer_set_next_event_phys;
> +                     break;
> +             default:
> +                     BUG();
>               }
>       } else {
>               clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
> @@ -338,17 +344,20 @@ static void arch_counter_set_user_access(void)
>       arch_timer_set_cntkctl(cntkctl);
>  }
>  
> +static bool arch_timer_has_nonsecure_ppi(void)
> +{
> +     return (arch_timer_uses_ppi == PHYS_SECURE_PPI &&
> +             arch_timer_ppi[PHYS_NONSECURE_PPI]);
> +}
> +
>  static int arch_timer_setup(struct clock_event_device *clk)
>  {
>       __arch_timer_setup(ARCH_CP15_TIMER, clk);
>  
> -     if (arch_timer_use_virtual)
> -             enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
> -     else {
> -             enable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI], 0);
> -             if (arch_timer_ppi[PHYS_NONSECURE_PPI])
> -                     enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 
> 0);
> -     }
> +     enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], 0);
> +
> +     if (arch_timer_has_nonsecure_ppi())
> +             enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
>  
>       arch_counter_set_user_access();
>       if (IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM))
> @@ -390,7 +399,7 @@ static void arch_timer_banner(unsigned type)
>                    (unsigned long)arch_timer_rate / 1000000,
>                    (unsigned long)(arch_timer_rate / 10000) % 100,
>                    type & ARCH_CP15_TIMER ?
> -                     arch_timer_use_virtual ? "virt" : "phys" :
> +                  (arch_timer_uses_ppi == VIRT_PPI) ? "virt" : "phys" :
>                       "",
>                    type == (ARCH_CP15_TIMER | ARCH_MEM_TIMER) ?  "/" : "",
>                    type & ARCH_MEM_TIMER ?
> @@ -460,7 +469,7 @@ static void __init arch_counter_register(unsigned type)
>  
>       /* Register the CP15 based counter if we have one */
>       if (type & ARCH_CP15_TIMER) {
> -             if (IS_ENABLED(CONFIG_ARM64) || arch_timer_use_virtual)
> +             if (IS_ENABLED(CONFIG_ARM64) || arch_timer_uses_ppi == VIRT_PPI)
>                       arch_timer_read_counter = arch_counter_get_cntvct;
>               else
>                       arch_timer_read_counter = arch_counter_get_cntpct;
> @@ -490,13 +499,9 @@ static void arch_timer_stop(struct clock_event_device 
> *clk)
>       pr_debug("arch_timer_teardown disable IRQ%d cpu #%d\n",
>                clk->irq, smp_processor_id());
>  
> -     if (arch_timer_use_virtual)
> -             disable_percpu_irq(arch_timer_ppi[VIRT_PPI]);
> -     else {
> -             disable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI]);
> -             if (arch_timer_ppi[PHYS_NONSECURE_PPI])
> -                     disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
> -     }
> +     disable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi]);
> +     if (arch_timer_has_nonsecure_ppi())
> +             disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
>  
>       clk->set_state_shutdown(clk);
>  }
> @@ -562,12 +567,14 @@ static int __init arch_timer_register(void)
>               goto out;
>       }
>  
> -     if (arch_timer_use_virtual) {
> -             ppi = arch_timer_ppi[VIRT_PPI];
> +     ppi = arch_timer_ppi[arch_timer_uses_ppi];
> +     switch (arch_timer_uses_ppi) {
> +     case VIRT_PPI:
>               err = request_percpu_irq(ppi, arch_timer_handler_virt,
>                                        "arch_timer", arch_timer_evt);
> -     } else {
> -             ppi = arch_timer_ppi[PHYS_SECURE_PPI];
> +             break;
> +     case PHYS_SECURE_PPI:
> +     case PHYS_NONSECURE_PPI:
>               err = request_percpu_irq(ppi, arch_timer_handler_phys,
>                                        "arch_timer", arch_timer_evt);
>               if (!err && arch_timer_ppi[PHYS_NONSECURE_PPI]) {
> @@ -578,6 +585,13 @@ static int __init arch_timer_register(void)
>                               free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
>                                               arch_timer_evt);
>               }
> +             break;
> +     case HYP_PPI:
> +             err = request_percpu_irq(ppi, arch_timer_handler_phys,
> +                                      "arch_timer", arch_timer_evt);
> +             break;
> +     default:
> +             BUG();
>       }
>  
>       if (err) {
> @@ -602,15 +616,10 @@ static int __init arch_timer_register(void)
>  out_unreg_notify:
>       unregister_cpu_notifier(&arch_timer_cpu_nb);
>  out_free_irq:
> -     if (arch_timer_use_virtual)
> -             free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
> -     else {
> -             free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
> +     free_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], arch_timer_evt);
> +     if (arch_timer_has_nonsecure_ppi())
> +             free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
>                               arch_timer_evt);
> -             if (arch_timer_ppi[PHYS_NONSECURE_PPI])
> -                     free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
> -                                     arch_timer_evt);
> -     }
>  
>  out_free:
>       free_percpu(arch_timer_evt);
> @@ -697,12 +706,25 @@ static void __init arch_timer_init(void)
>        *
>        * If no interrupt provided for virtual timer, we'll have to
>        * stick to the physical timer. It'd better be accessible...
> +      *
> +      * On ARMv8.1 with VH extensions, the kernel runs in HYP. VHE
> +      * accesses to CNTP_*_EL1 registers are silently redirected to
> +      * their CNTHP_*_EL2 counterparts, and use a different PPI
> +      * number.
>        */
>       if (is_hyp_mode_available() || !arch_timer_ppi[VIRT_PPI]) {
> -             arch_timer_use_virtual = false;
> +             bool has_ppi;
> +
> +             if (is_kernel_in_hyp_mode()) {
> +                     arch_timer_uses_ppi = HYP_PPI;
> +                     has_ppi = !!arch_timer_ppi[HYP_PPI];
> +             } else {
> +                     arch_timer_uses_ppi = PHYS_SECURE_PPI;
> +                     has_ppi = (!!arch_timer_ppi[PHYS_SECURE_PPI] ||
> +                                !!arch_timer_ppi[PHYS_NONSECURE_PPI]);

shouldn't this simply test the PHYS_SECURE_PPI since otherwise you could
potentially have the PHYS_NONSECURE_PPI but not PHYS_SECURE_PPI and
you'll try to request IRQ 0 for this later... ?

> +             }
>  
> -             if (!arch_timer_ppi[PHYS_SECURE_PPI] ||
> -                 !arch_timer_ppi[PHYS_NONSECURE_PPI]) {
> +             if (!has_ppi) {
>                       pr_warn("arch_timer: No interrupt available, giving 
> up\n");
>                       return;
>               }
> @@ -735,7 +757,7 @@ static void __init arch_timer_of_init(struct device_node 
> *np)
>        */
>       if (IS_ENABLED(CONFIG_ARM) &&
>           of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> -                     arch_timer_use_virtual = false;
> +             arch_timer_uses_ppi = PHYS_SECURE_PPI;
>  
>       arch_timer_init();
>  }
> -- 
> 2.1.4
> 

Thanks,
-Christoffer
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to