2010-12-03 10:48:52

by Joerg Roedel

Subject: [PATCH 0/12] KVM: SVM: Add support for VMCB state caching

Hi Avi, Hi Marcelo,

here is a patch-set which adds support for VMCB state caching to KVM.
This is a new CPU feature where software can mark certain parts of the
VMCB as unchanged since the last vmexit and the hardware can then avoid
reloading these parts from memory.

The feature is implemented downwards-compatible in hardware, so a 0-bit
means the state has changed and needs to be reloaded. This makes it
possible to implement the bits without checking for the feature, as done
in this patch-set (another reason is that the check is as expensive as
clearing the bit). Processors which do not implement VMCB state
caching just ignore these bits.

These patches were tested with multiple guests (Windows, Linux, also in
parallel) and also with nested-svm.

The patches apply on-top of the intercept mask wrapping patch-set I sent
earlier this week. Your feedback is appreciated.

Regards,
Joerg

arch/x86/include/asm/svm.h | 6 +++-
arch/x86/kvm/svm.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+), 1 deletions(-)

Joerg Roedel (12):
KVM: SVM: Add clean-bits infrastructure code
KVM: SVM: Add clean-bit for intercetps, tsc-offset and pause filter count
KVM: SVM: Add clean-bit for IOPM_BASE and MSRPM_BASE
KVM: SVM: Add clean-bit for the ASID
KVM: SVM: Add clean-bit for interrupt state
KVM: SVM: Add clean-bit for NPT state
KVM: SVM: Add clean-bit for control registers
KVM: SVM: Add clean-bit for DR6 and DR7
KVM: SVM: Add clean-bit for GDT and IDT
KVM: SVM: Add clean-bit for Segements and CPL
KVM: SVM: Add clean-bit for CR2 register
KVM: SVM: Add clean-bit for LBR state

2010-12-03 10:46:27

by Joerg Roedel

Subject: [PATCH 03/12] KVM: SVM: Add clean-bit for IOPM_BASE and MSRPM_BASE

This patch adds the clean bit for the physical addresses of
the MSRPM and the IOPM. It does not need to be set in the
code because the only place where these values are changed
is the nested-svm vmrun and vmexit path. These functions
already mark the complete VMCB as dirty.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d8058c9..3ab42be 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -188,6 +188,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
enum {
VMCB_INTERCEPTS, /* Intercept vectors, TSC offset,
pause filter count */
+ VMCB_PERM_MAP, /* IOPM Base and MSRPM Base */
VMCB_DIRTY_MAX,
};

--
1.7.1

2010-12-03 10:46:29

by Joerg Roedel

Subject: [PATCH 05/12] KVM: SVM: Add clean-bit for interrupt state

This patch implements the clean-bit for all interrupt
related state in the vmcb. This corresponds to vmcb offset
0x60-0x67.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8675048..b81d31a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -190,10 +190,12 @@ enum {
pause filter count */
VMCB_PERM_MAP, /* IOPM Base and MSRPM Base */
VMCB_ASID, /* ASID */
+ VMCB_INTR, /* int_ctl, int_vector */
VMCB_DIRTY_MAX,
};

-#define VMCB_ALWAYS_DIRTY_MASK 0U
+/* TPR is always written before VMRUN */
+#define VMCB_ALWAYS_DIRTY_MASK (1U << VMCB_INTR)

static inline void mark_all_dirty(struct vmcb *vmcb)
{
@@ -2507,6 +2509,8 @@ static int clgi_interception(struct vcpu_svm *svm)
svm_clear_vintr(svm);
svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;

+ mark_dirty(svm->vmcb, VMCB_INTR);
+
return 1;
}

@@ -2877,6 +2881,7 @@ static int interrupt_window_interception(struct vcpu_svm *svm)
kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
svm_clear_vintr(svm);
svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
+ mark_dirty(svm->vmcb, VMCB_INTR);
/*
* If the user space waits to inject interrupts, exit as soon as
* possible
@@ -3168,6 +3173,7 @@ static inline void svm_inject_irq(struct vcpu_svm *svm, int irq)
control->int_ctl &= ~V_INTR_PRIO_MASK;
control->int_ctl |= V_IRQ_MASK |
((/*control->int_vector >> 4*/ 0xf) << V_INTR_PRIO_SHIFT);
+ mark_dirty(svm->vmcb, VMCB_INTR);
}

