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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 27 Mar 2018, Andre Przywara wrote:
> 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.

I went through the whole lifecycle of SGIs with the new vgic and it is
quite different from before, but it makes sense to me now.

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> 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