Re: [Xen-devel] [RFC PATCH 31/49] ARM: new VGIC: Add PENDING registers handlers

[Andre Przywara <andre.przywara@xxxxxxxxxx>]

Hi,

On 19/02/18 15:43, Julien Grall wrote:
> 
> 
> On 19/02/18 15:32, Andre Przywara wrote:
>> Hi,
> 
> Hi Andre,
> 
>> On 16/02/18 17:16, Julien Grall wrote:
>>> On 09/02/18 14:39, Andre Przywara wrote:
>>>> +
>>>> +    /* Loop over all IRQs affected by this read */
>>>> +    for ( i = 0; i < len * 8; i++ )
>>>> +    {
>>>> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain, vcpu, intid
>>>> + i);
>>>> +
>>>> +        if ( irq_is_pending(irq) )
>>>> +            value |= (1U << i);
>>>
>>> Don't you need to propagate the value to the hardware for virtual
>>> interrupt mapped to physical interrupt?
>>
>> Do you mean in the write functions below? (This is the read function, I
>> don't see how this would apply here.)
> 
> Hmmm yes. Sorry I misplaced the comment.
> 
>>
>> In case you meant the write_[cs]pending() functions:
>> I don't think this makes too much sense. Why would you want to trigger
>> an hardware IRQ? All you want it is to deliver it to the guest, which is
>> what those functions below do. So what do I miss here?
> 
> Imagine you clear the pending bit on an hardware mapped interrupt. If
> you never clear the active bit on the physical one, you will never
> receive that interrupt again.
> 
> For setting pending bit, I am not entirely sure. But it looks like KVM
> is doing it (see latest master). So I am wondering why Xen is diverging
> here.

The simple reason is that "latest master" was something different when I
imported the VGIC, obviously. So this was a later addition. I now ported
this new patch over, though due to the locking order of the desc lock
this isn't so pretty (but still not too bad). But actually this function
should be extremely rare, up to the point that I currently cannot test
this easily.

Cheers,
Andre.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel