[PATCH 0/2] KVM: SVM: Aggressively clear vmcb02 clean bits

[PATCH 0/2] KVM: SVM: Aggressively clear vmcb02 clean bits

[Jim Mattson]

It is unlikely that L1 will toggle the MSR intercept bit in vmcb02,
or that L1 will change its own IA32_PAT MSR. However, if it does,
the affected fields in vmcb02 should not be marked clean.

An alternative approach would be to implement a set of mutators for
vmcb02 fields, and to clear the associated clean bit whenever a field
is modified.

Jim Mattson (2):
  KVM: SVM: Mark VMCB_PERM_MAP as dirty on nested VMRUN
  KVM: SVM: Mark VMCB_NPT as dirty on nested VMRUN

 arch/x86/kvm/svm/nested.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.51.0.470.ga7dc726c21-goog

[PATCH 1/2] KVM: SVM: Mark VMCB_PERM_MAP as dirty on nested VMRUN

[Jim Mattson]

Mark the VMCB_PERM_MAP bit as dirty in nested_vmcb02_prepare_control()
on every nested VMRUN.

If L1 changes MSR interception (INTERCEPT_MSR_PROT) between two VMRUN
instructions on the same L1 vCPU, the msrpm_base_pa in the associated
vmcb02 will change, and the VMCB_PERM_MAP clean bit should be cleared.

Fixes: 4bb170a5430b ("KVM: nSVM: do not mark all VMCB02 fields dirty on nested vmexit")
Reported-by: Matteo Rizzo <matteorizzo@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index b7fd2e869998..177a9764fb64 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -744,6 +744,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
 	vmcb02->control.nested_ctl = vmcb01->control.nested_ctl;
 	vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa;
 	vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
+	vmcb_mark_dirty(vmcb02, VMCB_PERM_MAP);
 
 	/*
 	 * Stash vmcb02's counter if the guest hasn't moved past the guilty
-- 
2.51.0.470.ga7dc726c21-goog

[PATCH 2/2] KVM: SVM: Mark VMCB_NPT as dirty on nested VMRUN

[Jim Mattson]

Mark the VMCB_NPT bit as dirty in nested_vmcb02_prepare_save()
on every nested VMRUN.

If L1 changes the PAT MSR between two VMRUN instructions on the same
L1 vCPU, the g_pat field in the associated vmcb02 will change, and the
VMCB_NPT clean bit should be cleared.

Fixes: 4bb170a5430b ("KVM: nSVM: do not mark all VMCB02 fields dirty on nested vmexit")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 177a9764fb64..9ecdc462e98c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -613,6 +613,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
 	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) {
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH 0/2] KVM: SVM: Aggressively clear vmcb02 clean bits

[Sean Christopherson]

On Mon, Sep 22, 2025, Jim Mattson wrote:
> It is unlikely that L1 will toggle the MSR intercept bit in vmcb02,
> or that L1 will change its own IA32_PAT MSR. However, if it does,
> the affected fields in vmcb02 should not be marked clean.
> 
> An alternative approach would be to implement a set of mutators for
> vmcb02 fields, and to clear the associated clean bit whenever a field
> is modified.

Any reason not to tag these for stable@?  I can't think of any meaningful
downsides, so erring on the side of caution seems prudent.

Re: [PATCH 0/2] KVM: SVM: Aggressively clear vmcb02 clean bits

[Jim Mattson]

On Mon, Oct 13, 2025 at 2:54 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Sep 22, 2025, Jim Mattson wrote:
> > It is unlikely that L1 will toggle the MSR intercept bit in vmcb02,
> > or that L1 will change its own IA32_PAT MSR. However, if it does,
> > the affected fields in vmcb02 should not be marked clean.
> >
> > An alternative approach would be to implement a set of mutators for
> > vmcb02 fields, and to clear the associated clean bit whenever a field
> > is modified.
>
> Any reason not to tag these for stable@?  I can't think of any meaningful
> downsides, so erring on the side of caution seems prudent.

SGTM. Do you want a new version?

Re: [PATCH 0/2] KVM: SVM: Aggressively clear vmcb02 clean bits

[Sean Christopherson]

On Mon, Oct 13, 2025, Jim Mattson wrote:
> On Mon, Oct 13, 2025 at 2:54 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Sep 22, 2025, Jim Mattson wrote:
> > > It is unlikely that L1 will toggle the MSR intercept bit in vmcb02,
> > > or that L1 will change its own IA32_PAT MSR. However, if it does,
> > > the affected fields in vmcb02 should not be marked clean.
> > >
> > > An alternative approach would be to implement a set of mutators for
> > > vmcb02 fields, and to clear the associated clean bit whenever a field
> > > is modified.
> >
> > Any reason not to tag these for stable@?  I can't think of any meaningful
> > downsides, so erring on the side of caution seems prudent.
> 
> SGTM. Do you want a new version?

Good gravy, no.  :-)

Re: [PATCH 0/2] KVM: SVM: Aggressively clear vmcb02 clean bits

[Sean Christopherson]

On Mon, 22 Sep 2025 09:29:21 -0700, Jim Mattson wrote:
> It is unlikely that L1 will toggle the MSR intercept bit in vmcb02,
> or that L1 will change its own IA32_PAT MSR. However, if it does,
> the affected fields in vmcb02 should not be marked clean.
> 
> An alternative approach would be to implement a set of mutators for
> vmcb02 fields, and to clear the associated clean bit whenever a field
> is modified.
> 
> [...]

Applied to kvm-x86 svm, thanks!

[1/2] KVM: SVM: Mark VMCB_PERM_MAP as dirty on nested VMRUN
      https://github.com/kvm-x86/linux/commit/93c9e107386d
[2/2] KVM: SVM: Mark VMCB_NPT as dirty on nested VMRUN
      https://github.com/kvm-x86/linux/commit/7c8b465a1c91

--
https://github.com/kvm-x86/linux/tree/next