Re: [PATCH 1/5] xen/arm: Physical IRQ is not always equal to virtual IRQ

[Julien Grall <julien.grall@linaro.org>]

On 06/25/2013 02:16 PM, Stefano Stabellini wrote:

> On Tue, 25 Jun 2013, Julien Grall wrote:
>> From: Julien Grall <julien.grall@linaro.org>
>>
>> When Xen needs to EOI a physical IRQ, we must use the IRQ number
>> in irq_desc instead of the virtual IRQ.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  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 177560e..0fee3f2 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -810,7 +810,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);
>> @@ -846,6 +846,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);
>> @@ -854,10 +855,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);
>>          }
> 
> I think that virq and pirq are guaranteed to always be the same, at
> least at the moment. Look at vgic_vcpu_inject_irq: it takes just one irq
> parameter, that is both the physical and the virtual irq number.

> Unless we change the vgic_vcpu_inject_irq interface to allow virq !=
> pirq, I don't think this patch makes much sense.


Right. I wrote this patch because it easier to forget to modify some
part when non-1:1 IRQ mappings will be created :).

-- 
Julien