On Fri, Feb 13, 2026 at 7:20 AM Sean Christopherson <[email protected]> wrote:
>
> Please trim your replies. Scrolling through 100+ lines of quoted text to find
> the ~12 lines of context that actually matter is annoying.
>
> On Fri, Feb 13, 2026, Yosry Ahmed wrote:
> > On Thu, Feb 12, 2026 at 07:58:52AM -0800, Jim Mattson wrote:
> > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > > index a49c48459e0b..88549705133f 100644
> > > --- a/arch/x86/kvm/svm/svm.h
> > > +++ b/arch/x86/kvm/svm/svm.h
> > > @@ -607,6 +607,22 @@ static inline bool nested_npt_enabled(struct
> > > vcpu_svm *svm)
> > > return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP;
> > > }
> > >
> > > +static inline void svm_set_gpat(struct vcpu_svm *svm, u64 data)
> > > +{
> > > + svm->nested.save.g_pat = data;
> > > + vmcb_set_gpat(svm->nested.vmcb02.ptr, data);
> > > +}
> > > +
> > > +static inline void svm_set_hpat(struct vcpu_svm *svm, u64 data)
> > > +{
> > > + svm->vcpu.arch.pat = data;
> > > + if (npt_enabled) {
>
> Peeking at the future patches, if we make this:
>
> if (!npt_enabled)
> return;
>
> then we can end up with this:
>
> if (npt_enabled)
> return;
>
> vmcb_set_gpat(svm->vmcb01.ptr, data);
> if (is_guest_mode(&svm->vcpu) && !nested_npt_enabled(svm))
> vmcb_set_gpat(svm->nested.vmcb02.ptr, data);
>
> if (svm->nested.legacy_gpat_semantics)
> svm_set_l2_pat(svm, data);
>
> Because legacy_gpat_semantics can only be true if npt_enabled is true.
> Without
> that guard, KVM _looks_ buggy because it's setting gpat in the VMCB even when
> it shouldn't exist.
>
> Actually, calling svm_set_l2_pat() when !is_guest_mode() is wrong too, no?
> E.g.
> shouldn't we end up with this?
Sigh. legacy_gpat_semantics is supposed to be set only when
is_guest_mode() and nested_npt_enabled(). I forgot about back-to-back
invocations of KVM_SET_NESTED_STATE. Are there other ways of leaving
guest mode or disabling nested NPT before the next KVM_RUN?
> static inline void svm_set_l1_pat(struct vcpu_svm *svm, u64 data)
> {
> svm->vcpu.arch.pat = data;
>
> if (npt_enabled)
> return;
>
> vmcb_set_gpat(svm->vmcb01.ptr, data);
>
> if (is_guest_mode(&svm->vcpu)) {
> if (svm->nested.legacy_gpat_semantics)
> svm_set_l2_pat(svm, data);
> else if (!nested_npt_enabled(svm))
> vmcb_set_gpat(svm->nested.vmcb02.ptr, data);
> }
> }
>
>
> > > + vmcb_set_gpat(svm->vmcb01.ptr, data);
> > > + if (is_guest_mode(&svm->vcpu) && !nested_npt_enabled(svm))
> > > + vmcb_set_gpat(svm->nested.vmcb02.ptr, data);
> > > + }
> > > +}
> >
> > Is it me, or is it a bit confusing that svm_set_gpat() sets L2's gPAT
> > not L1's, and svm_set_hpat() calls vmcb_set_gpat()?
>
> It's not just you. I don't find it confusing per se, more that it's really
> subtle.
>
> > "gpat" means different things in the context of the VMCB or otherwise,
> > which kinda makes sense but is also not super clear. Maybe
> > svm_set_l1_gpat() and svm_set_l2_gpat() is more clear?
>
> I think just svm_set_l1_pat() and svm_set_l2_pat(), because gpat straight up
> doesn't exist when NPT is disabled/unsupported.
My intention was that "gpat" and "hpat" were from the perspective of the vCPU.
I dislike svm_set_l1_pat() and svm_set_l2_pat(). As you point out
above, there is no independent L2 PAT when nested NPT is disabled. I
think that's less obvious than the fact that there is no gPAT from the
vCPU's perspective. My preference is to follow the APM terminology
when possible. Making up our own terms just leads to confusion.
