Re: [Xen-devel] [PATCH v3 13/39] ARM: new VGIC: Add IRQ sync/flush framework

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 26/03/18 22:30, Stefano Stabellini wrote:
> On Wed, 21 Mar 2018, Andre Przywara wrote:
>> Implement the framework for syncing IRQs between our emulation and the
>> list registers, which represent the guest's view of IRQs.
>> This is done in vgic_sync_from_lrs() and vgic_sync_to_lrs(), which
>> get called on guest entry and exit, respectively.
>> The code talking to the actual GICv2/v3 hardware is added in the
>> following patches.
>>
>> This is based on Linux commit 0919e84c0fc1, written by Marc Zyngier.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> 
> Just one question below, but the code looks nice
> 
> 
>> ---
>> Changelog v2 ... v3:
>> - replace "true" instead of "1" for the boolean parameter
>>
>> Changelog v1 ... v2:
>> - make functions void
>> - do underflow setting directly (no v2/v3 indirection)
>> - fix multiple SGIs injections (as the late Linux bugfix)
>>
>>  xen/arch/arm/vgic/vgic.c | 232 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  xen/arch/arm/vgic/vgic.h |   2 +
>>  2 files changed, 234 insertions(+)
>>
>> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
>> index ee0de8d2e0..52e1669888 100644
>> --- a/xen/arch/arm/vgic/vgic.c
>> +++ b/xen/arch/arm/vgic/vgic.c
>> @@ -409,6 +409,238 @@ void vgic_inject_irq(struct domain *d, struct vcpu 
>> *vcpu, unsigned int intid,

....

>> +/* Requires the ap_list_lock to be held. */
>> +static int compute_ap_list_depth(struct vcpu *vcpu)
>> +{
>> +    struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic;
>> +    struct vgic_irq *irq;
>> +    int count = 0;
>> +
>> +    ASSERT(spin_is_locked(&vgic_cpu->ap_list_lock));
>> +
>> +    list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list)
>> +    {
>> +        spin_lock(&irq->irq_lock);
>> +        /* GICv2 SGIs can count for more than one... */
>> +        if ( vgic_irq_is_sgi(irq->intid) && irq->source )
>> +            count += hweight8(irq->source);
> 
> Why is this done?

GICv2 SGIs always have a source CPU ID connected to them. So if two CPUs
signal another CPU at the same time, there are *two* distinct SGIs, with
two different source IDs. This is an architectural feature of GICv2, so
we have to properly emulate this.
Despite them being edge triggered IRQs, we cannot coalesce them in this
case.

Cheers,
Andre.

>> +        else
>> +            count++;
>> +        spin_unlock(&irq->irq_lock);
>> +    }
>> +    return count;
>> +}
>> +

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