2010-11-30 17:05:03

by Joerg Roedel

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

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


2010-11-30 17:04:22

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 4/6] KVM: SVM: Add manipulation functions for exception intercepts

This patch wraps changes to the exception 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 <[email protected]>
---
arch/x86/kvm/svm.c | 58 ++++++++++++++++++++++++++--------------------------
1 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 5250a2b..783142e 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -259,6 +259,24 @@ static inline void clr_dr_intercept(struct vcpu_svm *svm, int bit)
recalc_intercepts(svm);
}

+static inline void set_exception_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_exceptions |= (1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline void clr_exception_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_exceptions &= ~(1U << bit);
+
+ recalc_intercepts(svm);
+}
+
static inline void enable_gif(struct vcpu_svm *svm)
{
svm->vcpu.arch.hflags |= HF_GIF_MASK;
@@ -840,10 +858,9 @@ static void init_vmcb(struct vcpu_svm *svm)
set_dr_intercept(svm, INTERCEPT_DR6_WRITE);
set_dr_intercept(svm, INTERCEPT_DR7_WRITE);

- control->intercept_exceptions = (1 << PF_VECTOR) |
- (1 << UD_VECTOR) |
- (1 << MC_VECTOR);
-
+ set_exception_intercept(svm, PF_VECTOR);
+ set_exception_intercept(svm, UD_VECTOR);
+ set_exception_intercept(svm, MC_VECTOR);

control->intercept = (1ULL << INTERCEPT_INTR) |
(1ULL << INTERCEPT_NMI) |
@@ -920,7 +937,7 @@ static void init_vmcb(struct vcpu_svm *svm)
control->nested_ctl = 1;
control->intercept &= ~((1ULL << INTERCEPT_TASK_SWITCH) |
(1ULL << INTERCEPT_INVLPG));
- control->intercept_exceptions &= ~(1 << PF_VECTOR);
+ clr_exception_intercept(svm, PF_VECTOR);
clr_cr_intercept(svm, INTERCEPT_CR3_READ);
clr_cr_intercept(svm, INTERCEPT_CR3_WRITE);
save->g_pat = 0x0007040600070406ULL;
@@ -1381,20 +1398,18 @@ static void update_db_intercept(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);

- svm->vmcb->control.intercept_exceptions &=
- ~((1 << DB_VECTOR) | (1 << BP_VECTOR));
+ clr_exception_intercept(svm, DB_VECTOR);
+ clr_exception_intercept(svm, BP_VECTOR);

if (svm->nmi_singlestep)
- svm->vmcb->control.intercept_exceptions |= (1 << DB_VECTOR);
+ set_exception_intercept(svm, DB_VECTOR);

if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) {
if (vcpu->guest_debug &
(KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
- svm->vmcb->control.intercept_exceptions |=
- 1 << DB_VECTOR;
+ set_exception_intercept(svm, DB_VECTOR);
if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
- svm->vmcb->control.intercept_exceptions |=
- 1 << BP_VECTOR;
+ set_exception_intercept(svm, BP_VECTOR);
} else
vcpu->guest_debug = 0;
}
@@ -1515,21 +1530,8 @@ static int ud_interception(struct vcpu_svm *svm)
static void svm_fpu_activate(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
- u32 excp;
-
- if (is_guest_mode(vcpu)) {
- u32 h_excp, n_excp;

- h_excp = svm->nested.hsave->control.intercept_exceptions;
- n_excp = svm->nested.intercept_exceptions;
- h_excp &= ~(1 << NM_VECTOR);
- excp = h_excp | n_excp;
- } else {
- excp = svm->vmcb->control.intercept_exceptions;
- excp &= ~(1 << NM_VECTOR);
- }
-
- svm->vmcb->control.intercept_exceptions = excp;
+ clr_exception_intercept(svm, NM_VECTOR);

svm->vcpu.fpu_active = 1;
update_cr0_intercept(svm);
@@ -3635,9 +3637,7 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);

- svm->vmcb->control.intercept_exceptions |= 1 << NM_VECTOR;
- if (is_guest_mode(vcpu))
- svm->nested.hsave->control.intercept_exceptions |= 1 << NM_VECTOR;
+ set_exception_intercept(svm, NM_VECTOR);
update_cr0_intercept(svm);
}

--
1.7.1

2010-11-30 17:04:27

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 1/6] KVM: SVM: Add function to recalculate intercept masks

This patch adds a function to recalculate the effective
intercepts masks when the vcpu is in guest-mode and either
the host or the guest intercept masks change.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kvm/svm.c | 43 ++++++++++++++++++++++++++-----------------
1 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index bff391e..05fe851 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -192,6 +192,26 @@ static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
return container_of(vcpu, struct vcpu_svm, vcpu);
}

