All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	 Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Shuah Khan <shuah@kernel.org>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	 Yosry Ahmed <yosry.ahmed@linux.dev>
Subject: Re: [PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
Date: Fri, 6 Feb 2026 10:23:03 -0800	[thread overview]
Message-ID: <aYYxh8EiLrBTiq0L@google.com> (raw)
In-Reply-To: <20260205214326.1029278-4-jmattson@google.com>

On Thu, Feb 05, 2026, Jim Mattson wrote:
> When nested NPT is enabled in vmcb12, copy the (cached and validated)
> vmcb12 g_pat field to the guest PAT register. Under KVM, the guest PAT
> register lives in the vmcb02 g_pat field.
> 
> When NPT is enabled, but nested NPT is disabled, copy L1's IA32_PAT MSR to
> the vmcb02 g_pat field, since L2 shares the IA32_PAT MSR with L1.
> 
> When NPT is disabled, the vmcb02 g_pat field is ignored by hardware.

Uber nit, the "vmcb02" qualifier can be dropped, i.e.

  When NPT is disabled, the g_pat field is ignored by hardware.

Scoping it to vmcb02 makes it sound like there's a special rule about vmcb02.

> Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 1d4ff6408b34..1ff2ede96094 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -646,9 +646,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
>  	struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
>  	struct kvm_vcpu *vcpu = &svm->vcpu;
>  
> -	nested_vmcb02_compute_g_pat(svm);
> -	vmcb_mark_dirty(vmcb02, VMCB_NPT);
> -
>  	/* Load the nested guest state */
>  	if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
>  		new_vmcb12 = true;
> @@ -656,6 +653,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
>  		svm->nested.force_msr_bitmap_recalc = true;
>  	}
>  
> +	if (npt_enabled) {
> +		if (nested_npt_enabled(svm)) {
> +			if (unlikely(new_vmcb12 ||
> +				     vmcb_is_dirty(vmcb12, VMCB_NPT))) {
> +				vmcb02->save.g_pat = svm->nested.gpat;
> +				vmcb_mark_dirty(vmcb02, VMCB_NPT);
> +			}
> +		} else {
> +			vmcb02->save.g_pat = vcpu->arch.pat;
> +			vmcb_mark_dirty(vmcb02, VMCB_NPT);
> +		}
> +	}

To reduce indentation, how about this?  There's a consistency check for
nested_npt_enabled() vs. npt_enabled, so it's guaranteed to do the right thing.

	if (nested_npt_enabled(svm)) {
		if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_NPT))) {
			vmcb02->save.g_pat = svm->nested.gpat;
			vmcb_mark_dirty(vmcb02, VMCB_NPT);
		}
	} else if (npt_enabled) {
		vmcb02->save.g_pat = vcpu->arch.pat;
		vmcb_mark_dirty(vmcb02, VMCB_NPT);
	}

> +
>  	if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_SEG))) {
>  		vmcb02->save.es = vmcb12->save.es;
>  		vmcb02->save.cs = vmcb12->save.cs;
> -- 
> 2.53.0.rc2.204.g2597b5adb4-goog
> 

  reply	other threads:[~2026-02-06 18:23 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05 21:43 [PATCH v3 0/8] KVM: x86: nSVM: Improve PAT virtualization Jim Mattson
2026-02-05 21:43 ` [PATCH v3 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating g_pat in L2 Jim Mattson
2026-02-09 16:05   ` Yosry Ahmed
2026-02-05 21:43 ` [PATCH v3 2/8] KVM: x86: nSVM: Cache and validate vmcb12 g_pat Jim Mattson
2026-02-06 18:19   ` Sean Christopherson
2026-02-06 18:23     ` Yosry Ahmed
2026-02-06 18:32       ` Jim Mattson
2026-02-06 19:12         ` Sean Christopherson
2026-02-06 19:15           ` Yosry Ahmed
2026-02-06 19:50             ` Sean Christopherson
2026-02-06 20:56           ` Jim Mattson
2026-02-06 23:07             ` Sean Christopherson
2026-02-05 21:43 ` [PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT Jim Mattson
2026-02-06 18:23   ` Sean Christopherson [this message]
2026-02-06 18:29     ` Yosry Ahmed
2026-02-06 19:14       ` Sean Christopherson
2026-02-05 21:43 ` [PATCH v3 4/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT Jim Mattson
2026-02-05 21:43 ` [PATCH v3 5/8] KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT Jim Mattson
2026-02-05 21:43 ` [PATCH v3 6/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE Jim Mattson
2026-02-05 21:43 ` [PATCH v3 7/8] KVM: x86: nSVM: Handle restore of legacy nested state Jim Mattson
2026-02-06 19:17   ` Sean Christopherson
2026-02-06 22:38     ` Jim Mattson
2026-02-05 21:43 ` [PATCH v3 8/8] KVM: selftests: nSVM: Add svm_nested_pat test Jim Mattson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aYYxh8EiLrBTiq0L@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    --cc=yosry.ahmed@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.