Re: [GIT PULL] KVM (mostly s390) fixes for Linux 7.2-rc7

[Paolo Bonzini]

Il ven 7 ago 2026, 19:07 Sean Christopherson <seanjc@xxxxxxxxxx> ha scritto:
> On Fri, Aug 07, 2026, Paolo Bonzini wrote:
> > - disable use of INVLPGA when NPT is enabled, because it doesn't seem
> >   to flush TLBs correctly
> >
> > Paolo Bonzini (3):
> >       KVM: SVM: make svm_flush_tlb_gva do a full asid flush if NPT enabled
>
> Why did this get rushed in?

I waited two weeks with the patch on the mailing list. It didn't (and
doesn't) seem like rushing in?

>   There was quite a bit of discussion, both on the
> code itself[1][2] and on the assessment that this appears to be hardware issue[3].
> IMO, neither of those discussions was resolved enough to merge the patch as-is.
>
> I can appreciate wanting to get a fix into stable ASAP, but we could have gone
> with a *much* simpler fix for stable@.  The bulk of this change is a performance
> optimization masquerading as a bug fix, and I'm not convinced it's actually
> optimizing anything meaningful, because SVM only queues the TLB flush, and that
> is super cheap.  E.g. the "we don't know why this is failing, let's smack it with
> a hammer" patch could have been:

It is true that the patch below is smaller and pretty much the same
for all practical purposes. I don't even think it's a matter for
stable vs. mainline, the patch is totally fine in both; maybe with a
comment about the performance being totally fine, since that is
different on Intel vs. AMD, but that's it.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3057ca7051db..58bc4a1cb7a4 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4237,7 +4237,15 @@ static void svm_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t gva)
>  {
>         struct vcpu_svm *svm = to_svm(vcpu);
>
> -       invlpga(gva, svm->vmcb->control.asid);
> +       if (!npt_enabled) {
> +               invlpga(gva, svm->vmcb->control.asid);
> +               return;
> +       }
> +
> +       if (static_cpu_has(X86_FEATURE_FLUSHBYASID))
> +               svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
> +       else
> +               svm->current_vmcb->asid_generation--;
>  }
>
>  static void svm_flush_tlb_guest(struct kvm_vcpu *vcpu)
>
> I don't care _that_ much about the code itself, though I still think the out-param
> is all kinds of ugly. I'm not at all ok with doing so silently
>
> and without what I would consider an honest attempt to address review feedback.

Sorry about that, but again - I waited two weeks and got no reply to
https://lore.kernel.org/all/CABgObfbDE+=ir_phPC+9sVYnNXur6dFx6oAfEGSnCbrOqFoyVA@xxxxxxxxxxxxxx/.
I consider two weeks of wait, on a review that so far had been
exclusively about taste, to *be* an honest attempt to gather the next
round of feedback (which never came).

If you had proposed the above small patch, for example, it would have
been a completely different thing. I would have taken your suggestion
immediately.

Paolo

>
> [1] https://lore.kernel.org/all/amPo-XdhFGIZquES@xxxxxxxxxx
> [2] https://lore.kernel.org/all/CAO9r8zM=Mb=eTEFGF0G-POvwwbSr7e8SN6zcDp=GstzKtBKcow@xxxxxxxxxxxxxx
> [3] https://lore.kernel.org/all/CAO9r8zMopB=YKXs5pkQmPf9uHKE0P0XORcNbc60AWXTusuY3tg@xxxxxxxxxxxxxx
>