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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 20 Aug 2020 12:13:27 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     kvm@...r.kernel.org
Cc:     Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        linux-kernel@...r.kernel.org (open list:X86 ARCHITECTURE (32-BIT AND
        64-BIT)),
        x86@...nel.org (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)),
        Ingo Molnar <mingo@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Maxim Levitsky <mlevitsk@...hat.com>
Subject: [PATCH 8/8] KVM: nSVM: read only changed fields of the nested guest data area

This allows us to only read fields that are marked as dirty by the nested
guest on vmentry.

I doubt that this has any perf impact but this way it is a bit closer
to real hardware.

Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
---
 arch/x86/kvm/svm/nested.c | 58 +++++++++++++++++++++++++--------------
 arch/x86/kvm/svm/svm.c    |  2 +-
 arch/x86/kvm/svm/svm.h    |  5 ++++
 3 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index acc4b26fcfcc..f3eef48caee6 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -266,40 +266,57 @@ static void load_nested_vmcb_control(struct vcpu_svm *svm,
 }
 
 static void load_nested_vmcb_save(struct vcpu_svm *svm,
-				  struct vmcb_save_area *save)
+				  struct vmcb_save_area *save,
+				  u32 clean)
 {
 	svm->nested.vmcb->save.rflags = save->rflags;
 	svm->nested.vmcb->save.rax    = save->rax;
 	svm->nested.vmcb->save.rsp    = save->rsp;
 	svm->nested.vmcb->save.rip    = save->rip;
 
-	svm->nested.vmcb->save.es  = save->es;
-	svm->nested.vmcb->save.cs  = save->cs;
-	svm->nested.vmcb->save.ss  = save->ss;
-	svm->nested.vmcb->save.ds  = save->ds;
-	svm->nested.vmcb->save.cpl = save->cpl;
+	if (is_dirty(clean, VMCB_SEG)) {
+		svm->nested.vmcb->save.es  = save->es;
+		svm->nested.vmcb->save.cs  = save->cs;
+		svm->nested.vmcb->save.ss  = save->ss;
+		svm->nested.vmcb->save.ds  = save->ds;
+		svm->nested.vmcb->save.cpl = save->cpl;
+	}
 
-	svm->nested.vmcb->save.gdtr = save->gdtr;
-	svm->nested.vmcb->save.idtr = save->idtr;
+	if (is_dirty(clean, VMCB_DT)) {
+		svm->nested.vmcb->save.gdtr = save->gdtr;
+		svm->nested.vmcb->save.idtr = save->idtr;
+	}
 
-	svm->nested.vmcb->save.efer = save->efer;
-	svm->nested.vmcb->save.cr3 = save->cr3;
-	svm->nested.vmcb->save.cr4 = save->cr4;
-	svm->nested.vmcb->save.cr0 = save->cr0;
+	if (is_dirty(clean, VMCB_CR)) {
+		svm->nested.vmcb->save.efer = save->efer;
+		svm->nested.vmcb->save.cr3 = save->cr3;
+		svm->nested.vmcb->save.cr4 = save->cr4;
+		svm->nested.vmcb->save.cr0 = save->cr0;
+	}
 
-	svm->nested.vmcb->save.cr2 = save->cr2;
+	if (is_dirty(clean, VMCB_CR2))
+		svm->nested.vmcb->save.cr2 = save->cr2;
 
-	svm->nested.vmcb->save.dr7 = save->dr7;
-	svm->nested.vmcb->save.dr6 = save->dr6;
+	if (is_dirty(clean, VMCB_DR)) {
+		svm->nested.vmcb->save.dr7 = save->dr7;
+		svm->nested.vmcb->save.dr6 = save->dr6;
+	}
 
-	svm->nested.vmcb->save.g_pat = save->g_pat;
+	if ((clean & VMCB_NPT) == 0)
+		svm->nested.vmcb->save.g_pat = save->g_pat;
 }
 
 void load_nested_vmcb(struct vcpu_svm *svm, struct vmcb *nested_vmcb, u64 vmcb_gpa)
 {
-	svm->nested.vmcb_gpa = vmcb_gpa;
+	u32 clean = nested_vmcb->control.clean;
+
+	if (svm->nested.vmcb_gpa != vmcb_gpa) {
+		svm->nested.vmcb_gpa = vmcb_gpa;
+		clean = 0;
+	}
+
 	load_nested_vmcb_control(svm, &nested_vmcb->control);
-	load_nested_vmcb_save(svm, &nested_vmcb->save);
+	load_nested_vmcb_save(svm, &nested_vmcb->save, clean);
 }
 
 /*
@@ -619,7 +636,6 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 
 	/* Exit Guest-Mode */
 	leave_guest_mode(&svm->vcpu);
-	svm->nested.vmcb_gpa = 0;
 	WARN_ON_ONCE(svm->nested.nested_run_pending);
 
 	/* in case we halted in L2 */
@@ -676,7 +692,7 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 	 * Note: since CPU might have changed the values we can't
 	 * trust clean bits
 	 */
-	load_nested_vmcb_save(svm, &nested_vmcb->save);
+	load_nested_vmcb_save(svm, &nested_vmcb->save, 0);
 
 	/* Restore the original control entries */
 	copy_vmcb_control_area(&vmcb->control, &hsave->control);
@@ -759,6 +775,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)
 		goto free_page3;
 
 	svm->nested.vmcb = page_address(vmcb_page);
+	svm->nested.vmcb_gpa = U64_MAX;
 	clear_page(svm->nested.vmcb);
 
 	svm->nested.initialized = true;
@@ -785,6 +802,7 @@ void svm_free_nested(struct vcpu_svm *svm)
 
 	__free_page(virt_to_page(svm->nested.vmcb));
 	svm->nested.vmcb = NULL;
+	svm->nested.vmcb_gpa = U64_MAX;
 
 	svm->nested.initialized = false;
 }
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 06668e0f93e7..f0bb7f622dca 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3924,7 +3924,7 @@ static int svm_pre_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
 		if (kvm_vcpu_map(&svm->vcpu, gpa_to_gfn(vmcb_gpa), &map) == -EINVAL)
 			return 1;
 
-		load_nested_vmcb(svm, map.hva, vmcb);
+		load_nested_vmcb(svm, map.hva, vmcb_gpa);
 		ret = enter_svm_guest_mode(svm);
 
 		kvm_vcpu_unmap(&svm->vcpu, &map, true);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 80231ef8de6f..4a383c519fdf 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -204,6 +204,11 @@ static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
 	vmcb->control.clean &= ~(1 << bit);
 }
 
+static inline bool is_dirty(u32 clean, int bit)
+{
+	return (clean & (1 << bit)) == 0;
+}
+
 static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
 {
 	return container_of(vcpu, struct vcpu_svm, vcpu);
-- 
2.26.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