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

[wangwudi <wangwudi@hisilicon.com>]

在 2023/5/29 21:24, wangwudi 写道:
> 
> 
> -----邮件原件-----
> 发件人: Marc Zyngier [mailto:maz@kernel.org] 
> 发送时间: 2023年5月27日 21:22
> 收件人: wangwudi <wangwudi@hisilicon.com>
> 抄送: linux-kernel@vger.kernel.org
> 主题: Re: [Question about gic vmovp cmd] Consider adding VINVALL after VMOVP
> 
> On Sat, 27 May 2023 10:51:50 +0100,
> wangwudi <wangwudi@hisilicon.com> wrote:
>>
>>
>>
>> 在 2023/5/27 16:56, wangwudi 写道:
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Marc Zyngier [mailto:maz@kernel.org]
>>> 发送时间: 2023年5月26日 15:03
>>> 收件人: wangwudi <wangwudi@hisilicon.com>
>>> 抄送: linux-kernel@vger.kernel.org
>>> 主题: Re: [Question about gic vmovp cmd] Consider adding VINVALL after 
>>> VMOVP
>>>
>>> On Fri, 26 May 2023 07:04:34 +0100,
>>> wangwudi <wangwudi@hisilicon.com> 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.