static void svm_set_irq(struct kvm_vcpu *vcpu)
--
1.7.1

2010-12-03 10:46:57

by Joerg Roedel

Subject: [PATCH 11/12] KVM: SVM: Add clean-bit for CR2 register

This patch implements the clean-bit for the cr2 register in
the vmcb.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 4c366fe..7643f83 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -196,11 +196,12 @@ enum {
VMCB_DR, /* DR6, DR7 */
VMCB_DT, /* GDT, IDT */
VMCB_SEG, /* CS, DS, SS, ES, CPL */
+ VMCB_CR2, /* CR2 only */
VMCB_DIRTY_MAX,
};

-/* TPR is always written before VMRUN */
-#define VMCB_ALWAYS_DIRTY_MASK (1U << VMCB_INTR)
+/* TPR and CR2 are always written before VMRUN */
+#define VMCB_ALWAYS_DIRTY_MASK ((1U << VMCB_INTR) | (1U << VMCB_CR2))

static inline void mark_all_dirty(struct vmcb *vmcb)
{
--
1.7.1

2010-12-03 10:46:28

by Joerg Roedel

Subject: [PATCH 12/12] KVM: SVM: Add clean-bit for LBR state

This patch implements the clean-bit for all LBR related
state. This includes the debugctl, br_from, br_to,
last_excp_from, and last_excp_to msrs.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 7643f83..8966669 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -197,6 +197,7 @@ enum {
VMCB_DT, /* GDT, IDT */
VMCB_SEG, /* CS, DS, SS, ES, CPL */
VMCB_CR2, /* CR2 only */
+ VMCB_LBR, /* DBGCTL, BR_FROM, BR_TO, LAST_EX_FROM, LAST_EX_TO */
VMCB_DIRTY_MAX,
};

@@ -2846,6 +2847,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data)
return 1;

svm->vmcb->save.dbgctl = data;
+ mark_dirty(svm->vmcb, VMCB_LBR);
if (data & (1ULL<<0))
svm_enable_lbrv(svm);
else
--
1.7.1

2010-12-03 10:47:14

by Joerg Roedel

Subject: [PATCH 07/12] KVM: SVM: Add clean-bit for control registers

This patch implements the CRx clean-bit for the vmcb. This
bit covers cr0, cr3, cr4, and efer.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3b5d894..1b35969 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -192,6 +192,7 @@ enum {
VMCB_ASID, /* ASID */
VMCB_INTR, /* int_ctl, int_vector */
VMCB_NPT, /* npt_en, nCR3, gPAT */
+ VMCB_CR, /* CR0, CR3, CR4, EFER */
VMCB_DIRTY_MAX,
};

@@ -441,6 +442,7 @@ static void svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
efer &= ~EFER_LME;

to_svm(vcpu)->vmcb->save.efer = efer | EFER_SVME;
+ mark_dirty(to_svm(vcpu)->vmcb, VMCB_CR);
}

static int is_external_interrupt(u32 info)
@@ -1338,6 +1340,7 @@ static void update_cr0_intercept(struct vcpu_svm *svm)
*hcr0 = (*hcr0 & ~SVM_CR0_SELECTIVE_MASK)
| (gcr0 & SVM_CR0_SELECTIVE_MASK);

+ mark_dirty(svm->vmcb, VMCB_CR);

if (gcr0 == *hcr0 && svm->vcpu.fpu_active) {
clr_cr_intercept(svm, INTERCEPT_CR0_READ);
@@ -1404,6 +1407,7 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
*/
cr0 &= ~(X86_CR0_CD | X86_CR0_NW);
svm->vmcb->save.cr0 = cr0;
+ mark_dirty(svm->vmcb, VMCB_CR);
update_cr0_intercept(svm);
}

@@ -1420,6 +1424,7 @@ static void svm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
cr4 |= X86_CR4_PAE;
cr4 |= host_cr4_mce;
to_svm(vcpu)->vmcb->save.cr4 = cr4;
+ mark_dirty(to_svm(vcpu)->vmcb, VMCB_CR);
}

static void svm_set_segment(struct kvm_vcpu *vcpu,
@@ -3546,6 +3551,7 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu, unsigned long root)
struct vcpu_svm *svm = to_svm(vcpu);

