On Fri, Feb 13, 2026 at 2:19 PM Sean Christopherson <[email protected]> wrote: > > On Fri, Feb 13, 2026, Jim Mattson wrote: > > On Fri, Feb 13, 2026 at 7:20 AM Sean Christopherson <[email protected]> > > wrote: > > > > > +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? > > KVM_SET_VCPU_EVENTS will do it if userspace forces a change in SMM state: > > if (!!(vcpu->arch.hflags & HF_SMM_MASK) != events->smi.smm) { > kvm_leave_nested(vcpu); > kvm_smm_changed(vcpu, events->smi.smm); > } > > I honestly wasn't even thinking of anything in particular, it just looked > weird.
At the very least, then, kvm_leave_nested() has to clear legacy_gpat_semantics. I will look for other paths. > > > > > + 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. > > How about svm_set_pat() and svm_get_gpat()? Because hPAT doesn't exist when > NPT > is unsupported/disabled, but KVM still needs to set the vCPU's emulated PAT > value. What if we don't break it up this way at all? Instead of distributing the logic between svm_[gs]set_msr() and a few helper functions, we could just have svm_[gs]et_msr() call svm_[gs]et_pat(), and all of the logic can go in these two functions.