+static void recalc_intercepts(struct vcpu_svm *svm)
+{
+ struct vmcb_control_area *c, *h;
+ struct nested_state *g;
+
+ if (!is_guest_mode(&svm->vcpu))
+ return;
+
+ c = &svm->vmcb->control;
+ h = &svm->nested.hsave->control;
+ g = &svm->nested;
+
+ c->intercept_cr_read = h->intercept_cr_read | g->intercept_cr_read;
+ c->intercept_cr_write = h->intercept_cr_write | g->intercept_cr_write;
+ c->intercept_dr_read = h->intercept_dr_read | g->intercept_dr_read;
+ c->intercept_dr_write = h->intercept_dr_write | g->intercept_dr_write;
+ c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
+ c->intercept = h->intercept | g->intercept;
+}
+
static inline void enable_gif(struct vcpu_svm *svm)
{
svm->vcpu.arch.hflags |= HF_GIF_MASK;
@@ -2272,23 +2292,6 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
/* We don't want to see VMMCALLs from a nested guest */
svm->vmcb->control.intercept &= ~(1ULL << INTERCEPT_VMMCALL);

- /*
- * We don't want a nested guest to be more powerful than the guest, so
- * all intercepts are ORed
- */
- svm->vmcb->control.intercept_cr_read |=
- nested_vmcb->control.intercept_cr_read;
- svm->vmcb->control.intercept_cr_write |=
- nested_vmcb->control.intercept_cr_write;
- svm->vmcb->control.intercept_dr_read |=
- nested_vmcb->control.intercept_dr_read;
- svm->vmcb->control.intercept_dr_write |=
- nested_vmcb->control.intercept_dr_write;
- svm->vmcb->control.intercept_exceptions |=
- nested_vmcb->control.intercept_exceptions;
-
- svm->vmcb->control.intercept |= nested_vmcb->control.intercept;
-
svm->vmcb->control.lbr_ctl = nested_vmcb->control.lbr_ctl;
svm->vmcb->control.int_vector = nested_vmcb->control.int_vector;
svm->vmcb->control.int_state = nested_vmcb->control.int_state;
@@ -2301,6 +2304,12 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
/* Enter Guest-Mode */
enter_guest_mode(&svm->vcpu);

+ /*
+ * Merge guest and host intercepts - must be called with vcpu in
+ * guest-mode to take affect here
+ */
+ recalc_intercepts(svm);
+
svm->nested.vmcb = vmcb_gpa;

enable_gif(svm);
--
1.7.1

2010-11-30 17:04:43

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 2/6] KVM: SVM: Add manipulation functions for CRx intercepts