svm->vmcb->save.cr3 = root;
+ mark_dirty(svm->vmcb, VMCB_CR);
force_new_asid(vcpu);
}

@@ -3558,6 +3564,7 @@ static void set_tdp_cr3(struct kvm_vcpu *vcpu, unsigned long root)

/* Also sync guest cr3 here in case we live migrate */
svm->vmcb->save.cr3 = vcpu->arch.cr3;
+ mark_dirty(svm->vmcb, VMCB_CR);

force_new_asid(vcpu);
}
--
1.7.1

2010-12-03 10:47:35

by Joerg Roedel

Subject: [PATCH 06/12] KVM: SVM: Add clean-bit for NPT state

This patch implements the clean-bit for all nested paging
related state in the vmcb.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index b81d31a..3b5d894 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -191,6 +191,7 @@ enum {
VMCB_PERM_MAP, /* IOPM Base and MSRPM Base */
VMCB_ASID, /* ASID */
VMCB_INTR, /* int_ctl, int_vector */
+ VMCB_NPT, /* npt_en, nCR3, gPAT */
VMCB_DIRTY_MAX,
};

@@ -1749,6 +1750,7 @@ static void nested_svm_set_tdp_cr3(struct kvm_vcpu *vcpu,
struct vcpu_svm *svm = to_svm(vcpu);

svm->vmcb->control.nested_cr3 = root;
+ mark_dirty(svm->vmcb, VMCB_NPT);
force_new_asid(vcpu);
}

@@ -3552,6 +3554,7 @@ static void set_tdp_cr3(struct kvm_vcpu *vcpu, unsigned long root)
struct vcpu_svm *svm = to_svm(vcpu);

svm->vmcb->control.nested_cr3 = root;
+ mark_dirty(svm->vmcb, VMCB_NPT);

/* Also sync guest cr3 here in case we live migrate */
svm->vmcb->save.cr3 = vcpu->arch.cr3;
--
1.7.1

2010-12-03 10:47:36

by Joerg Roedel

Subject: [PATCH 09/12] KVM: SVM: Add clean-bit for GDT and IDT

This patch implements the clean-bit for the base and limit
of the gdt and idt in the vmcb.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 0517505..2236c9a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -194,6 +194,7 @@ enum {
VMCB_NPT, /* npt_en, nCR3, gPAT */
VMCB_CR, /* CR0, CR3, CR4, EFER */
VMCB_DR, /* DR6, DR7 */
+ VMCB_DT, /* GDT, IDT */
VMCB_DIRTY_MAX,
};

@@ -1304,6 +1305,7 @@ static void svm_set_idt(struct kvm_vcpu *vcpu, struct desc_ptr *dt)

svm->vmcb->save.idtr.limit = dt->size;
svm->vmcb->save.idtr.base = dt->address ;
+ mark_dirty(svm->vmcb, VMCB_DT);
}

static void svm_get_gdt(struct kvm_vcpu *vcpu, struct desc_ptr *dt)
@@ -1320,6 +1322,7 @@ static void svm_set_gdt(struct kvm_vcpu *vcpu, struct desc_ptr *dt)

svm->vmcb->save.gdtr.limit = dt->size;
svm->vmcb->save.gdtr.base = dt->address ;
+ mark_dirty(svm->vmcb, VMCB_DT);
}

static void svm_decache_cr0_guest_bits(struct kvm_vcpu *vcpu)
--
1.7.1

2010-12-03 10:46:23

by Joerg Roedel

Subject: [PATCH 02/12] KVM: SVM: Add clean-bit for intercetps, tsc-offset and pause filter count

This patch adds the clean-bit for intercepts-vectors, the
TSC offset and the pause-filter count to the appropriate
places. The IO and MSR permission bitmaps are not subject to
this bit.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3ee59b0..d8058c9 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -186,6 +186,8 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
bool has_error_code, u32 error_code);

enum {
+ VMCB_INTERCEPTS, /* Intercept vectors, TSC offset,
+ pause filter count */
VMCB_DIRTY_MAX,
};

@@ -217,6 +219,8 @@ static void recalc_intercepts(struct vcpu_svm *svm)
struct vmcb_control_area *c, *h;
struct nested_state *g;

+ mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
+
if (!is_guest_mode(&svm->vcpu))
return;

