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

From: Zenghui Yu
Date: Wed Dec 18 2019 - 22:05:27 EST


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