Re: [PATCH v4 4/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT

[Sean Christopherson <seanjc@google.com>]

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?

  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.