@@ -854,6 +858,8 @@ static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
}

svm->vmcb->control.tsc_offset = offset + g_tsc_offset;
+
+ mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
}

static void svm_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment)
@@ -863,6 +869,7 @@ static void svm_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment)
svm->vmcb->control.tsc_offset += adjustment;
if (is_guest_mode(vcpu))
svm->nested.hsave->control.tsc_offset += adjustment;
+ mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
}

static void init_vmcb(struct vcpu_svm *svm)
--
1.7.1

2010-12-03 10:47:33

by Joerg Roedel

Subject: [PATCH 10/12] KVM: SVM: Add clean-bit for Segements and CPL

This patch implements the clean-bit defined for the cs, ds,
ss, an es segemnts and the current cpl saved in the vmcb.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2236c9a..4c366fe 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -195,6 +195,7 @@ enum {
VMCB_CR, /* CR0, CR3, CR4, EFER */
VMCB_DR, /* DR6, DR7 */
VMCB_DT, /* GDT, IDT */
+ VMCB_SEG, /* CS, DS, SS, ES, CPL */
VMCB_DIRTY_MAX,
};

@@ -1457,6 +1458,7 @@ static void svm_set_segment(struct kvm_vcpu *vcpu,
= (svm->vmcb->save.cs.attrib
>> SVM_SELECTOR_DPL_SHIFT) & 3;

+ mark_dirty(svm->vmcb, VMCB_SEG);
}

static void update_db_intercept(struct kvm_vcpu *vcpu)
--
1.7.1

2010-12-03 10:48:28

by Joerg Roedel

Subject: [PATCH 08/12] KVM: SVM: Add clean-bit for DR6 and DR7

This patch implements the clean-bit for the dr6 and dr7
debug registers in the vmcb.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1b35969..0517505 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -193,6 +193,7 @@ enum {
VMCB_INTR, /* int_ctl, int_vector */
VMCB_NPT, /* npt_en, nCR3, gPAT */
VMCB_CR, /* CR0, CR3, CR4, EFER */
+ VMCB_DR, /* DR6, DR7 */
VMCB_DIRTY_MAX,
};

@@ -1484,6 +1485,8 @@ static void svm_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg)
else
svm->vmcb->save.dr7 = vcpu->arch.dr7;

+ mark_dirty(svm->vmcb, VMCB_DR);
+
update_db_intercept(vcpu);
}

@@ -1506,6 +1509,7 @@ static void svm_set_dr7(struct kvm_vcpu *vcpu, unsigned long value)
struct vcpu_svm *svm = to_svm(vcpu);

svm->vmcb->save.dr7 = value;
+ mark_dirty(svm->vmcb, VMCB_DR);
}

static int pf_interception(struct vcpu_svm *svm)
--
1.7.1

2010-12-03 10:46:25

by Joerg Roedel

Subject: [PATCH 04/12] KVM: SVM: Add clean-bit for the ASID

This patch implements the clean-bit for the asid in the
vmcb.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3ab42be..8675048 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -189,6 +189,7 @@ enum {
VMCB_INTERCEPTS, /* Intercept vectors, TSC offset,
pause filter count */
VMCB_PERM_MAP, /* IOPM Base and MSRPM Base */
+ VMCB_ASID, /* ASID */
VMCB_DIRTY_MAX,
};

@@ -1488,6 +1489,8 @@ static void new_asid(struct vcpu_svm *svm, struct svm_cpu_data *sd)

svm->asid_generation = sd->asid_generation;
svm->vmcb->control.asid = sd->next_asid++;
+
+ mark_dirty(svm->vmcb, VMCB_ASID);
}

static void svm_set_dr7(struct kvm_vcpu *vcpu, unsigned long value)
--
1.7.1

2010-12-03 10:49:05

by Joerg Roedel

Subject: [PATCH 01/12] KVM: SVM: Add clean-bits infrastructure code

This patch adds the infrastructure for the implementation of
the individual clean-bits.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/svm.h | 6 +++++-
arch/x86/kvm/svm.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 11dbca7..d786399 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -49,6 +49,9 @@ enum {
INTERCEPT_MWAIT_COND,
};

+enum {
+ VMCB_CLEAN_MAX,
+};

