Re: [PATCH v7 14/26] KVM: nSVM: Drop the non-architectural consistency check for NP_ENABLE

Re: [PATCH v7 14/26] KVM: nSVM: Drop the non-architectural consistency check for NP_ENABLE

[kernel test robot]

:::::: 
:::::: Manual check reason: "high confidence checkpatch report"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20260303003421.2185681-15-yosry@kernel.org>
References: <20260303003421.2185681-15-yosry@kernel.org>
TO: Yosry Ahmed <yosry@kernel.org>
TO: Sean Christopherson <seanjc@google.com>

Hi Yosry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 183bb0ce8c77b0fd1fb25874112bc8751a461e49]

url:    https://github.com/intel-lab-lkp/linux/commits/Yosry-Ahmed/KVM-nSVM-Avoid-clearing-VMCB_LBR-in-vmcb12/20260303-084832
base:   183bb0ce8c77b0fd1fb25874112bc8751a461e49
patch link:    https://lore.kernel.org/r/20260303003421.2185681-15-yosry%40kernel.org
patch subject: [PATCH v7 14/26] KVM: nSVM: Drop the non-architectural consistency check for NP_ENABLE
:::::: branch date: 36 minutes ago
:::::: commit date: 37 minutes ago
reproduce: (https://download.01.org/0day-ci/archive/20260303/202603030209.SBjun6NN-lkp@intel.com/reproduce)

# many are suggestions rather than must-fix

WARNING:REPEATED_WORD: Possible repeated word: 'Nested'
#23: 
Fixes: 4b16184c1cca ("KVM: SVM: Initialize Nested Nested MMU context on VMRUN")

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v7 00/26] Nested SVM fixes, cleanups, and hardening

[Yosry Ahmed]

A group of semi-related fixes, cleanups, and hardening patches for nSVM.
The series is essentially a group of related mini-series stitched
together for syntactic and semantic dependencies. The first 17 patches
(except patch 3) are all optimistically CC'd to stable as they are fixes
or refactoring leading up to bug fixes. Although I am not sure how much
of that will actually apply to stable trees.

Patches 1-3 here are v2 of the last 3 patches in the LBRV fixes series
[1]. The first 3 patches of [1] are already upstream.

Patches 4-12 are fixes for failure handling in the nested VMRUN and
#VMEXIT code paths.

Patches 13-17 are fixes for missing or made-up consistency checks.

Patches 18-19 are renames and cleanups.

Patches 20-25 add hardening to reading the VMCB12, caching all used
fields in the save area to prevent theoritical TOC-TOU bugs, sanitizing
used fields in the control area, and restricting accesses to the VMCB12
through guest memory.

Finally, patch 26 is a selftest for nested VMRUN and #VMEXIT failures
due to failing to map vmcb12.

v6 -> v7:
- Dropped unification of VMRUN failure paths and refactoring patches
  leading up to it, consistency checks are now moved into the helper
  copying vmcb12 to cache instead of enter_svm_guest_mode().
- Clear reserved bits in dbgctl in KVM_SET_NESTED_STATE.
- Dropped consistency check on hCR0 as CR0.PG is already checked.
- Dropped redundant check on CR4.PAE in new CS consistency check.
- Correctly cache clean bits from vmcb12.
- Update selftest to use a single VMRUN instruction and avoid missing
  post-VMRUN L1 registers restore.

v6: https://lore.kernel.org/kvm/20260224223405.3270433-1-yosry@kernel.org/

[1]https://lore.kernel.org/kvm/20251108004524.1600006-1-yosry.ahmed@linux.dev/