This patch wraps changes to the CRx 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 <[email protected]>
---
arch/x86/include/asm/svm.h | 15 +++--
arch/x86/kvm/svm.c | 119 +++++++++++++++++++++++---------------------
2 files changed, 72 insertions(+), 62 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 0e83105..39f9ddf 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -51,8 +51,7 @@ enum {


struct __attribute__ ((__packed__)) vmcb_control_area {
- u16 intercept_cr_read;
- u16 intercept_cr_write;
+ u32 intercept_cr;
u16 intercept_dr_read;
u16 intercept_dr_write;
u32 intercept_exceptions;
@@ -204,10 +203,14 @@ struct __attribute__ ((__packed__)) vmcb {
#define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
#define SVM_SELECTOR_CODE_MASK (1 << 3)

-#define INTERCEPT_CR0_MASK 1
-#define INTERCEPT_CR3_MASK (1 << 3)
-#define INTERCEPT_CR4_MASK (1 << 4)
-#define INTERCEPT_CR8_MASK (1 << 8)
+#define INTERCEPT_CR0_READ 0
+#define INTERCEPT_CR3_READ 3
+#define INTERCEPT_CR4_READ 4
+#define INTERCEPT_CR8_READ 8
+#define INTERCEPT_CR0_WRITE (16 + 0)
+#define INTERCEPT_CR3_WRITE (16 + 3)
+#define INTERCEPT_CR4_WRITE (16 + 4)
+#define INTERCEPT_CR8_WRITE (16 + 8)

#define INTERCEPT_DR0_MASK 1
#define INTERCEPT_DR1_MASK (1 << 1)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 05fe851..53d24ea 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -98,8 +98,7 @@ struct nested_state {
unsigned long vmexit_rax;

/* cache for intercepts of the guest */
- u16 intercept_cr_read;
- u16 intercept_cr_write;
+ u32 intercept_cr;
u16 intercept_dr_read;
u16 intercept_dr_write;
u32 intercept_exceptions;
@@ -204,14 +203,46 @@ static void recalc_intercepts(struct vcpu_svm *svm)
h = &svm->nested.hsave->control;
g = &svm->nested;

- c->intercept_cr_read = h->intercept_cr_read | g->intercept_cr_read;
- c->intercept_cr_write = h->intercept_cr_write | g->intercept_cr_write;
+ c->intercept_cr = h->intercept_cr | g->intercept_cr;
c->intercept_dr_read = h->intercept_dr_read | g->intercept_dr_read;
c->intercept_dr_write = h->intercept_dr_write | g->intercept_dr_write;
c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
c->intercept = h->intercept | g->intercept;
}

+static inline struct vmcb *get_host_vmcb(struct vcpu_svm *svm)
+{
+ if (is_guest_mode(&svm->vcpu))
+ return svm->nested.hsave;
+ else
+ return svm->vmcb;
+}
+
+static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_cr |= (1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline void clr_cr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_cr &= ~(1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline bool is_cr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ return vmcb->control.intercept_cr & (1U << bit);
+}
+
static inline void enable_gif(struct vcpu_svm *svm)
{
svm->vcpu.arch.hflags |= HF_GIF_MASK;
@@ -767,14 +798,13 @@ static void init_vmcb(struct vcpu_svm *svm)

svm->vcpu.fpu_active = 1;

- control->intercept_cr_read = INTERCEPT_CR0_MASK |
- INTERCEPT_CR3_MASK |
- INTERCEPT_CR4_MASK;
-
- control->intercept_cr_write = INTERCEPT_CR0_MASK |
- INTERCEPT_CR3_MASK |
- INTERCEPT_CR4_MASK |
- INTERCEPT_CR8_MASK;
+ set_cr_intercept(svm, INTERCEPT_CR0_READ);
+ set_cr_intercept(svm, INTERCEPT_CR3_READ);
+ set_cr_intercept(svm, INTERCEPT_CR4_READ);
+ set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
+ set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
+ set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
+ set_cr_intercept(svm, INTERCEPT_CR8_WRITE);

control->intercept_dr_read = INTERCEPT_DR0_MASK |
INTERCEPT_DR1_MASK |
@@ -875,8 +905,8 @@ static void init_vmcb(struct vcpu_svm *svm)
control->intercept &= ~((1ULL << INTERCEPT_TASK_SWITCH) |
(1ULL << INTERCEPT_INVLPG));
control->intercept_exceptions &= ~(1 << PF_VECTOR);
- control->intercept_cr_read &= ~INTERCEPT_CR3_MASK;
- control->intercept_cr_write &= ~INTERCEPT_CR3_MASK;
+ clr_cr_intercept(svm, INTERCEPT_CR3_READ);
+ clr_cr_intercept(svm, INTERCEPT_CR3_WRITE);
save->g_pat = 0x0007040600070406ULL;
save->cr3 = 0;
save->cr4 = 0;
@@ -1210,7 +1240,6 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)

static void update_cr0_intercept(struct vcpu_svm *svm)
{
- struct vmcb *vmcb = svm->vmcb;
ulong gcr0 = svm->vcpu.arch.cr0;
u64 *hcr0 = &svm->vmcb->save.cr0;

@@ -1222,25 +1251,11 @@ static void update_cr0_intercept(struct vcpu_svm *svm)


if (gcr0 == *hcr0 && svm->vcpu.fpu_active) {
- vmcb->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK;
- vmcb->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
- if (is_guest_mode(&svm->vcpu)) {
- struct vmcb *hsave = svm->nested.hsave;
-
- hsave->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK;
- hsave->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
- vmcb->control.intercept_cr_read |= svm->nested.intercept_cr_read;
- vmcb->control.intercept_cr_write |= svm->nested.intercept_cr_write;
- }
+ clr_cr_intercept(svm, INTERCEPT_CR0_READ);
+ clr_cr_intercept(svm, INTERCEPT_CR0_WRITE);
} else {
- svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
- svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
- if (is_guest_mode(&svm->vcpu)) {
- struct vmcb *hsave = svm->nested.hsave;
-
- hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
- hsave->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
- }
+ set_cr_intercept(svm, INTERCEPT_CR0_READ);
+ set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
}
}

@@ -1900,15 +1915,9 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
case SVM_EXIT_IOIO:
vmexit = nested_svm_intercept_ioio(svm);
break;
- case SVM_EXIT_READ_CR0 ... SVM_EXIT_READ_CR8: {
- u32 cr_bits = 1 << (exit_code - SVM_EXIT_READ_CR0);
- if (svm->nested.intercept_cr_read & cr_bits)
- vmexit = NESTED_EXIT_DONE;
- break;
- }
- case SVM_EXIT_WRITE_CR0 ... SVM_EXIT_WRITE_CR8: {
- u32 cr_bits = 1 << (exit_code - SVM_EXIT_WRITE_CR0);
- if (svm->nested.intercept_cr_write & cr_bits)
+ case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8: {
+ u32 bit = 1U << (exit_code - SVM_EXIT_READ_CR0);
+ if (svm->nested.intercept_cr & bit)
vmexit = NESTED_EXIT_DONE;
break;
}
@@ -1965,8 +1974,7 @@ static inline void copy_vmcb_control_area(struct vmcb *dst_vmcb, struct vmcb *fr
struct vmcb_control_area *dst = &dst_vmcb->control;
struct vmcb_control_area *from = &from_vmcb->control;

- dst->intercept_cr_read = from->intercept_cr_read;
- dst->intercept_cr_write = from->intercept_cr_write;
+ dst->intercept_cr = from->intercept_cr;
dst->intercept_dr_read = from->intercept_dr_read;
dst->intercept_dr_write = from->intercept_dr_write;
dst->intercept_exceptions = from->intercept_exceptions;
@@ -2188,8 +2196,8 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
nested_vmcb->control.event_inj,
nested_vmcb->control.nested_ctl);

- trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr_read,
- nested_vmcb->control.intercept_cr_write,
+ trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr & 0xffff,
+ nested_vmcb->control.intercept_cr >> 16,
nested_vmcb->control.intercept_exceptions,
nested_vmcb->control.intercept);

@@ -2269,8 +2277,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
svm->nested.vmcb_iopm = nested_vmcb->control.iopm_base_pa & ~0x0fffULL;

/* cache intercepts */
- svm->nested.intercept_cr_read = nested_vmcb->control.intercept_cr_read;
- svm->nested.intercept_cr_write = nested_vmcb->control.intercept_cr_write;
+ svm->nested.intercept_cr = nested_vmcb->control.intercept_cr;
svm->nested.intercept_dr_read = nested_vmcb->control.intercept_dr_read;
svm->nested.intercept_dr_write = nested_vmcb->control.intercept_dr_write;
svm->nested.intercept_exceptions = nested_vmcb->control.intercept_exceptions;
@@ -2285,8 +2292,8 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)

if (svm->vcpu.arch.hflags & HF_VINTR_MASK) {
/* We only want the cr8 intercept bits of the guest */
- svm->vmcb->control.intercept_cr_read &= ~INTERCEPT_CR8_MASK;
- svm->vmcb->control.intercept_cr_write &= ~INTERCEPT_CR8_MASK;
+ clr_cr_intercept(svm, INTERCEPT_CR8_READ);
+ clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
}

/* We don't want to see VMMCALLs from a nested guest */
@@ -2578,7 +2585,7 @@ static int cr8_write_interception(struct vcpu_svm *svm)
/* instruction emulation calls kvm_set_cr8() */
emulate_instruction(&svm->vcpu, 0, 0, 0);
if (irqchip_in_kernel(svm->vcpu.kvm)) {
- svm->vmcb->control.intercept_cr_write &= ~INTERCEPT_CR8_MASK;
+ clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
return 1;
}
if (cr8_prev <= kvm_get_cr8(&svm->vcpu))
@@ -2895,8 +2902,8 @@ void dump_vmcb(struct kvm_vcpu *vcpu)
struct vmcb_save_area *save = &svm->vmcb->save;

pr_err("VMCB Control Area:\n");
- pr_err("cr_read: %04x\n", control->intercept_cr_read);
- pr_err("cr_write: %04x\n", control->intercept_cr_write);
+ pr_err("cr_read: %04x\n", control->intercept_cr & 0xffff);
+ pr_err("cr_write: %04x\n", control->intercept_cr >> 16);
pr_err("dr_read: %04x\n", control->intercept_dr_read);
pr_err("dr_write: %04x\n", control->intercept_dr_write);
pr_err("exceptions: %08x\n", control->intercept_exceptions);
@@ -2997,7 +3004,7 @@ static int handle_exit(struct kvm_vcpu *vcpu)

trace_kvm_exit(exit_code, vcpu, KVM_ISA_SVM);

- if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR0_MASK))
+ if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE))
vcpu->arch.cr0 = svm->vmcb->save.cr0;
if (npt_enabled)
vcpu->arch.cr3 = svm->vmcb->save.cr3;
@@ -3123,7 +3130,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
return;