struct __attribute__ ((__packed__)) vmcb_control_area {
u32 intercept_cr;
@@ -79,7 +82,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
u32 event_inj_err;
u64 nested_cr3;
u64 lbr_ctl;
- u64 reserved_5;
+ u32 clean;
+ u32 reserved_5;
u64 next_rip;
u8 reserved_6[816];
};
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c00ea90..3ee59b0 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -185,6 +185,28 @@ static int nested_svm_vmexit(struct vcpu_svm *svm);
static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
bool has_error_code, u32 error_code);

+enum {
+ VMCB_DIRTY_MAX,
+};
+
+#define VMCB_ALWAYS_DIRTY_MASK 0U
+
+static inline void mark_all_dirty(struct vmcb *vmcb)
+{
+ vmcb->control.clean = 0;
+}
+
+static inline void mark_all_clean(struct vmcb *vmcb)
+{
+ vmcb->control.clean = ((1 << VMCB_DIRTY_MAX) - 1)
+ & ~VMCB_ALWAYS_DIRTY_MASK;
+}
+
+static inline void mark_dirty(struct vmcb *vmcb, int bit)
+{
+ vmcb->control.clean &= ~(1 << bit);
+}
+
static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
{
return container_of(vcpu, struct vcpu_svm, vcpu);
@@ -973,6 +995,8 @@ static void init_vmcb(struct vcpu_svm *svm)
set_intercept(svm, INTERCEPT_PAUSE);
}

+ mark_all_dirty(svm->vmcb);
+
enable_gif(svm);
}

@@ -1089,6 +1113,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)

if (unlikely(cpu != vcpu->cpu)) {
svm->asid_generation = 0;
+ mark_all_dirty(svm->vmcb);
}

#ifdef CONFIG_X86_64
@@ -2139,6 +2164,8 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
svm->vmcb->save.cpl = 0;
svm->vmcb->control.exit_int_info = 0;

+ mark_all_dirty(svm->vmcb);
+
nested_svm_unmap(page);

nested_svm_uninit_mmu_context(&svm->vcpu);
@@ -2350,6 +2377,8 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)

enable_gif(svm);

+ mark_all_dirty(svm->vmcb);
+
return true;
}

@@ -3487,6 +3516,8 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
if (unlikely(svm->vmcb->control.exit_code ==
SVM_EXIT_EXCP_BASE + MC_VECTOR))
svm_handle_mce(svm);
+
+ mark_all_clean(svm->vmcb);
}

#undef R
--
1.7.1

2010-12-03 12:15:28

by Joerg Roedel

Subject: Re: [PATCH 01/12] KVM: SVM: Add clean-bits infrastructure code

On Fri, Dec 03, 2010 at 05:45:48AM -0500, Joerg Roedel wrote:
> +enum {
> + VMCB_CLEAN_MAX,
> +};

This is a left-over from an earlier version. I forgot to remove it. Here
is an updated patch. Sorry.


>From 7e3f4f175561429d0054daac94763e67d12424ba Mon Sep 17 00:00:00 2001
From: Joerg Roedel <[email protected]>
Date: Wed, 1 Dec 2010 12:01:08 +0100
Subject: [PATCH 01/12] KVM: SVM: Add clean-bits infrastructure code

This patch adds the infrastructure for the implementation of
the individual clean-bits.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/svm.h | 3 ++-
arch/x86/kvm/svm.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 11dbca7..235dd73 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -79,7 +79,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
u32 event_inj_err;
u64 nested_cr3;
u64 lbr_ctl;
- u64 reserved_5;
+ u32 clean;
+ u32 reserved_5;
u64 next_rip;
u8 reserved_6[816];
};
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c00ea90..3ee59b0 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -185,6 +185,28 @@ static int nested_svm_vmexit(struct vcpu_svm *svm);
static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
bool has_error_code, u32 error_code);

+enum {
+ VMCB_DIRTY_MAX,
+};
+
+#define VMCB_ALWAYS_DIRTY_MASK 0U
+
+static inline void mark_all_dirty(struct vmcb *vmcb)
+{
+ vmcb->control.clean = 0;
+}
+
+static inline void mark_all_clean(struct vmcb *vmcb)
+{
+ vmcb->control.clean = ((1 << VMCB_DIRTY_MAX) - 1)
+ & ~VMCB_ALWAYS_DIRTY_MASK;
+}
+
+static inline void mark_dirty(struct vmcb *vmcb, int bit)
+{
+ vmcb->control.clean &= ~(1 << bit);
+}
+
static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
{
return container_of(vcpu, struct vcpu_svm, vcpu);
@@ -973,6 +995,8 @@ static void init_vmcb(struct vcpu_svm *svm)
set_intercept(svm, INTERCEPT_PAUSE);
}

