Re: [Xen-devel] [PATCH v3 08/24] xen/arm: Allow virq != irq

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 20/02/15 15:52, Ian Campbell wrote:
> On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote:
>> Actually Xen is assuming that the virtual IRQ will always be the same as IRQ.
> 
> s/Actually/Currently/?

Yes, I always mix both because the former is close to the french word.

>> Modify route_guest_irq to take the virtual IRQ in parameter and let Xen
>> assign a different IRQ number.
> 
> I think I must be misunderstanding this, but if route_guest_irq is
> taking the vIRQ as a parameter then why does it then need to assign a
> different IRQ number?
> 
> Oh, did you mean allowing the *caller* to setup a non-1:1 mapping
> perhaps? I think s/and let Xen assign/which allows Xen to.../ might be
> closer to that meaning if that's the intention?

Yes, the IRQ is not mapped 1:1 to the guest. I will replace by "which
allows Xen to...".

>>  Also store the vIRQ in the desc action to
>> easily retrieve easily the IRQ target when we need to inject the interrupt.
> 
> -ETooManyEasilys ;-)
> 
> (I think you want to drop the second one)

Will fix it.

> 
>> As DOM0 will get most the devices, the vIRQ is equal to the IRQ in that case.
> 
> Am I correct that after this patch all callers still pass irq==virq to
> the new function?
> 
>> At the same time modify the behavior of irq_get_domain. The function now
>> assumes that the irq_desc belongs to an IRQ assigned to a guest.
> 
> s/assumes/requires/?

Will fix it.

>>  
>>      action = xmalloc(struct irqaction);
>> -    if (!action)
>> +    if ( !action )
>> +        return -ENOMEM;
>> +
>> +    info = xmalloc(struct irq_guest);
> 
> FWIW you might (subject to sizing/alignment needs) be able to do
>       action = _xmalloc(sizeof(struct irqaction) + sizeof(struct irq_guest);
>       info = (sturct irq_guest *)(action + 1);
> 
> which would save some memory overhead for free pointers etc and allow
> you to avoid manually managing the info.
> 
> You probably won't like that though, so feel free to ignore.

Actually it's a good idea :). I haven't though about it.

Regards,

-- 
Julien Grall

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