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

Zenghui Yu
2019-12-19T03:05:33Z

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@kernel.org>
>>
>> Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
>>
>>> ---
>>>   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