if (tpr >= irr)
- svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR8_MASK;
+ set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
}

static int svm_nmi_allowed(struct kvm_vcpu *vcpu)
@@ -3230,7 +3237,7 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
return;

- if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR8_MASK)) {
+ if (!is_cr_intercept(svm, INTERCEPT_CR8_WRITE)) {
int cr8 = svm->vmcb->control.int_ctl & V_TPR_MASK;
kvm_set_cr8(vcpu, cr8);
}
--
1.7.1

2010-11-30 17:05:12

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 6/6] KVM: SVM: Use get_host_vmcb function in svm_get_msr for TSC

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 <[email protected]>
---
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

2010-11-30 17:05:19

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 3/6] KVM: SVM: Add manipulation functions for DRx intercepts

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 <[email protected]>
---
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 @@ struct nested_state {

/* cache for intercepts of the guest */
u32 intercept_cr;
- u16 intercept_dr_read;
- u16 intercept_dr_write;
+ u32 intercept_dr;
u32 intercept_exceptions;
u64 intercept;

@@ -204,8 +203,7 @@ static void recalc_intercepts(struct vcpu_svm *svm)
g = &svm->nested;

c->intercept_cr = h->intercept_cr | g->intercept_cr;
- c->intercept_dr_read = h->intercept_dr_read | g->intercept_dr_read;
- c->intercept_dr_write = h->intercept_dr_write | g->intercept_dr_write;
+ c->intercept_dr = h->intercept_dr | g->intercept_dr;
c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
c->intercept = h->intercept | g->intercept;
}
@@ -243,6 +241,24 @@ static inline bool is_cr_intercept(struct vcpu_svm *svm, int bit)
return vmcb->control.intercept_cr & (1U << bit);
}

+static inline void set_dr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_dr |= (1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline void clr_dr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_dr &= ~(1U << bit);
+
+ recalc_intercepts(svm);
+}
+
static inline void enable_gif(struct vcpu_svm *svm)
{
svm->vcpu.arch.hflags |= HF_GIF_MASK;
@@ -806,23 +822,23 @@ static void init_vmcb(struct vcpu_svm *svm)
set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
set_cr_intercept(svm, INTERCEPT_CR8_WRITE);

- control->intercept_dr_read = INTERCEPT_DR0_MASK |
- INTERCEPT_DR1_MASK |
- INTERCEPT_DR2_MASK |
- INTERCEPT_DR3_MASK |
- INTERCEPT_DR4_MASK |
- INTERCEPT_DR5_MASK |
- INTERCEPT_DR6_MASK |
- INTERCEPT_DR7_MASK;
-
- control->intercept_dr_write = INTERCEPT_DR0_MASK |
- INTERCEPT_DR1_MASK |
- INTERCEPT_DR2_MASK |
- INTERCEPT_DR3_MASK |
- INTERCEPT_DR4_MASK |
- INTERCEPT_DR5_MASK |
- INTERCEPT_DR6_MASK |
- INTERCEPT_DR7_MASK;
+ set_dr_intercept(svm, INTERCEPT_DR0_READ);
+ set_dr_intercept(svm, INTERCEPT_DR1_READ);
+ set_dr_intercept(svm, INTERCEPT_DR2_READ);
+ set_dr_intercept(svm, INTERCEPT_DR3_READ);
+ set_dr_intercept(svm, INTERCEPT_DR4_READ);
+ set_dr_intercept(svm, INTERCEPT_DR5_READ);
+ set_dr_intercept(svm, INTERCEPT_DR6_READ);
+ set_dr_intercept(svm, INTERCEPT_DR7_READ);
+
+ set_dr_intercept(svm, INTERCEPT_DR0_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR1_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR2_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR3_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR4_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR5_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR6_WRITE);
+ set_dr_intercept(svm, INTERCEPT_DR7_WRITE);

control->intercept_exceptions = (1 << PF_VECTOR) |
(1 << UD_VECTOR) |
@@ -1921,15 +1937,9 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
vmexit = NESTED_EXIT_DONE;
break;
}
- case SVM_EXIT_READ_DR0 ... SVM_EXIT_READ_DR7: {
- u32 dr_bits = 1 << (exit_code - SVM_EXIT_READ_DR0);
- if (svm->nested.intercept_dr_read & dr_bits)
- vmexit = NESTED_EXIT_DONE;
- break;
- }
- case SVM_EXIT_WRITE_DR0 ... SVM_EXIT_WRITE_DR7: {
- u32 dr_bits = 1 << (exit_code - SVM_EXIT_WRITE_DR0);
- if (svm->nested.intercept_dr_write & dr_bits)
+ case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
+ u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
+ if (svm->nested.intercept_dr & bit)
vmexit = NESTED_EXIT_DONE;
break;
}
@@ -1975,8 +1985,7 @@ static inline void copy_vmcb_control_area(struct vmcb *dst_vmcb, struct vmcb *fr
struct vmcb_control_area *from = &from_vmcb->control;

dst->intercept_cr = from->intercept_cr;
- dst->intercept_dr_read = from->intercept_dr_read;
- dst->intercept_dr_write = from->intercept_dr_write;
+ dst->intercept_dr = from->intercept_dr;
dst->intercept_exceptions = from->intercept_exceptions;
dst->intercept = from->intercept;
dst->iopm_base_pa = from->iopm_base_pa;
@@ -2278,8 +2287,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)

