| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220607213604.3346000-13-seanjc@google.com>
Date: Tue, 7 Jun 2022 21:36:01 +0000
From: Sean Christopherson <seanjc@...gle.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, Eric Li <ercli@...avis.edu>,
David Matlack <dmatlack@...gle.com>,
Oliver Upton <oupton@...gle.com>
Subject: [PATCH v5 12/15] KVM: nVMX: Extend VMX MSRs quirk to CR0/4 fixed1 bits
Extend the VMX MSRs quirk to the CR0/4_FIXED1 MSRs, i.e. when the quirk
is disabled, allow userspace to set the MSRs and do not rewrite the MSRs
during CPUID updates. The bits that the guest (L2 in this case) is
allowed to set are not directly tied to CPUID. Enumerating to L1 that it
can set reserved CR0/4 bits is nonsensical and will ultimately result in
a failed nested VM-Entry (KVM enforces guest reserved CR4 bits on top of
the VMX MSRs), but KVM typically doesn't police the vCPU model except
when doing so is necessary to protect the host kernel.
Further restricting CR4 bits is however a reasonable thing to do, e.g. to
work around a bug in nested virtualization, in which case exposing a
feature to L1 is ok, but letting L2 use the feature is not. Of course,
whether or not the L1 hypervisor will actually _check_ the FIXED1 bits is
another matter entirely, e.g. KVM currently assumes all bits that can be
set in the host can also be set in the guest.
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
Documentation/virt/kvm/api.rst | 8 ++++++++
arch/x86/kvm/vmx/nested.c | 33 ++++++++++++++++++++++++++++++---
arch/x86/kvm/vmx/vmx.c | 6 +++---
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 1095692ddab7..88d1bbae031e 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7391,6 +7391,14 @@ The valid bits in cap.args[0] are:
IA32_VMX_TRUE_EXIT_CTLS[bit 44]
('load IA32_PERF_GLOBAL_CTRL'). Otherwise,
these corresponding MSR bits are cleared.
+ - MSR_IA32_VMX_CR0_FIXED1 is unconditionally
+ set to 0xffffffff
+ - CR4.PCE is unconditionally set in
+ MSR_IA32_VMX_CR4_FIXED1.
+ - All CR4 bits with an associated CPUID
+ feature flag are set in
+ MSR_IA32_VMX_CR4_FIXED1 if the feature is
+ reported as supported in guest CPUID.
When this quirk is disabled, KVM will not
change the values of the aformentioned VMX
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 5533c2474128..abce74cfefc9 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -1385,6 +1385,30 @@ static int vmx_restore_fixed0_msr(struct vcpu_vmx *vmx, u32 msr_index, u64 data)
return 0;
}
+static u64 *vmx_get_fixed1_msr(struct nested_vmx_msrs *msrs, u32 msr_index)
+{
+ switch (msr_index) {
+ case MSR_IA32_VMX_CR0_FIXED1:
+ return &msrs->cr0_fixed1;
+ case MSR_IA32_VMX_CR4_FIXED1:
+ return &msrs->cr4_fixed1;
+ default:
+ BUG();
+ }
+}
+
+static int vmx_restore_fixed1_msr(struct vcpu_vmx *vmx, u32 msr_index, u64 data)
+{
+ const u64 *msr = vmx_get_fixed1_msr(&vmcs_config.nested, msr_index);
+
+ /* Bits that "must-be-0" must not be set in the restored value. */
+ if (!is_bitwise_subset(*msr, data, -1ULL))
+ return -EINVAL;
+
+ *vmx_get_fixed1_msr(&vmx->nested.msrs, msr_index) = data;
+ return 0;
+}
+
/*
* Called when userspace is restoring VMX MSRs.
*
@@ -1432,10 +1456,13 @@ int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
case MSR_IA32_VMX_CR0_FIXED1:
case MSR_IA32_VMX_CR4_FIXED1:
/*
- * These MSRs are generated based on the vCPU's CPUID, so we
- * do not support restoring them directly.
+ * These MSRs are generated based on the vCPU's CPUID when KVM
+ * "owns" the VMX MSRs, do not allow restoring them directly.
*/
- return -EINVAL;
+ if (kvm_check_has_quirk(vmx->vcpu.kvm, KVM_X86_QUIRK_TWEAK_VMX_MSRS))
+ return -EINVAL;
+
+ return vmx_restore_fixed1_msr(vmx, msr_index, data);
case MSR_IA32_VMX_EPT_VPID_CAP:
return vmx_restore_vmx_ept_vpid_cap(vmx, data);
case MSR_IA32_VMX_VMCS_ENUM:
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4c31c8f24329..139f365ca6bb 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7520,10 +7520,10 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
~(FEAT_CTL_VMX_ENABLED_INSIDE_SMX |
FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX);
- if (nested_vmx_allowed(vcpu)) {
+ if (nested_vmx_allowed(vcpu) &&
+ kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_TWEAK_VMX_MSRS)) {
nested_vmx_cr_fixed1_bits_update(vcpu);
- if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_TWEAK_VMX_MSRS))
- nested_vmx_entry_exit_ctls_update(vcpu);
+ nested_vmx_entry_exit_ctls_update(vcpu);
}
if (boot_cpu_has(X86_FEATURE_INTEL_PT) &&
--
2.36.1.255.ge46751e96f-goog
Powered by blists - more mailing lists