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

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

[wangwudi]

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.
We might consider adding a VINVALL cmd after VMOVP to increase robustness.

@@ -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?

Thanks,
Wudi

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

[Marc Zyngier]

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?

> 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.

We already issue a VINVALL when a VPE is mapped. Why would you need
anything else?

> 
> @@ -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.

Thanks,

	M.

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

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

[wangwudi]

在 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.

> 
>> 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.

>>
>> @@ -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 :-).

Thanks for your explaination
Wudi

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

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

[Marc Zyngier]

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.

Thanks,

	M.

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

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

[wangwudi]

在 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.