/* cache intercepts */
svm->nested.intercept_cr = nested_vmcb->control.intercept_cr;
- svm->nested.intercept_dr_read = nested_vmcb->control.intercept_dr_read;
- svm->nested.intercept_dr_write = nested_vmcb->control.intercept_dr_write;
+ svm->nested.intercept_dr = nested_vmcb->control.intercept_dr;
svm->nested.intercept_exceptions = nested_vmcb->control.intercept_exceptions;
svm->nested.intercept = nested_vmcb->control.intercept;

@@ -2904,8 +2912,8 @@ void dump_vmcb(struct kvm_vcpu *vcpu)
pr_err("VMCB Control Area:\n");
pr_err("cr_read: %04x\n", control->intercept_cr & 0xffff);
pr_err("cr_write: %04x\n", control->intercept_cr >> 16);
- pr_err("dr_read: %04x\n", control->intercept_dr_read);
- pr_err("dr_write: %04x\n", control->intercept_dr_write);
+ pr_err("dr_read: %04x\n", control->intercept_dr & 0xffff);
+ pr_err("dr_write: %04x\n", control->intercept_dr >> 16);
pr_err("exceptions: %08x\n", control->intercept_exceptions);
pr_err("intercepts: %016llx\n", control->intercept);
pr_err("pause filter count: %d\n", control->pause_filter_count);
--
1.7.1

2010-11-30 17:05:39

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 5/6] KVM: SVM: Add manipulation functions for misc intercepts

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 <[email protected]>
---
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) |
- (1ULL << INTERCEPT_MONITOR) |
- (1ULL << INTERCEPT_MWAIT);
+ set_intercept(svm, INTERCEPT_INTR);
+ set_intercept(svm, INTERCEPT_NMI);
+ set_intercept(svm, INTERCEPT_SMI);
+ set_intercept(svm, INTERCEPT_SELECTIVE_CR0);
+ set_intercept(svm, INTERCEPT_CPUID);
+ set_intercept(svm, INTERCEPT_INVD);
+ set_intercept(svm, INTERCEPT_HLT);
+ set_intercept(svm, INTERCEPT_INVLPG);
+ set_intercept(svm, INTERCEPT_INVLPGA);
+ set_intercept(svm, INTERCEPT_IOIO_PROT);
+ set_intercept(svm, INTERCEPT_MSR_PROT);
+ set_intercept(svm, INTERCEPT_TASK_SWITCH);
+ set_intercept(svm, INTERCEPT_SHUTDOWN);
+ set_intercept(svm, INTERCEPT_VMRUN);
+ set_intercept(svm, INTERCEPT_VMMCALL);
+ set_intercept(svm, INTERCEPT_VMLOAD);
+ set_intercept(svm, INTERCEPT_VMSAVE);
+ set_intercept(svm, INTERCEPT_STGI);
+ set_intercept(svm, INTERCEPT_CLGI);
+ set_intercept(svm, INTERCEPT_SKINIT);
+ set_intercept(svm, INTERCEPT_WBINVD);
+ set_intercept(svm, INTERCEPT_MONITOR);
+ set_intercept(svm, INTERCEPT_MWAIT);

control->iopm_base_pa = iopm_base;
control->msrpm_base_pa = __pa(svm->msrpm);
@@ -935,8 +953,8 @@ static void init_vmcb(struct vcpu_svm *svm)
if (npt_enabled) {
/* Setup VMCB for Nested Paging */
control->nested_ctl = 1;
- control->intercept &= ~((1ULL << INTERCEPT_TASK_SWITCH) |
- (1ULL << INTERCEPT_INVLPG));
+ clr_intercept(svm, INTERCEPT_TASK_SWITCH);
+ clr_intercept(svm, INTERCEPT_INVLPG);
clr_exception_intercept(svm, PF_VECTOR);
clr_cr_intercept(svm, INTERCEPT_CR3_READ);
clr_cr_intercept(svm, INTERCEPT_CR3_WRITE);
@@ -951,7 +969,7 @@ static void init_vmcb(struct vcpu_svm *svm)

if (boot_cpu_has(X86_FEATURE_PAUSEFILTER)) {
control->pause_filter_count = 3000;
- control->intercept |= (1ULL << INTERCEPT_PAUSE);
+ set_intercept(svm, INTERCEPT_PAUSE);
}

enable_gif(svm);
@@ -1125,12 +1143,12 @@ static void svm_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg)

static void svm_set_vintr(struct vcpu_svm *svm)
{
- svm->vmcb->control.intercept |= 1ULL << INTERCEPT_VINTR;
+ set_intercept(svm, INTERCEPT_VINTR);
}

static void svm_clear_vintr(struct vcpu_svm *svm)
{
- svm->vmcb->control.intercept &= ~(1ULL << INTERCEPT_VINTR);
+ clr_intercept(svm, INTERCEPT_VINTR);
}

