On 9/23/20 8:53 AM, Paolo Bonzini wrote:
On 29/08/20 02:48, Krish Sadhukhan wrote:
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
---
arch/x86/kvm/svm/nested.c | 51 ++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index fb68467e6049..7a51ce465f3e 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -215,9 +215,35 @@ static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
return true;
}
+static bool nested_vmcb_check_cr3_cr4(struct vcpu_svm *svm,
+ struct vmcb_save_area *save)
+{
+ bool nested_vmcb_lma =
+ (save->efer & EFER_LME) &&
+ (save->cr0 & X86_CR0_PG);
+
+ if (!nested_vmcb_lma) {
+ if (save->cr4 & X86_CR4_PAE) {
+ if (save->cr3 & MSR_CR3_LEGACY_PAE_RESERVED_MASK)
+ return false;
+ } else {
+ if (save->cr3 & MSR_CR3_LEGACY_RESERVED_MASK)
+ return false;
+ }
+ } else {
+ if (!(save->cr4 & X86_CR4_PAE) ||
+ !(save->cr0 & X86_CR0_PE) ||
+ (save->cr3 & MSR_CR3_LONG_MBZ_MASK))
+ return false;
+ }
+ if (kvm_valid_cr4(&svm->vcpu, save->cr4))
+ return false;
+
+ return true;
+}
+
static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb)
{
- bool nested_vmcb_lma;
if ((vmcb->save.efer & EFER_SVME) == 0)
return false;
@@ -228,25 +254,7 @@ static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb)
if (!kvm_dr6_valid(vmcb->save.dr6) || !kvm_dr7_valid(vmcb->save.dr7))
return false;
- nested_vmcb_lma =
- (vmcb->save.efer & EFER_LME) &&
- (vmcb->save.cr0 & X86_CR0_PG);
-
- if (!nested_vmcb_lma) {
- if (vmcb->save.cr4 & X86_CR4_PAE) {
- if (vmcb->save.cr3 & MSR_CR3_LEGACY_PAE_RESERVED_MASK)
- return false;
- } else {
- if (vmcb->save.cr3 & MSR_CR3_LEGACY_RESERVED_MASK)
- return false;
- }
- } else {
- if (!(vmcb->save.cr4 & X86_CR4_PAE) ||
- !(vmcb->save.cr0 & X86_CR0_PE) ||
- (vmcb->save.cr3 & MSR_CR3_LONG_RESERVED_MASK))
- return false;
- }
- if (kvm_valid_cr4(&svm->vcpu, vmcb->save.cr4))
+ if (!nested_vmcb_check_cr3_cr4(svm, &(vmcb->save)))
return false;
return nested_vmcb_check_controls(&vmcb->control);
@@ -1114,9 +1122,8 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
/*
* Validate host state saved from before VMRUN (see
* nested_svm_check_permissions).
- * TODO: validate reserved bits for all saved state.
*/
- if (!(save.cr0 & X86_CR0_PG))
+ if (!nested_vmcb_check_cr3_cr4(svm, &save))
return -EINVAL;
Removing the "if" is incorrect.
Sorry, my mistake.
Also are there really no more reserved
bits to check?
I have sent v2 which has added checks for DR6, DR7 and EFER.
Paolo