Re: [PATCH] xen/arm: Handle platforms with edge-triggered virtual timer

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Nov 28, 2014 at 03:17:06PM +0000, Julien Grall wrote:
> Some platforms (such as Xgene and ARMv8 models) use an edge-triggered interrupt
> for the virtual timer. Even if the timer output signal is masked in the
> context switch, the GIC will keep track that of any interrupts raised
> while IRQs are disabled. As soon as IRQs are re-enabled, the virtual
> interrupt timer will be injected to Xen.
> 
> If an idle vVCPU was scheduled next then the interrupt handler doesn't
> expect to the receive the IRQ and will crash:
> 
> (XEN)    [<0000000000228388>] _spin_lock_irqsave+0x28/0x94 (PC)
> (XEN)    [<0000000000228380>] _spin_lock_irqsave+0x20/0x94 (LR)
> (XEN)    [<0000000000250510>] vgic_vcpu_inject_irq+0x40/0x1b0
> (XEN)    [<000000000024bcd0>] vtimer_interrupt+0x4c/0x54
> (XEN)    [<0000000000247010>] do_IRQ+0x1a4/0x220
> (XEN)    [<0000000000244864>] gic_interrupt+0x50/0xec
> (XEN)    [<000000000024fbac>] do_trap_irq+0x20/0x2c
> (XEN)    [<0000000000255240>] hyp_irq+0x5c/0x60
> (XEN)    [<0000000000241084>] context_switch+0xb8/0xc4
> (XEN)    [<000000000022482c>] schedule+0x684/0x6d0
> (XEN)    [<000000000022785c>] __do_softirq+0xcc/0xe8
> (XEN)    [<00000000002278d4>] do_softirq+0x14/0x1c
> (XEN)    [<0000000000240fac>] idle_loop+0x134/0x154
> (XEN)    [<000000000024c160>] start_secondary+0x14c/0x15c
> (XEN)    [<0000000000000001>] 0000000000000001
> 
> The proper solution is to context switch the virtual interrupt state at
> the GIC level. This would also avoid masking the output signal which
> requires specific handling in the guest OS and more complex code in Xen
> to deal with EOIs, and so is desirable for that reason too.
> 
> Sadly, this solution requires some refactoring which would not be
> suitable for a freeze exception for the Xen 4.5 release.
> 
> For now implement a temporary solution which ignores the virtual timer
> interrupt when the idle VCPU is running.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> ---
> 
> Changes in v2:
>     - Reword the commit message and comment in the code to explain the
>     real bug. Based on Ian's reword.
>     - Use unlikely
> 
> This patch is a bug fix candidate for Xen 4.5 and backport for Xen 4.4.

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> It affects at least Xgene platform and ARMv8 models where Xen may
> randomly crash.
> 
> This patch don't inject the virtual timer interrupt if the current VCPU
> is the idle one. For now, I think this patch is the safest way to resolve
> the problem.
> 
> I will work on a proper solution for Xen 4.6.
> ---
>  xen/arch/arm/time.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index a6436f1..471d7a9 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -169,6 +169,19 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
>  
>  static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
>  {
> +    /*
> +     * Edge-triggered interrupt can be used for the virtual timer. Even
> +     * if the timer output signal is masked in the context switch, the
> +     * GIC will keep track that of any interrupts raised while IRQS as
> +     * disabled. As soon as IRQs are re-enabled, the virtual interrupt
> +     * will be injected to Xen.
> +     *
> +     * If an IDLE vCPU was scheduled next then we should ignore the
> +     * interrupt.
> +     */
> +    if ( unlikely(is_idle_vcpu(current)) )
> +        return;
> +
>      current->arch.virt_timer.ctl = READ_SYSREG32(CNTV_CTL_EL0);
>      WRITE_SYSREG32(current->arch.virt_timer.ctl | CNTx_CTL_MASK, CNTV_CTL_EL0);
>      vgic_vcpu_inject_irq(current, current->arch.virt_timer.irq);
> -- 
> 2.1.3
>