static struct vmcb_seg *svm_seg(struct kvm_vcpu *vcpu, int seg)
@@ -2307,7 +2325,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
}

/* We don't want to see VMMCALLs from a nested guest */
- svm->vmcb->control.intercept &= ~(1ULL << INTERCEPT_VMMCALL);
+ clr_intercept(svm, INTERCEPT_VMMCALL);

svm->vmcb->control.lbr_ctl = nested_vmcb->control.lbr_ctl;
svm->vmcb->control.int_vector = nested_vmcb->control.int_vector;
@@ -2555,7 +2573,7 @@ static int cpuid_interception(struct vcpu_svm *svm)
static int iret_interception(struct vcpu_svm *svm)
{
++svm->vcpu.stat.nmi_window_exits;
- svm->vmcb->control.intercept &= ~(1ULL << INTERCEPT_IRET);
+ clr_intercept(svm, INTERCEPT_IRET);
svm->vcpu.arch.hflags |= HF_IRET_MASK;
return 1;
}
@@ -3101,7 +3119,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu)

svm->vmcb->control.event_inj = SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_NMI;
vcpu->arch.hflags |= HF_NMI_MASK;
- svm->vmcb->control.intercept |= (1ULL << INTERCEPT_IRET);
+ set_intercept(svm, INTERCEPT_IRET);
++vcpu->stat.nmi_injections;
}

@@ -3168,10 +3186,10 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)

if (masked) {
svm->vcpu.arch.hflags |= HF_NMI_MASK;
- svm->vmcb->control.intercept |= (1ULL << INTERCEPT_IRET);
+ set_intercept(svm, INTERCEPT_IRET);
} else {
svm->vcpu.arch.hflags &= ~HF_NMI_MASK;
- svm->vmcb->control.intercept &= ~(1ULL << INTERCEPT_IRET);
+ clr_intercept(svm, INTERCEPT_IRET);
}
}

--
1.7.1

2010-11-30 17:42:34

by Avi Kivity

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

On 11/30/2010 07:03 PM, Joerg Roedel wrote:
> 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).
>

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

2010-12-01 10:34:22

by Joerg Roedel

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

On Tue, Nov 30, 2010 at 12:42:28PM -0500, Avi Kivity wrote:
> On 11/30/2010 07:03 PM, Joerg Roedel wrote:
> > 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).
> >
>
> 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.

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

2010-12-02 16:46:27

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: [PATCH 2/6] KVM: SVM: Add manipulation functions for CRx intercepts

On Tue, Nov 30, 2010 at 06:03:57PM +0100, Joerg Roedel wrote:
> This patch wraps changes to the CRx 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 <[email protected]>
> ---
> arch/x86/include/asm/svm.h | 15 +++--
> arch/x86/kvm/svm.c | 119 +++++++++++++++++++++++---------------------
> 2 files changed, 72 insertions(+), 62 deletions(-)
>

> - control->intercept_cr_read = INTERCEPT_CR0_MASK |
> - INTERCEPT_CR3_MASK |
> - INTERCEPT_CR4_MASK;
> -
> - control->intercept_cr_write = INTERCEPT_CR0_MASK |
> - INTERCEPT_CR3_MASK |
> - INTERCEPT_CR4_MASK |
> - INTERCEPT_CR8_MASK;
> + set_cr_intercept(svm, INTERCEPT_CR0_READ);
> + set_cr_intercept(svm, INTERCEPT_CR3_READ);
> + set_cr_intercept(svm, INTERCEPT_CR4_READ);
> + set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
> + set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
> + set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
> + set_cr_intercept(svm, INTERCEPT_CR8_WRITE);

Should clear hflags before using is_guest_mode().

2010-12-03 09:51:03

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 2/6] KVM: SVM: Add manipulation functions for CRx intercepts

On Thu, Dec 02, 2010 at 11:43:50AM -0500, Marcelo Tosatti wrote:
> On Tue, Nov 30, 2010 at 06:03:57PM +0100, Joerg Roedel wrote:
> > - control->intercept_cr_read = INTERCEPT_CR0_MASK |
> > - INTERCEPT_CR3_MASK |
> > - INTERCEPT_CR4_MASK;
> > -
> > - control->intercept_cr_write = INTERCEPT_CR0_MASK |
> > - INTERCEPT_CR3_MASK |
> > - INTERCEPT_CR4_MASK |
> > - INTERCEPT_CR8_MASK;
> > + set_cr_intercept(svm, INTERCEPT_CR0_READ);
> > + set_cr_intercept(svm, INTERCEPT_CR3_READ);
> > + set_cr_intercept(svm, INTERCEPT_CR4_READ);
> > + set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
> > + set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
> > + set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
> > + set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
>
> Should clear hflags before using is_guest_mode().

Right, thanks. Here is the updated patch.

>From 78e9a425d1440fdf9232e38dc3093127b96f26bb Mon Sep 17 00:00:00 2001
From: Joerg Roedel <[email protected]>
Date: Tue, 30 Nov 2010 15:30:21 +0100
Subject: [PATCH 2/6] KVM: SVM: Add manipulation functions for CRx intercepts

