Re: [PATCH 0/6] KVM: SVM: Wrap access to intercept masks into functions

Previous thread: [PATCH 2/6] KVM: SVM: Add manipulation functions for CRx intercepts by Joerg Roedel on Tuesday, November 30, 2010 - 10:03 am. (3 messages)

Next thread: [PATCH v2] soc_camera: Add the ability to bind regulators to soc_camedra devices by Alberto Panizzo on Tuesday, November 30, 2010 - 10:12 am. (4 messages)
From: Joerg Roedel
Date: Tuesday, November 30, 2010 - 10:03 am

Hi Avi, Hi Marcelo,

this patchset wraps the access to the intercept vectors in the VMCB into
specific functions. There are two reasons for this:

	1) In the nested-svm code the effective intercept masks are
	   calculated from the host and the guest intercept masks.
	   Whenever KVM changes the host intercept mask while the VCPU
	   is in guest-mode the effective intercept masks need to be
	   re-calculated. This is nicely wrapped into these functions
	   now and makes the code more robust.

	2) These changes make the implementation of the upcoming
	   vmcb-clean-bits feature easier and also more robust (which
	   was the main reason for writing this patchset).

These patches were developed on-top of the patch-set I sent yesterday. I
tested these patches with various guests (Windows-64, Linux 32,32e and
64 as well as with nested-svm).

Regards,

	Joerg

Summary:

 arch/x86/include/asm/svm.h |   44 +++--
 arch/x86/kvm/svm.c         |  391 ++++++++++++++++++++++++--------------------
 2 files changed, 241 insertions(+), 194 deletions(-)

Joerg Roedel (6):
      KVM: SVM: Add function to recalculate intercept masks
      KVM: SVM: Add manipulation functions for CRx intercepts
      KVM: SVM: Add manipulation functions for DRx intercepts
      KVM: SVM: Add manipulation functions for exception intercepts
      KVM: SVM: Add manipulation functions for misc intercepts
      KVM: SVM: Use get_host_vmcb function in svm_get_msr for TSC


--

From: Joerg Roedel
Date: Tuesday, November 30, 2010 - 10:04 am