+ mark_all_dirty(svm->vmcb);
+
enable_gif(svm);
}

@@ -1089,6 +1113,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)

if (unlikely(cpu != vcpu->cpu)) {
svm->asid_generation = 0;
+ mark_all_dirty(svm->vmcb);
}

#ifdef CONFIG_X86_64
@@ -2139,6 +2164,8 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
svm->vmcb->save.cpl = 0;
svm->vmcb->control.exit_int_info = 0;

+ mark_all_dirty(svm->vmcb);
+
nested_svm_unmap(page);

nested_svm_uninit_mmu_context(&svm->vcpu);
@@ -2350,6 +2377,8 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)

enable_gif(svm);

+ mark_all_dirty(svm->vmcb);
+
return true;
}

@@ -3487,6 +3516,8 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
if (unlikely(svm->vmcb->control.exit_code ==
SVM_EXIT_EXCP_BASE + MC_VECTOR))
svm_handle_mce(svm);
+
+ mark_all_clean(svm->vmcb);
}

#undef R
--
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-06 19:48:51

by Marcelo Tosatti

Subject: Re: [PATCH 05/12] KVM: SVM: Add clean-bit for interrupt state

On Fri, Dec 03, 2010 at 11:45:52AM +0100, Joerg Roedel wrote:
> This patch implements the clean-bit for all interrupt
> related state in the vmcb. This corresponds to vmcb offset
> 0x60-0x67.
>
> Signed-off-by: Joerg Roedel <[email protected]>

Don't you have to mark dirty on sync_lapic_to_cr8?

2010-12-06 19:50:09

by Marcelo Tosatti

Subject: Re: [PATCH 0/12] KVM: SVM: Add support for VMCB state caching

On Fri, Dec 03, 2010 at 11:45:47AM +0100, Joerg Roedel wrote:
> Hi Avi, Hi Marcelo,
>
> here is a patch-set which adds support for VMCB state caching to KVM.
> This is a new CPU feature where software can mark certain parts of the
> VMCB as unchanged since the last vmexit and the hardware can then avoid
> reloading these parts from memory.
>
> The feature is implemented downwards-compatible in hardware, so a 0-bit
> means the state has changed and needs to be reloaded. This makes it
> possible to implement the bits without checking for the feature, as done
> in this patch-set (another reason is that the check is as expensive as
> clearing the bit). Processors which do not implement VMCB state
> caching just ignore these bits.
>
> These patches were tested with multiple guests (Windows, Linux, also in
> parallel) and also with nested-svm.
>
> The patches apply on-top of the intercept mask wrapping patch-set I sent
> earlier this week. Your feedback is appreciated.
>
> Regards,
> Joerg
>
> arch/x86/include/asm/svm.h | 6 +++-
> arch/x86/kvm/svm.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 75 insertions(+), 1 deletions(-)
>
> Joerg Roedel (12):
> KVM: SVM: Add clean-bits infrastructure code
> KVM: SVM: Add clean-bit for intercetps, tsc-offset and pause filter count
> KVM: SVM: Add clean-bit for IOPM_BASE and MSRPM_BASE
> KVM: SVM: Add clean-bit for the ASID
> KVM: SVM: Add clean-bit for interrupt state
> KVM: SVM: Add clean-bit for NPT state
> KVM: SVM: Add clean-bit for control registers
> KVM: SVM: Add clean-bit for DR6 and DR7
> KVM: SVM: Add clean-bit for GDT and IDT
> KVM: SVM: Add clean-bit for Segements and CPL
> KVM: SVM: Add clean-bit for CR2 register
> KVM: SVM: Add clean-bit for LBR state

Wouldnt it be good to wrap assignment of field & dirty bit update
in wrappers, for long term maintainability?

Looks good to me, except comment on patch 5.

2010-12-07 08:23:53

by Joerg Roedel

Subject: Re: [PATCH 05/12] KVM: SVM: Add clean-bit for interrupt state