This patch wraps changes to the CRx 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 <[email protected]>
---
arch/x86/include/asm/svm.h | 15 +++--
arch/x86/kvm/svm.c | 120 +++++++++++++++++++++++--------------------
2 files changed, 73 insertions(+), 62 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 0e83105..39f9ddf 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -51,8 +51,7 @@ enum {


struct __attribute__ ((__packed__)) vmcb_control_area {
- u16 intercept_cr_read;
- u16 intercept_cr_write;
+ u32 intercept_cr;
u16 intercept_dr_read;
u16 intercept_dr_write;
u32 intercept_exceptions;
@@ -204,10 +203,14 @@ struct __attribute__ ((__packed__)) vmcb {
#define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
#define SVM_SELECTOR_CODE_MASK (1 << 3)

-#define INTERCEPT_CR0_MASK 1
-#define INTERCEPT_CR3_MASK (1 << 3)
-#define INTERCEPT_CR4_MASK (1 << 4)
-#define INTERCEPT_CR8_MASK (1 << 8)
+#define INTERCEPT_CR0_READ 0
+#define INTERCEPT_CR3_READ 3
+#define INTERCEPT_CR4_READ 4
+#define INTERCEPT_CR8_READ 8
+#define INTERCEPT_CR0_WRITE (16 + 0)
+#define INTERCEPT_CR3_WRITE (16 + 3)
+#define INTERCEPT_CR4_WRITE (16 + 4)
+#define INTERCEPT_CR8_WRITE (16 + 8)

#define INTERCEPT_DR0_MASK 1
#define INTERCEPT_DR1_MASK (1 << 1)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 05fe851..57c597b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -98,8 +98,7 @@ struct nested_state {
unsigned long vmexit_rax;

/* cache for intercepts of the guest */
- u16 intercept_cr_read;
- u16 intercept_cr_write;
+ u32 intercept_cr;
u16 intercept_dr_read;
u16 intercept_dr_write;
u32 intercept_exceptions;
@@ -204,14 +203,46 @@ static void recalc_intercepts(struct vcpu_svm *svm)
h = &svm->nested.hsave->control;
g = &svm->nested;

- c->intercept_cr_read = h->intercept_cr_read | g->intercept_cr_read;
- c->intercept_cr_write = h->intercept_cr_write | g->intercept_cr_write;
+ c->intercept_cr = h->intercept_cr | g->intercept_cr;
c->intercept_dr_read = h->intercept_dr_read | g->intercept_dr_read;
c->intercept_dr_write = h->intercept_dr_write | g->intercept_dr_write;
c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
c->intercept = h->intercept | g->intercept;
}

+static inline struct vmcb *get_host_vmcb(struct vcpu_svm *svm)
+{
+ if (is_guest_mode(&svm->vcpu))
+ return svm->nested.hsave;
+ else
+ return svm->vmcb;
+}
+
+static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_cr |= (1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline void clr_cr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_cr &= ~(1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline bool is_cr_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ return vmcb->control.intercept_cr & (1U << bit);
+}
+
static inline void enable_gif(struct vcpu_svm *svm)
{
svm->vcpu.arch.hflags |= HF_GIF_MASK;
@@ -766,15 +797,15 @@ static void init_vmcb(struct vcpu_svm *svm)
struct vmcb_save_area *save = &svm->vmcb->save;

svm->vcpu.fpu_active = 1;
+ svm->vcpu.arch.hflags = 0;

- control->intercept_cr_read = INTERCEPT_CR0_MASK |
- INTERCEPT_CR3_MASK |
- INTERCEPT_CR4_MASK;
-
- control->intercept_cr_write = INTERCEPT_CR0_MASK |
- INTERCEPT_CR3_MASK |
- INTERCEPT_CR4_MASK |
- INTERCEPT_CR8_MASK;
+ set_cr_intercept(svm, INTERCEPT_CR0_READ);
+ set_cr_intercept(svm, INTERCEPT_CR3_READ);
+ set_cr_intercept(svm, INTERCEPT_CR4_READ);
+ set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
+ set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
+ set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
+ set_cr_intercept(svm, INTERCEPT_CR8_WRITE);

control->intercept_dr_read = INTERCEPT_DR0_MASK |
INTERCEPT_DR1_MASK |
@@ -875,8 +906,8 @@ static void init_vmcb(struct vcpu_svm *svm)
control->intercept &= ~((1ULL << INTERCEPT_TASK_SWITCH) |
(1ULL << INTERCEPT_INVLPG));
control->intercept_exceptions &= ~(1 << PF_VECTOR);
- control->intercept_cr_read &= ~INTERCEPT_CR3_MASK;
- control->intercept_cr_write &= ~INTERCEPT_CR3_MASK;
+ clr_cr_intercept(svm, INTERCEPT_CR3_READ);
+ clr_cr_intercept(svm, INTERCEPT_CR3_WRITE);
save->g_pat = 0x0007040600070406ULL;
save->cr3 = 0;
save->cr4 = 0;
@@ -1210,7 +1241,6 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)

static void update_cr0_intercept(struct vcpu_svm *svm)
{
- struct vmcb *vmcb = svm->vmcb;
ulong gcr0 = svm->vcpu.arch.cr0;
u64 *hcr0 = &svm->vmcb->save.cr0;

@@ -1222,25 +1252,11 @@ static void update_cr0_intercept(struct vcpu_svm *svm)


if (gcr0 == *hcr0 && svm->vcpu.fpu_active) {
- vmcb->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK;
- vmcb->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
- if (is_guest_mode(&svm->vcpu)) {
- struct vmcb *hsave = svm->nested.hsave;
-
- hsave->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK;
- hsave->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
- vmcb->control.intercept_cr_read |= svm->nested.intercept_cr_read;
- vmcb->control.intercept_cr_write |= svm->nested.intercept_cr_write;
- }
+ clr_cr_intercept(svm, INTERCEPT_CR0_READ);
+ clr_cr_intercept(svm, INTERCEPT_CR0_WRITE);
} else {
- svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
- svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
- if (is_guest_mode(&svm->vcpu)) {
- struct vmcb *hsave = svm->nested.hsave;
-
- hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
- hsave->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
- }
+ set_cr_intercept(svm, INTERCEPT_CR0_READ);
+ set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
}
}

@@ -1900,15 +1916,9 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
case SVM_EXIT_IOIO:
vmexit = nested_svm_intercept_ioio(svm);
break;
- case SVM_EXIT_READ_CR0 ... SVM_EXIT_READ_CR8: {
- u32 cr_bits = 1 << (exit_code - SVM_EXIT_READ_CR0);
- if (svm->nested.intercept_cr_read & cr_bits)
- vmexit = NESTED_EXIT_DONE;
- break;
- }
- case SVM_EXIT_WRITE_CR0 ... SVM_EXIT_WRITE_CR8: {
- u32 cr_bits = 1 << (exit_code - SVM_EXIT_WRITE_CR0);
- if (svm->nested.intercept_cr_write & cr_bits)
+ case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8: {
+ u32 bit = 1U << (exit_code - SVM_EXIT_READ_CR0);
+ if (svm->nested.intercept_cr & bit)
vmexit = NESTED_EXIT_DONE;
break;
}
@@ -1965,8 +1975,7 @@ static inline void copy_vmcb_control_area(struct vmcb *dst_vmcb, struct vmcb *fr
struct vmcb_control_area *dst = &dst_vmcb->control;
struct vmcb_control_area *from = &from_vmcb->control;

- dst->intercept_cr_read = from->intercept_cr_read;
- dst->intercept_cr_write = from->intercept_cr_write;
+ dst->intercept_cr = from->intercept_cr;
dst->intercept_dr_read = from->intercept_dr_read;
dst->intercept_dr_write = from->intercept_dr_write;
dst->intercept_exceptions = from->intercept_exceptions;
@@ -2188,8 +2197,8 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
nested_vmcb->control.event_inj,
nested_vmcb->control.nested_ctl);

- trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr_read,
- nested_vmcb->control.intercept_cr_write,
+ trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr & 0xffff,
+ nested_vmcb->control.intercept_cr >> 16,
nested_vmcb->control.intercept_exceptions,
nested_vmcb->control.intercept);

@@ -2269,8 +2278,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
svm->nested.vmcb_iopm = nested_vmcb->control.iopm_base_pa & ~0x0fffULL;

/* cache intercepts */
- svm->nested.intercept_cr_read = nested_vmcb->control.intercept_cr_read;
- svm->nested.intercept_cr_write = nested_vmcb->control.intercept_cr_write;
+ svm->nested.intercept_cr = nested_vmcb->control.intercept_cr;
svm->nested.intercept_dr_read = nested_vmcb->control.intercept_dr_read;
svm->nested.intercept_dr_write = nested_vmcb->control.intercept_dr_write;
svm->nested.intercept_exceptions = nested_vmcb->control.intercept_exceptions;
@@ -2285,8 +2293,8 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)

if (svm->vcpu.arch.hflags & HF_VINTR_MASK) {
/* We only want the cr8 intercept bits of the guest */
- svm->vmcb->control.intercept_cr_read &= ~INTERCEPT_CR8_MASK;
- svm->vmcb->control.intercept_cr_write &= ~INTERCEPT_CR8_MASK;
+ clr_cr_intercept(svm, INTERCEPT_CR8_READ);
+ clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
}

/* We don't want to see VMMCALLs from a nested guest */
@@ -2578,7 +2586,7 @@ static int cr8_write_interception(struct vcpu_svm *svm)
/* instruction emulation calls kvm_set_cr8() */
emulate_instruction(&svm->vcpu, 0, 0, 0);
if (irqchip_in_kernel(svm->vcpu.kvm)) {
- svm->vmcb->control.intercept_cr_write &= ~INTERCEPT_CR8_MASK;
+ clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
return 1;
}
if (cr8_prev <= kvm_get_cr8(&svm->vcpu))
@@ -2895,8 +2903,8 @@ void dump_vmcb(struct kvm_vcpu *vcpu)
struct vmcb_save_area *save = &svm->vmcb->save;

pr_err("VMCB Control Area:\n");
- pr_err("cr_read: %04x\n", control->intercept_cr_read);
- pr_err("cr_write: %04x\n", control->intercept_cr_write);
+ pr_err("cr_read: %04x\n", control->intercept_cr & 0xffff);
+ pr_err("cr_write: %04x\n", control->intercept_cr >> 16);
pr_err("dr_read: %04x\n", control->intercept_dr_read);
pr_err("dr_write: %04x\n", control->intercept_dr_write);
pr_err("exceptions: %08x\n", control->intercept_exceptions);
@@ -2997,7 +3005,7 @@ static int handle_exit(struct kvm_vcpu *vcpu)

trace_kvm_exit(exit_code, vcpu, KVM_ISA_SVM);

- if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR0_MASK))
+ if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE))
vcpu->arch.cr0 = svm->vmcb->save.cr0;
if (npt_enabled)
vcpu->arch.cr3 = svm->vmcb->save.cr3;
@@ -3123,7 +3131,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
return;

if (tpr >= irr)
- svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR8_MASK;
+ set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
}

static int svm_nmi_allowed(struct kvm_vcpu *vcpu)
@@ -3230,7 +3238,7 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
return;

- if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR8_MASK)) {
+ if (!is_cr_intercept(svm, INTERCEPT_CR8_WRITE)) {
int cr8 = svm->vmcb->control.int_ctl & V_TPR_MASK;
kvm_set_cr8(vcpu, cr8);
}
--
1.7.1


--
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

2010-12-03 20:24:29

by Marcelo Tosatti

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

On Tue, Nov 30, 2010 at 06:03:55PM +0100, Joerg Roedel wrote:
> 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

Applied, thanks.