Yosry Ahmed (26):
  KVM: nSVM: Avoid clearing VMCB_LBR in vmcb12
  KVM: SVM: Switch svm_copy_lbrs() to a macro
  KVM: SVM: Add missing save/restore handling of LBR MSRs
  KVM: selftests: Add a test for LBR save/restore (ft. nested)
  KVM: nSVM: Always inject a #GP if mapping VMCB12 fails on nested VMRUN
  KVM: nSVM: Refactor checking LBRV enablement in vmcb12 into a helper
  KVM: nSVM: Refactor writing vmcb12 on nested #VMEXIT as a helper
  KVM: nSVM: Triple fault if mapping VMCB12 fails on nested #VMEXIT
  KVM: nSVM: Triple fault if restore host CR3 fails on nested #VMEXIT
  KVM: nSVM: Clear GIF on nested #VMEXIT(INVALID)
  KVM: nSVM: Clear EVENTINJ fields in vmcb12 on nested #VMEXIT
  KVM: nSVM: Clear tracking of L1->L2 NMI and soft IRQ on nested #VMEXIT
  KVM: nSVM: Drop nested_vmcb_check_{save/control}() wrappers
  KVM: nSVM: Drop the non-architectural consistency check for NP_ENABLE
  KVM: nSVM: Add missing consistency check for nCR3 validity
  KVM: nSVM: Add missing consistency check for EFER, CR0, CR4, and CS
  KVM: nSVM: Add missing consistency check for EVENTINJ
  KVM: SVM: Rename vmcb->nested_ctl to vmcb->misc_ctl
  KVM: SVM: Rename vmcb->virt_ext to vmcb->misc_ctl2
  KVM: nSVM: Cache all used fields from VMCB12
  KVM: nSVM: Restrict mapping vmcb12 on nested VMRUN
  KVM: nSVM: Use PAGE_MASK to drop lower bits of bitmap GPAs from vmcb12
  KVM: nSVM: Sanitize TLB_CONTROL field when copying from vmcb12
  KVM: nSVM: Sanitize INT/EVENTINJ fields when copying from vmcb12
  KVM: nSVM: Only copy SVM_MISC_ENABLE_NP from VMCB01's misc_ctl
  KVM: selftest: Add a selftest for VMRUN/#VMEXIT with unmappable vmcb12

 arch/x86/include/asm/svm.h                    |  20 +-
 arch/x86/kvm/svm/nested.c                     | 459 +++++++++++-------
 arch/x86/kvm/svm/sev.c                        |   4 +-
 arch/x86/kvm/svm/svm.c                        |  72 +--
 arch/x86/kvm/svm/svm.h                        |  50 +-
 arch/x86/kvm/x86.c                            |   3 +
 tools/testing/selftests/kvm/Makefile.kvm      |   2 +
 .../selftests/kvm/include/x86/processor.h     |   5 +
 tools/testing/selftests/kvm/include/x86/svm.h |  14 +-
 tools/testing/selftests/kvm/lib/x86/svm.c     |   2 +-
 .../kvm/x86/nested_vmsave_vmload_test.c       |  16 +-
 .../selftests/kvm/x86/svm_lbr_nested_state.c  | 145 ++++++
 .../kvm/x86/svm_nested_invalid_vmcb12_gpa.c   |  98 ++++
 13 files changed, 644 insertions(+), 246 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/svm_lbr_nested_state.c
 create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c


base-commit: 183bb0ce8c77b0fd1fb25874112bc8751a461e49
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v7 14/26] KVM: nSVM: Drop the non-architectural consistency check for NP_ENABLE

[Yosry Ahmed]

KVM currenty fails a nested VMRUN and injects VMEXIT_INVALID (aka
SVM_EXIT_ERR) if L1 sets NP_ENABLE and the host does not support NPTs.
On first glance, it seems like the check should actually be for
guest_cpu_cap_has(X86_FEATURE_NPT) instead, as it is possible for the
host to support NPTs but the guest CPUID to not advertise it.

However, the consistency check is not architectural to begin with. The
APM does not mention VMEXIT_INVALID if NP_ENABLE is set on a processor
that does not have X86_FEATURE_NPT. Hence, NP_ENABLE should be ignored
if X86_FEATURE_NPT is not available for L1, so sanitize it when copying
from the VMCB12 to KVM's cache.

Apart from the consistency check, NP_ENABLE in VMCB12 is currently
ignored because the bit is actually copied from VMCB01 to VMCB02, not
from VMCB12.

Fixes: 4b16184c1cca ("KVM: SVM: Initialize Nested Nested MMU context on VMRUN")
Cc: stable@vger.kernel.org
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/nested.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 21e1a43c91879..613d5e2e7c3d1 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -348,9 +348,6 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 	if (CC(control->asid == 0))
 		return false;
 
-	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) && !npt_enabled))
-		return false;
-
 	if (CC(!nested_svm_check_bitmap_pa(vcpu, control->msrpm_base_pa,
 					   MSRPM_SIZE)))
 		return false;
@@ -431,6 +428,11 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
 	nested_svm_sanitize_intercept(vcpu, to, SKINIT);
 	nested_svm_sanitize_intercept(vcpu, to, RDPRU);
 
+	/* Always clear SVM_NESTED_CTL_NP_ENABLE if the guest cannot use NPTs */
+	to->nested_ctl          = from->nested_ctl;
+	if (!guest_cpu_cap_has(vcpu, X86_FEATURE_NPT))
+		to->nested_ctl &= ~SVM_NESTED_CTL_NP_ENABLE;
+
 	to->iopm_base_pa        = from->iopm_base_pa;
 	to->msrpm_base_pa       = from->msrpm_base_pa;
 	to->tsc_offset          = from->tsc_offset;
@@ -444,7 +446,6 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
 	to->exit_info_2         = from->exit_info_2;
 	to->exit_int_info       = from->exit_int_info;
 	to->exit_int_info_err   = from->exit_int_info_err;
-	to->nested_ctl          = from->nested_ctl;
 	to->event_inj           = from->event_inj;
 	to->event_inj_err       = from->event_inj_err;
 	to->next_rip            = from->next_rip;
-- 
2.53.0.473.g4a7958ca14-goog