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

[Marc Zyngier]

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.

I'll try and update the comment here.

Thanks,

        M.
--
Jazz is not dead. It just smells funny...