On Mon, Dec 06, 2010 at 05:29:24PM -0200, Marcelo Tosatti wrote:
> On Fri, Dec 03, 2010 at 11:45:52AM +0100, Joerg Roedel wrote:
> > This patch implements the clean-bit for all interrupt
> > related state in the vmcb. This corresponds to vmcb offset
> > 0x60-0x67.
> >
> > Signed-off-by: Joerg Roedel <[email protected]>
>
> Don't you have to mark dirty on sync_lapic_to_cr8?

In theory yes, but sync_lapic_to_cr8 is called in every vmexit/vmrun
cycle. Thats why I added the interrupt bit to the always-dirty mask. I
left this as a future optimization to only write cr8 to the vmcb when it
actually changes.

Joerg

2010-12-07 08:26:23

by Joerg Roedel

Subject: Re: [PATCH 0/12] KVM: SVM: Add support for VMCB state caching

On Mon, Dec 06, 2010 at 05:49:55PM -0200, Marcelo Tosatti wrote:
> On Fri, Dec 03, 2010 at 11:45:47AM +0100, Joerg Roedel wrote:
> > Hi Avi, Hi Marcelo,
> >
> > here is a patch-set which adds support for VMCB state caching to KVM.
> > This is a new CPU feature where software can mark certain parts of the
> > VMCB as unchanged since the last vmexit and the hardware can then avoid
> > reloading these parts from memory.
> >
> > The feature is implemented downwards-compatible in hardware, so a 0-bit
> > means the state has changed and needs to be reloaded. This makes it
> > possible to implement the bits without checking for the feature, as done
> > in this patch-set (another reason is that the check is as expensive as
> > clearing the bit). Processors which do not implement VMCB state
> > caching just ignore these bits.
> >
> > These patches were tested with multiple guests (Windows, Linux, also in
> > parallel) and also with nested-svm.
> >
> > The patches apply on-top of the intercept mask wrapping patch-set I sent
> > earlier this week. Your feedback is appreciated.
> >
> > Regards,
> > Joerg
> >
> > arch/x86/include/asm/svm.h | 6 +++-
> > arch/x86/kvm/svm.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 75 insertions(+), 1 deletions(-)
> >
> > Joerg Roedel (12):
> > KVM: SVM: Add clean-bits infrastructure code
> > KVM: SVM: Add clean-bit for intercetps, tsc-offset and pause filter count
> > KVM: SVM: Add clean-bit for IOPM_BASE and MSRPM_BASE
> > KVM: SVM: Add clean-bit for the ASID
> > KVM: SVM: Add clean-bit for interrupt state
> > KVM: SVM: Add clean-bit for NPT state
> > KVM: SVM: Add clean-bit for control registers
> > KVM: SVM: Add clean-bit for DR6 and DR7
> > KVM: SVM: Add clean-bit for GDT and IDT
> > KVM: SVM: Add clean-bit for Segements and CPL
> > KVM: SVM: Add clean-bit for CR2 register
> > KVM: SVM: Add clean-bit for LBR state
>
> Wouldnt it be good to wrap assignment of field & dirty bit update
> in wrappers, for long term maintainability?

Right. I did that for the intercept masks already. Most other stuff is
also already wrapped. But there is certainly still room for improvement.

Joerg

2010-12-07 09:50:19

by Avi Kivity

Subject: Re: [PATCH 0/12] KVM: SVM: Add support for VMCB state caching

On 12/03/2010 12:45 PM, Joerg Roedel wrote:
> Hi Avi, Hi Marcelo,
>
> here is a patch-set which adds support for VMCB state caching to KVM.
> This is a new CPU feature where software can mark certain parts of the
> VMCB as unchanged since the last vmexit and the hardware can then avoid
> reloading these parts from memory.
>
> The feature is implemented downwards-compatible in hardware, so a 0-bit
> means the state has changed and needs to be reloaded. This makes it
> possible to implement the bits without checking for the feature, as done
> in this patch-set (another reason is that the check is as expensive as
> clearing the bit). Processors which do not implement VMCB state
> caching just ignore these bits.
>
> These patches were tested with multiple guests (Windows, Linux, also in
> parallel) and also with nested-svm.
>
> The patches apply on-top of the intercept mask wrapping patch-set I sent
> earlier this week. Your feedback is appreciated.
>

Applied, thanks.

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