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

[Marc Zyngier <maz@kernel.org>]

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.