This patch replaces the open-coded vmcb-selection for the
TSC calculation with the new get_host_vmcb helper function
introduced in this patchset.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 6b51d31..0803795 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2628,14 +2628,9 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)
 
 	switch (ecx) {
 	case MSR_IA32_TSC: {
-		u64 tsc_offset;
+		struct vmcb *vmcb = get_host_vmcb(svm);
 
-		if (is_guest_mode(vcpu))
-			tsc_offset = svm->nested.hsave->control.tsc_offset;
-		else
-			tsc_offset = svm->vmcb->control.tsc_offset;
-
-		*data = tsc_offset + native_read_tsc();
+		*data = vmcb->control.tsc_offset + native_read_tsc();
 		break;
 	}
 	case MSR_STAR:
-- 
1.7.1


--

From: Joerg Roedel
Date: Tuesday, November 30, 2010 - 10:03 am

This patch wraps changes to the DRx intercepts of SVM into
seperate functions to abstract nested-svm better and prepare
the implementation of the vmcb-clean-bits feature.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/svm.h |   27 +++++++++-----
 arch/x86/kvm/svm.c         |   80 ++++++++++++++++++++++++--------------------
 2 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 39f9ddf..11dbca7 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -52,8 +52,7 @@ enum {
 
 struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 intercept_cr;
-	u16 intercept_dr_read;
-	u16 intercept_dr_write;
+	u32 intercept_dr;
 	u32 intercept_exceptions;
 	u64 intercept;
 	u8 reserved_1[42];
@@ -212,14 +211,22 @@ struct __attribute__ ((__packed__)) vmcb {
 #define INTERCEPT_CR4_WRITE	(16 + 4)
 #define INTERCEPT_CR8_WRITE	(16 + 8)
 
-#define INTERCEPT_DR0_MASK 1
-#define INTERCEPT_DR1_MASK (1 << 1)
-#define INTERCEPT_DR2_MASK (1 << 2)
-#define INTERCEPT_DR3_MASK (1 << 3)
-#define INTERCEPT_DR4_MASK (1 << 4)
-#define INTERCEPT_DR5_MASK (1 << 5)
-#define INTERCEPT_DR6_MASK (1 << 6)
-#define INTERCEPT_DR7_MASK (1 << 7)
+#define INTERCEPT_DR0_READ	0
+#define INTERCEPT_DR1_READ	1
+#define INTERCEPT_DR2_READ	2
+#define INTERCEPT_DR3_READ	3
+#define INTERCEPT_DR4_READ	4
+#define INTERCEPT_DR5_READ	5
+#define INTERCEPT_DR6_READ	6
+#define INTERCEPT_DR7_READ	7
+#define INTERCEPT_DR0_WRITE	(16 + 0)
+#define INTERCEPT_DR1_WRITE	(16 + 1)
+#define INTERCEPT_DR2_WRITE	(16 + 2)
+#define INTERCEPT_DR3_WRITE	(16 + 3)
+#define INTERCEPT_DR4_WRITE	(16 + 4)
+#define INTERCEPT_DR5_WRITE	(16 + 5)
+#define INTERCEPT_DR6_WRITE	(16 + 6)
+#define INTERCEPT_DR7_WRITE	(16 + 7)
 
 #define SVM_EVTINJ_VEC_MASK 0xff
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 53d24ea..5250a2b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -99,8 +99,7 @@ ...
From: Joerg Roedel
Date: Tuesday, November 30, 2010 - 10:04 am

This patch wraps changes to the misc intercepts of SVM
into seperate functions to abstract nested-svm better and
prepare the implementation of the vmcb-clean-bits feature.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   84 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 783142e..6b51d31 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -277,6 +277,24 @@ static inline void clr_exception_intercept(struct vcpu_svm *svm, int bit)
 	recalc_intercepts(svm);
 }
 
+static inline void set_intercept(struct vcpu_svm *svm, int bit)
+{
+	struct vmcb *vmcb = get_host_vmcb(svm);
+
+	vmcb->control.intercept |= (1ULL << bit);
+
+	recalc_intercepts(svm);
+}
+
+static inline void clr_intercept(struct vcpu_svm *svm, int bit)
+{
+	struct vmcb *vmcb = get_host_vmcb(svm);
+
+	vmcb->control.intercept &= ~(1ULL << bit);
+
+	recalc_intercepts(svm);
+}
+
 static inline void enable_gif(struct vcpu_svm *svm)
 {
 	svm->vcpu.arch.hflags |= HF_GIF_MASK;
@@ -862,29 +880,29 @@ static void init_vmcb(struct vcpu_svm *svm)
 	set_exception_intercept(svm, UD_VECTOR);
 	set_exception_intercept(svm, MC_VECTOR);
 
-	control->intercept =	(1ULL << INTERCEPT_INTR) |
-				(1ULL << INTERCEPT_NMI) |
-				(1ULL << INTERCEPT_SMI) |
-				(1ULL << INTERCEPT_SELECTIVE_CR0) |
-				(1ULL << INTERCEPT_CPUID) |
-				(1ULL << INTERCEPT_INVD) |
-				(1ULL << INTERCEPT_HLT) |
-				(1ULL << INTERCEPT_INVLPG) |
-				(1ULL << INTERCEPT_INVLPGA) |
-				(1ULL << INTERCEPT_IOIO_PROT) |
-				(1ULL << INTERCEPT_MSR_PROT) |
-				(1ULL << INTERCEPT_TASK_SWITCH) |
-				(1ULL << INTERCEPT_SHUTDOWN) |
-				(1ULL << INTERCEPT_VMRUN) |
-				(1ULL << INTERCEPT_VMMCALL) |
-				(1ULL << INTERCEPT_VMLOAD) |
-				(1ULL << INTERCEPT_VMSAVE) |
-				(1ULL << INTERCEPT_STGI) |
-				(1ULL << INTERCEPT_CLGI) |
-				(1ULL << INTERCEPT_SKINIT) |
-				(1ULL << INTERCEPT_WBINVD) ...
From: Avi Kivity
Date: Tuesday, November 30, 2010 - 10:42 am

Looks good.

One potential issue is that a series of set_intercept()s causes 
recalc_intercepts() to be called multiple times.  If it turns out to be 
a problem, we can fix it by having an intercepts dirty bit and 
recalculating during guest entry if the bit is set.

Since it's nested svm only, I doubt we'll see a problem in the short term.

-- 
error compiling committee.c: too many arguments to function

--

From: Roedel, Joerg
Date: Wednesday, December 1, 2010 - 3:34 am

Yes, we can optimize it if it turns out to be a problem. I thought that
these changes happen infrequently enough and that there should be no
more than 2-3 bits changed per exit-entry cycle at all.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

--

From: Marcelo Tosatti
Date: Friday, December 3, 2010 - 1:23 pm

Applied, thanks.

--

Previous thread: [PATCH 2/6] KVM: SVM: Add manipulation functions for CRx intercepts by Joerg Roedel on Tuesday, November 30, 2010 - 10:03 am. (3 messages)

Next thread: [PATCH v2] soc_camera: Add the ability to bind regulators to soc_camedra devices by Alberto Panizzo on Tuesday, November 30, 2010 - 10:12 am. (4 messages)