Re: [Xen-devel] [PATCH 1/6] xen/arm: Physical IRQ is not always equal to virtual IRQ

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-12-06 at 17:26 +0000, Stefano Stabellini wrote:
> From: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> From: Julien Grall <julien.grall@xxxxxxxxxx>

Oops ;-)
> 
> When Xen needs to EOI a physical IRQ, we should use the IRQ number
> in irq_desc instead of the virtual IRQ.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> ---
>  xen/arch/arm/gic.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 43c11cb..7e87acb 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -880,7 +880,7 @@ static void gic_irq_eoi(void *info)
>  
>  static void maintenance_interrupt(int irq, void *dev_id, struct 
> cpu_user_regs *regs)
>  {
> -    int i = 0, virq;
> +    int i = 0, virq, pirq;
>      uint32_t lr;
>      struct vcpu *v = current;
>      uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
> @@ -916,6 +916,7 @@ static void maintenance_interrupt(int irq, void *dev_id, 
> struct cpu_user_regs *r
>              /* Assume only one pcpu needs to EOI the irq */
>              cpu = p->desc->arch.eoi_cpu;
>              eoi = 1;
> +            pirq = p->desc->irq;
>          }
>          list_del_init(&p->inflight);
>          spin_unlock_irq(&v->arch.vgic.lock);
> @@ -924,10 +925,10 @@ static void maintenance_interrupt(int irq, void 
> *dev_id, struct cpu_user_regs *r
>              /* this is not racy because we can't receive another irq of the
>               * same type until we EOI it.  */
>              if ( cpu == smp_processor_id() )
> -                gic_irq_eoi((void*)(uintptr_t)virq);
> +                gic_irq_eoi((void*)(uintptr_t)pirq);
>              else
>                  on_selected_cpus(cpumask_of(cpu),
> -                                 gic_irq_eoi, (void*)(uintptr_t)virq, 0);
> +                                 gic_irq_eoi, (void*)(uintptr_t)pirq, 0);

This relies on pirq being set whenever eoi is, which is currently true,
but it seems a bit fragile. eoi is interchangeable with p->desc != NULL,
isn't it? And since everything needed to do the eoi is in there is there
any reason not to switch to that conditional?




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