Re: [Question about gic vmovp cmd] Consider adding VINVALL after VMOVP

From: wangwudi
Date: Mon May 29 2023 - 09:35:56 EST




在 2023/5/29 21:24, wangwudi 写道:
>
>
> -----邮件原件-----
> 发件人: Marc Zyngier [mailto:maz@xxxxxxxxxx]
> 发送时间: 2023年5月27日 21:22
> 收件人: wangwudi <wangwudi@xxxxxxxxxxxxx>
> 抄送: linux-kernel@xxxxxxxxxxxxxxx
> 主题: Re: [Question about gic vmovp cmd] Consider adding VINVALL after VMOVP
>
> On Sat, 27 May 2023 10:51:50 +0100,
> wangwudi <wangwudi@xxxxxxxxxxxxx> wrote:
>>
>>
>>
>> 在 2023/5/27 16:56, wangwudi 写道:
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Marc Zyngier [mailto:maz@xxxxxxxxxx]
>>> 发送时间: 2023年5月26日 15:03
>>> 收件人: wangwudi <wangwudi@xxxxxxxxxxxxx>
>>> 抄送: linux-kernel@xxxxxxxxxxxxxxx
>>> 主题: Re: [Question about gic vmovp cmd] Consider adding VINVALL after
>>> VMOVP
>>>
>>> On Fri, 26 May 2023 07:04:34 +0100,
>>> wangwudi <wangwudi@xxxxxxxxxxxxx> wrote:
>>>>
>>>> Hi Marc,
>>>>
>>>> During vpe migration, VMOVP needs to be executed.
>>>> If the vpe is migrated for the first time, especially before it is
>>>> scheduled for the first time, there may be some unusual hanppens
>>>> over kexec.
>>>
>>> What may happen?
>>
>> Actually, I'm not sure.
>
> Then what is that all for?
>
>>
>>>
>>>> We might consider adding a VINVALL cmd after VMOVP to increase
>>>> robustness.
>>>
>>> What are you trying to guarantee by adding this? From a performance
>>> perspective, this is terrible as you're forcing the ITS to drop its
>>> caches and reload everything, making the interrupt latency far worse
>>> than what it should be on each and every vcpu migration.
>>
>> Agree, this reduces performance.
>>
>>>
>>> We already issue a VINVALL when a VPE is mapped. Why would you need
>>> anything else?
>>>
>>
>> It is just for robustness, like the VINALL when a VPE is mapped.
>
> The VINVALL at the point a VPE is mapped serves a purpose: to invalidate the caches from a previous instance of a VPE with the same VPEID. It's not for "robustness" but for *correctness*.
>
>>
>>>>
>>>> @@ -1327,6 +1327,7 @@ static void its_send_vmovp(struct its_vpe
>>>> *vpe)
>>>>
>>>> desc.its_vmovp_cmd.col = &its->collections[col_id];
>>>> its_send_single_vcommand(its, its_build_vmovp_cmd,
>>>> &desc);
>>>> + its_send_vinvall(its, vpe);
>>>> }
>>>>
>>>> Do you think it's all right?
>>>
>>> I think this is pretty bad. If your HW requires this, then we can
>>> add it as a workaround for your particular platform, but in general,
>>> this is not needed.
>>
>> Got it, you are right :-).
>
> May I suggest that in the future, you post patches that actually serve a real purpose and avoid wasting people's time? Your company employs a bunch of good people, some of which are pretty knowledgeable when it comes to the GIC. Please consult with them before posting such thing.
>

Okay, thank you for your patience.

> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.