Re: [PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set

[Zenghui Yu]

Hi Marc,

On 2019/12/18 22:39, Marc Zyngier wrote:
> On 2019-11-01 11:05, Zenghui Yu wrote:
> > Hi Marc,
> >
> > On 2019/10/27 22:42, Marc Zyngier wrote:
> > > The infamous VPE proxy device isn't used with GICv4.1 because:
> > > - we can invalidate any LPI from the DirectLPI MMIO interface
> > > - the ITS and redistributors understand the life cycle of
> > > ÂÂ the doorbell, so we don't need to enable/disable it all
> > > ÂÂ the time
> > > So let's escape early from the proxy related functions.
> > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> >
> > Reviewed-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>
> >
> > > ---
> > > Â drivers/irqchip/irq-gic-v3-its.c | 23 ++++++++++++++++++++++-
> > > Â 1 file changed, 22 insertions(+), 1 deletion(-)
> > > diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> > > b/drivers/irqchip/irq-gic-v3-its.c
> > > index 220d490d516e..999e61a9b2c3 100644
> > > --- a/drivers/irqchip/irq-gic-v3-its.c
> > > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > > @@ -3069,7 +3069,7 @@ static const struct irq_domain_ops 
> > > its_domain_ops = {
> > > Â /*
> > > ÂÂ * This is insane.
> > > ÂÂ *
> > > - * If a GICv4 doesn't implement Direct LPIs (which is extremely
> > > + * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
> > > ÂÂ * likely), the only way to perform an invalidate is to use a fake
> > > ÂÂ * device to issue an INV command, implying that the LPI has first
> > > ÂÂ * been mapped to some event on that device. Since this is not exactly
> > > @@ -3077,9 +3077,18 @@ static const struct irq_domain_ops 
> > > its_domain_ops = {
> > > ÂÂ * only issue an UNMAP if we're short on available slots.
> > > ÂÂ *
> > > ÂÂ * Broken by design(tm).
> > > + *
> > > + * GICv4.1 actually mandates that we're able to invalidate by 
> > > writing to a
> > > + * MMIO register. It doesn't implement the whole of DirectLPI, but 
> > > that's
> > > + * good enough. And most of the time, we don't even have to invalidate
> > > + * anything, so that's actually pretty good!
> >
> > I can't understand the meaning of this last sentence. May I ask for an
> > explanation? :)
>
> Yeah, reading this now, it feels pretty clumsy, and only remotely
> connected to the patch.
>
> What I'm trying to say here is that, although GICv4.1 doesn't have
> the full spectrum of v4.0 DirectLPI (it only allows a subset of it),
> this subset is more then enough for us. Here's the rational:
>
> When a vPE exits from the hypervisor, we know whether we need to
> request a doorbell or not (depending on whether we're blocking on
> WFI or not). On GICv4.0, this translates into enabling the doorbell
> interrupt, which generates an invalidation (costly). And whenever
> we've taken a doorbell, or are scheduled again, we need to turn
> the doorbell off (invalidation again).
>
> With v4.1, we can just say *at exit time* whether we want doorbells
> to be subsequently generated (see its_vpe_4_1_deschedule() and the
> req_db parameter in the info structure). This is part of making
> the vPE non-resident, so we have 0 overhead at this stage.

Great, and get it. Thanks for this clear explanation!


Zenghui