Skip to contentSkip to conversation

KVM: add basic paravirt support

PATCH New Marcelo Tosatti 6 participants 21 messages Feb 16, 2008 – Feb 19, 2008 5 messages not in the archive

Subsystems (4)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/5 KVM: add basic paravirt support

Add basic KVM paravirt support. Avoid vm-exits on IO delays.

Add KVM_GET_PARA_FEATURES ioctl so paravirt features can be reported via cpuid.

arch/x86/Kconfig +8 -0
Index: kvm.paravirt/arch/x86/Kconfig
===================================================================
@@ -372,6 +372,14 @@ config VMI
 	  at the moment), by linking the kernel to a GPL-ed ROM module
 	  provided by the hypervisor.
 
+config KVM_GUEST
+ 	bool "KVM Guest support"
+ 	select PARAVIRT
+ 	depends on !(X86_VISWS || X86_VOYAGER)
+ 	help
+ 	 This option enables various optimizations for running under the KVM
+ 	 hypervisor.
+
 source "arch/x86/lguest/Kconfig"
 
 config PARAVIRT
arch/x86/kernel/Makefile +1 -0
Index: kvm.paravirt/arch/x86/kernel/Makefile
===================================================================
@@ -69,6 +69,7 @@ obj-$(CONFIG_DEBUG_RODATA_TEST)	+= test_
 obj-$(CONFIG_DEBUG_NX_TEST)	+= test_nx.o
 
 obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
+obj-$(CONFIG_KVM_GUEST)		+= kvm.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o paravirt_patch_$(BITS).o
 
 ifdef CONFIG_INPUT_PCSPKR
arch/x86/kernel/kvm.c new file +52 -0

52 lines not on this page · raw message

arch/x86/kernel/setup_32.c +2 -0
Index: kvm.paravirt/arch/x86/kernel/setup_32.c
===================================================================
@@ -46,6 +46,7 @@
 #include <linux/pfn.h>
 #include <linux/pci.h>
 #include <linux/init_ohci1394_dma.h>
+#include <linux/kvm_para.h>
 
 #include <video/edid.h>
 
@@ -779,6 +780,7 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	vmi_init();
 #endif
+	kvm_guest_init();
 
 	/*
 	 * NOTE: before this point _nobody_ is allowed to allocate
arch/x86/kernel/setup_64.c +3 -0
Index: kvm.paravirt/arch/x86/kernel/setup_64.c
===================================================================
@@ -41,6 +41,7 @@
 #include <linux/ctype.h>
 #include <linux/uaccess.h>
 #include <linux/init_ohci1394_dma.h>
+#include <linux/kvm_para.h>
 
 #include <asm/mtrr.h>
 #include <asm/uaccess.h>
@@ -447,6 +448,8 @@ void __init setup_arch(char **cmdline_p)
 	init_apic_mappings();
 	ioapic_init_mappings();
 
+	kvm_guest_init();
+
 	/*
 	 * We trust e820 completely. No explicit ROM probing in memory.
 	 */
arch/x86/kvm/x86.c +10 -0
Index: kvm.paravirt/arch/x86/kvm/x86.c
===================================================================
@@ -696,6 +696,7 @@ int kvm_dev_ioctl_check_extension(long e
 	case KVM_CAP_USER_MEMORY:
 	case KVM_CAP_SET_TSS_ADDR:
 	case KVM_CAP_EXT_CPUID:
+	case KVM_CAP_PARA_FEATURES:
 		r = 1;
 		break;
 	case KVM_CAP_VAPIC:
@@ -761,6 +762,15 @@ long kvm_arch_dev_ioctl(struct file *fil
 		r = 0;
 		break;
 	}
+	case KVM_GET_PARA_FEATURES: {
+		__u32 para_features = KVM_PARA_FEATURES;
+
+		r = -EFAULT;
+		if (copy_to_user(argp, &para_features, sizeof para_features))
+			goto out;
+		r = 0;
+		break;
+	}
 	default:
 		r = -EINVAL;
 	}
include/asm-x86/kvm_para.h +3 -0
Index: kvm.paravirt/include/asm-x86/kvm_para.h
===================================================================
@@ -5,6 +5,7 @@
  * should be used to determine that a VM is running under KVM.
  */
 #define KVM_CPUID_SIGNATURE	0x40000000
+#define KVM_FEATURE_NOP_IO_DELAY	0
 
 /* This CPUID returns a feature bitmap in eax.  Before enabling a particular
  * paravirtualization, the appropriate feature bit should be checked.
@@ -14,6 +15,8 @@
 #ifdef __KERNEL__
 #include <asm/processor.h>
 
+#define KVM_PARA_FEATURES (1UL << KVM_FEATURE_NOP_IO_DELAY)
+
 /* This instruction is vmcall.  On non-VT architectures, it will generate a
  * trap that we will then rewrite to the appropriate instruction.
  */
include/linux/kvm.h +2 -0
Index: kvm.paravirt/include/linux/kvm.h
===================================================================
@@ -222,6 +222,7 @@ struct kvm_vapic_addr {
  */
 #define KVM_GET_VCPU_MMAP_SIZE    _IO(KVMIO,   0x04) /* in bytes */
 #define KVM_GET_SUPPORTED_CPUID   _IOWR(KVMIO, 0x05, struct kvm_cpuid2)
+#define KVM_GET_PARA_FEATURES     _IOR(KVMIO,  0x05, __u32)
 
 /*
  * Extension capability list.
@@ -233,6 +234,7 @@ struct kvm_vapic_addr {
 #define KVM_CAP_SET_TSS_ADDR 4
 #define KVM_CAP_VAPIC 6
 #define KVM_CAP_EXT_CPUID 7
+#define KVM_CAP_PARA_FEATURES 8
 
 /*
  * ioctls for VM fds
include/linux/kvm_para.h +6 -0
Index: kvm.paravirt/include/linux/kvm_para.h
===================================================================
@@ -20,6 +20,12 @@
 #include <asm/kvm_para.h>
 
 #ifdef __KERNEL__
+#ifdef CONFIG_KVM_GUEST
+void __init kvm_guest_init(void);
+#else
+#define kvm_guest_init() do { } while (0)
+#endif
+
 static inline int kvm_para_has_feature(unsigned int feature)
 {
 	if (kvm_arch_para_features() & (1UL << feature))
Signature · 4 lines
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#2 lore raw

PATCH 2/5 KVM: hypercall based pte updates and TLB flushes

In reply to a message not in the archive

Hypercall based pte updates are faster than faults, and also allow use of the lazy MMU mode to batch operations.

Don't report the feature if two dimensional paging is enabled.

arch/x86/kernel/kvm.c +103 -0
Index: kvm.paravirt/arch/x86/kernel/kvm.c
===================================================================
@@ -33,6 +33,91 @@ static void kvm_io_delay(void)
 {
 }
 
+static void kvm_mmu_write(void *dest, const void *src, size_t size)
+{
+	const uint8_t *p = src;
+	unsigned long a0 = *(unsigned long *)p;
+	unsigned long a1 = 0;
+
+	size >>= 2;
+#ifdef CONFIG_X86_32
+	if (size == 2)
+		a1 = *(u32 *)&p[4];
+#endif
+	kvm_hypercall4(KVM_HYPERCALL_MMU_WRITE, (unsigned long)dest, size, a0,
+		       a1);
+}
+
+/*
+ * We only need to hook operations that are MMU writes.  We hook these so that
+ * we can use lazy MMU mode to batch these operations.  We could probably
+ * improve the performance of the host code if we used some of the information
+ * here to simplify processing of batched writes.
+ */
+static void kvm_set_pte(pte_t *ptep, pte_t pte)
+{
+	kvm_mmu_write(ptep, &pte, sizeof(pte));
+}
+
+static void kvm_set_pte_at(struct mm_struct *mm, unsigned long addr,
+			   pte_t *ptep, pte_t pte)
+{
+	kvm_mmu_write(ptep, &pte, sizeof(pte));
+}
+
+static void kvm_set_pmd(pmd_t *pmdp, pmd_t pmd)
+{
+	kvm_mmu_write(pmdp, &pmd, sizeof(pmd));
+}
+
+#if PAGETABLE_LEVELS >= 3
+#ifdef CONFIG_X86_PAE
+static void kvm_set_pte_atomic(pte_t *ptep, pte_t pte)
+{
+	kvm_mmu_write(ptep, &pte, sizeof(pte));
+}
+
+static void kvm_set_pte_present(struct mm_struct *mm, unsigned long addr,
+				pte_t *ptep, pte_t pte)
+{
+	kvm_mmu_write(ptep, &pte, sizeof(pte));
+}
+
+static void kvm_pte_clear(struct mm_struct *mm,
+			  unsigned long addr, pte_t *ptep)
+{
+	pte_t pte = __pte(0);
+	kvm_mmu_write(ptep, &pte, sizeof(pte));
+}
+
+static void kvm_pmd_clear(pmd_t *pmdp)
+{
+	pmd_t pmd = __pmd(0);
+	kvm_mmu_write(pmdp, &pmd, sizeof(pmd));
+}
+#endif
+
+static void kvm_set_pgd(pgd_t *pgdp, pgd_t pgd)
+{
+	kvm_mmu_write(pgdp, &pgd, sizeof(pgd));
+}
+
+static void kvm_set_pud(pud_t *pudp, pud_t pud)
+{
+	kvm_mmu_write(pudp, &pud, sizeof(pud));
+}
+#endif /* PAGETABLE_LEVELS >= 3 */
+
+static void kvm_flush_tlb(void)
+{
+	kvm_hypercall0(KVM_HYPERCALL_FLUSH_TLB);
+}
+
+static void kvm_release_pt(u32 pfn)
+{
+	kvm_hypercall1(KVM_HYPERCALL_RELEASE_PT, pfn << PAGE_SHIFT);
+}
+
 static void paravirt_ops_setup(void)
 {
 	pv_info.name = "KVM";
@@ -41,6 +126,24 @@ static void paravirt_ops_setup(void)
 	if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
 		pv_cpu_ops.io_delay = kvm_io_delay;
 
+	if (kvm_para_has_feature(KVM_FEATURE_MMU_WRITE)) {
+		pv_mmu_ops.set_pte = kvm_set_pte;
+		pv_mmu_ops.set_pte_at = kvm_set_pte_at;
+		pv_mmu_ops.set_pmd = kvm_set_pmd;
+#if PAGETABLE_LEVELS >= 3
+#ifdef CONFIG_X86_PAE
+		pv_mmu_ops.set_pte_atomic = kvm_set_pte_atomic;
+		pv_mmu_ops.set_pte_present = kvm_set_pte_present;
+		pv_mmu_ops.pte_clear = kvm_pte_clear;
+		pv_mmu_ops.pmd_clear = kvm_pmd_clear;
+#endif
+		pv_mmu_ops.set_pud = kvm_set_pud;
+		pv_mmu_ops.set_pgd = kvm_set_pgd;
+#endif
+		pv_mmu_ops.flush_tlb_user = kvm_flush_tlb;
+		pv_mmu_ops.release_pt = kvm_release_pt;
+		pv_mmu_ops.release_pd = kvm_release_pt;
+	}
 }
 
 void __init kvm_guest_init(void)
arch/x86/kvm/mmu.c +3 -3
Index: kvm.paravirt/arch/x86/kvm/mmu.c
===================================================================
@@ -39,7 +39,7 @@
  * 2. while doing 1. it walks guest-physical to host-physical
  * If the hardware supports that we don't need to do shadow paging.
  */
-static bool tdp_enabled = false;
+bool tdp_enabled = false;
 
 #undef MMU_DEBUG
 
@@ -288,7 +288,7 @@ static void mmu_free_memory_cache_page(s
 		free_page((unsigned long)mc->objects[--mc->nobjs]);
 }
 
-static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu)
+int mmu_topup_memory_caches(struct kvm_vcpu *vcpu)
 {
 	int r;
 
@@ -857,7 +857,7 @@ static int kvm_mmu_unprotect_page(struct
 	return r;
 }
 
-static void mmu_unshadow(struct kvm *kvm, gfn_t gfn)
+void mmu_unshadow(struct kvm *kvm, gfn_t gfn)
 {
 	struct kvm_mmu_page *sp;
 
arch/x86/kvm/mmu.h +3 -0
Index: kvm.paravirt/arch/x86/kvm/mmu.h
===================================================================
@@ -47,4 +47,7 @@ static inline int is_paging(struct kvm_v
 	return vcpu->arch.cr0 & X86_CR0_PG;
 }
 
+void mmu_unshadow(struct kvm *kvm, gfn_t gfn);
+int mmu_topup_memory_caches(struct kvm_vcpu *vcpu);
+
 #endif
arch/x86/kvm/x86.c +57 -0

75 lines not on this page · raw message

include/asm-x86/kvm_para.h +3 -1
Index: kvm.paravirt/include/asm-x86/kvm_para.h
===================================================================
@@ -6,6 +6,7 @@
  */
 #define KVM_CPUID_SIGNATURE	0x40000000
 #define KVM_FEATURE_NOP_IO_DELAY	0
+#define KVM_FEATURE_MMU_WRITE		1
 
 /* This CPUID returns a feature bitmap in eax.  Before enabling a particular
  * paravirtualization, the appropriate feature bit should be checked.
@@ -15,7 +16,8 @@
 #ifdef __KERNEL__
 #include <asm/processor.h>
 
-#define KVM_PARA_FEATURES (1UL << KVM_FEATURE_NOP_IO_DELAY)
+#define KVM_PARA_FEATURES ((1UL << KVM_FEATURE_NOP_IO_DELAY)	|	\
+			   (1UL << KVM_FEATURE_MMU_WRITE))
 
 /* This instruction is vmcall.  On non-VT architectures, it will generate a
  * trap that we will then rewrite to the appropriate instruction.
include/linux/kvm_para.h +6 -1
Index: kvm.paravirt/include/linux/kvm_para.h
===================================================================
@@ -11,8 +11,13 @@
 
 /* Return values for hypercalls */
 #define KVM_ENOSYS		1000
+#define KVM_EFAULT		EFAULT
+#define KVM_E2BIG		E2BIG
 
-#define KVM_HC_VAPIC_POLL_IRQ            1
+#define KVM_HC_VAPIC_POLL_IRQ		1
+#define KVM_HYPERCALL_MMU_WRITE		2
+#define KVM_HYPERCALL_FLUSH_TLB		3
+#define KVM_HYPERCALL_RELEASE_PT	4
 
 /*
  * hypercalls use architecture specific
include/asm-x86/kvm_host.h +2 -0
Index: kvm.paravirt/include/asm-x86/kvm_host.h
===================================================================
@@ -412,6 +412,8 @@ void kvm_mmu_change_mmu_pages(struct kvm
 
 int load_pdptrs(struct kvm_vcpu *vcpu, unsigned long cr3);
 
+extern bool tdp_enabled;
+
 enum emulation_result {
 	EMULATE_DONE,       /* no further processing */
 	EMULATE_DO_MMIO,      /* kvm_run filled with mmio request */
Signature · 4 lines
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#3 lore raw

PATCH 3/5 KVM: hypercall batching

In reply to a message not in the archive

Batch pte updates and tlb flushes in lazy MMU mode.

arch/x86/kernel/kvm.c +95 -3
Index: kvm.paravirt/arch/x86/kernel/kvm.c
===================================================================
@@ -25,6 +25,74 @@
 #include <linux/kvm_para.h>
 #include <linux/cpu.h>
 #include <linux/mm.h>
+#include <linux/hardirq.h>
+
+#define MAX_MULTICALL_NR (PAGE_SIZE / sizeof(struct kvm_multicall_entry))
+
+struct kvm_para_state {
+	struct kvm_multicall_entry queue[MAX_MULTICALL_NR];
+	int queue_index;
+	enum paravirt_lazy_mode mode;
+};
+
+static DEFINE_PER_CPU(struct kvm_para_state, para_state);
+
+static int can_defer_hypercall(struct kvm_para_state *state, unsigned int nr)
+{
+	if (state->mode == PARAVIRT_LAZY_MMU) {
+		switch (nr) {
+		case KVM_HYPERCALL_MMU_WRITE:
+		case KVM_HYPERCALL_FLUSH_TLB:
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static void hypercall_queue_flush(struct kvm_para_state *state)
+{
+	if (state->queue_index) {
+		kvm_hypercall2(KVM_HYPERCALL_MULTICALL, __pa(&state->queue),
+			      state->queue_index);
+		state->queue_index = 0;
+	}
+}
+
+static void kvm_hypercall_defer(struct kvm_para_state *state,
+				unsigned int nr,
+				unsigned long a0, unsigned long a1,
+				unsigned long a2, unsigned long a3)
+{
+	struct kvm_multicall_entry *entry;
+
+	BUG_ON(preemptible());
+
+	if (state->queue_index == MAX_MULTICALL_NR)
+		hypercall_queue_flush(state);
+
+	entry = &state->queue[state->queue_index++];
+	entry->nr = nr;
+	entry->a0 = a0;
+	entry->a1 = a1;
+	entry->a2 = a2;
+	entry->a3 = a3;
+}
+
+static long kvm_hypercall(unsigned int nr, unsigned long a0,
+			  unsigned long a1, unsigned long a2,
+			  unsigned long a3)
+{
+	struct kvm_para_state *state = &get_cpu_var(para_state);
+	long ret = 0;
+
+	if (can_defer_hypercall(state, nr))
+		kvm_hypercall_defer(state, nr, a0, a1, a2, a3);
+	else
+		ret = kvm_hypercall4(nr, a0, a1, a2, a3);
+
+	put_cpu_var(para_state);
+	return ret;
+}
 
 /*
  * No need for any "IO delay" on KVM
@@ -44,7 +112,7 @@ static void kvm_mmu_write(void *dest, co
 	if (size == 2)
 		a1 = *(u32 *)&p[4];
 #endif
-	kvm_hypercall4(KVM_HYPERCALL_MMU_WRITE, (unsigned long)dest, size, a0,
+	kvm_hypercall(KVM_HYPERCALL_MMU_WRITE, (unsigned long)dest, size, a0,
 		       a1);
 }
 
@@ -110,12 +178,31 @@ static void kvm_set_pud(pud_t *pudp, pud
 
 static void kvm_flush_tlb(void)
 {
-	kvm_hypercall0(KVM_HYPERCALL_FLUSH_TLB);
+	kvm_hypercall(KVM_HYPERCALL_FLUSH_TLB, 0, 0, 0, 0);
 }
 
 static void kvm_release_pt(u32 pfn)
 {
-	kvm_hypercall1(KVM_HYPERCALL_RELEASE_PT, pfn << PAGE_SHIFT);
+	kvm_hypercall(KVM_HYPERCALL_RELEASE_PT, pfn << PAGE_SHIFT, 0, 0, 0);
+}
+
+static void kvm_enter_lazy_mmu(void)
+{
+	struct kvm_para_state *state
+		= &per_cpu(para_state, smp_processor_id());
+
+	paravirt_enter_lazy_mmu();
+	state->mode = paravirt_get_lazy_mode();
+}
+
+static void kvm_leave_lazy_mmu(void)
+{
+	struct kvm_para_state *state
+		= &per_cpu(para_state, smp_processor_id());
+
+	hypercall_queue_flush(state);
+	paravirt_leave_lazy(paravirt_get_lazy_mode());
+	state->mode = paravirt_get_lazy_mode();
 }
 
 static void paravirt_ops_setup(void)
@@ -144,6 +231,11 @@ static void paravirt_ops_setup(void)
 		pv_mmu_ops.release_pt = kvm_release_pt;
 		pv_mmu_ops.release_pd = kvm_release_pt;
 	}
+
+	if (kvm_para_has_feature(KVM_FEATURE_MULTICALL)) {
+		pv_mmu_ops.lazy_mode.enter = kvm_enter_lazy_mmu;
+		pv_mmu_ops.lazy_mode.leave = kvm_leave_lazy_mmu;
+	}
 }
 
 void __init kvm_guest_init(void)
arch/x86/kvm/x86.c +56 -18

99 lines not on this page · raw message

include/asm-x86/kvm_host.h +2 -0
Index: kvm.paravirt/include/asm-x86/kvm_host.h
===================================================================
@@ -320,6 +320,8 @@ struct kvm_vcpu_stat {
 	u32 fpu_reload;
 	u32 insn_emulation;
 	u32 insn_emulation_fail;
+	u32 multicall;
+	u32 multicall_nr;
 };
 
 struct descriptor_table {
include/asm-x86/kvm_para.h +12 -1
Index: kvm.paravirt/include/asm-x86/kvm_para.h
===================================================================
@@ -7,6 +7,7 @@
 #define KVM_CPUID_SIGNATURE	0x40000000
 #define KVM_FEATURE_NOP_IO_DELAY	0
 #define KVM_FEATURE_MMU_WRITE		1
+#define KVM_FEATURE_MULTICALL		2
 
 /* This CPUID returns a feature bitmap in eax.  Before enabling a particular
  * paravirtualization, the appropriate feature bit should be checked.
@@ -17,7 +18,17 @@
 #include <asm/processor.h>
 
 #define KVM_PARA_FEATURES ((1UL << KVM_FEATURE_NOP_IO_DELAY)	|	\
-			   (1UL << KVM_FEATURE_MMU_WRITE))
+			   (1UL << KVM_FEATURE_MMU_WRITE)	|	\
+			   (1UL << KVM_FEATURE_MULTICALL))
+
+struct kvm_multicall_entry
+{
+	u64 nr;
+	u64 a0;
+	u64 a1;
+	u64 a2;
+	u64 a3;
+};
 
 /* This instruction is vmcall.  On non-VT architectures, it will generate a
  * trap that we will then rewrite to the appropriate instruction.
include/linux/kvm_para.h +2 -0
Index: kvm.paravirt/include/linux/kvm_para.h
===================================================================
@@ -13,11 +13,13 @@
 #define KVM_ENOSYS		1000
 #define KVM_EFAULT		EFAULT
 #define KVM_E2BIG		E2BIG
+#define KVM_EINVAL		EINVAL
 
 #define KVM_HC_VAPIC_POLL_IRQ		1
 #define KVM_HYPERCALL_MMU_WRITE		2
 #define KVM_HYPERCALL_FLUSH_TLB		3
 #define KVM_HYPERCALL_RELEASE_PT	4
+#define KVM_HYPERCALL_MULTICALL		5
 
 /*
  * hypercalls use architecture specific
Signature · 4 lines
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#4 lore raw

PATCH 4/5 KVM: ignore zapped root pagetables

In reply to a message not in the archive

Mark zapped root pagetables as invalid and ignore such pages during lookup.

This is a problem with the cr3-target feature, where a zapped root table fools the faulting code into creating a read-only mapping. The result is a lockup if the instruction can't be emulated.

arch/x86/kvm/mmu.c +5 -2
Index: kvm.paravirt/arch/x86/kvm/mmu.c
===================================================================
@@ -668,7 +668,8 @@ static struct kvm_mmu_page *kvm_mmu_look
 	index = kvm_page_table_hashfn(gfn);
 	bucket = &kvm->arch.mmu_page_hash[index];
 	hlist_for_each_entry(sp, node, bucket, hash_link)
-		if (sp->gfn == gfn && !sp->role.metaphysical) {
+		if (sp->gfn == gfn && !sp->role.metaphysical
+		    && !sp->role.invalid) {
 			pgprintk("%s: found role %x\n",
 				 __FUNCTION__, sp->role.word);
 			return sp;
@@ -796,8 +797,10 @@ static void kvm_mmu_zap_page(struct kvm 
 	if (!sp->root_count) {
 		hlist_del(&sp->hash_link);
 		kvm_mmu_free_page(kvm, sp);
-	} else
+	} else {
 		list_move(&sp->link, &kvm->arch.active_mmu_pages);
+		sp->role.invalid = 1;
+	}
 	kvm_mmu_reset_last_pte_updated(kvm);
 }
 
include/asm-x86/kvm_host.h +1 -0
Index: kvm.paravirt/include/asm-x86/kvm_host.h
===================================================================
@@ -140,6 +140,7 @@ union kvm_mmu_page_role {
 		unsigned pad_for_nice_hex_output : 6;
 		unsigned metaphysical : 1;
 		unsigned access : 3;
+		unsigned invalid : 1;
 	};
 };
 
Signature · 4 lines
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#5 lore raw

PATCH 5/5 KVM: VMX cr3 cache support

In reply to a message not in the archive

Add support for the cr3 cache feature on Intel VMX CPU's. This avoids vmexits on context switch if the cr3 value is cached in one of the entries (currently 4 are present).

This is especially important for Xenner, where each guest syscalls involves a cr3 switch.

arch/x86/kernel/kvm.c +120 -1
Index: kvm.paravirt/arch/x86/kernel/kvm.c
===================================================================
@@ -26,14 +26,16 @@
 #include <linux/cpu.h>
 #include <linux/mm.h>
 #include <linux/hardirq.h>
+#include <asm/tlbflush.h>
 
 #define MAX_MULTICALL_NR (PAGE_SIZE / sizeof(struct kvm_multicall_entry))
 
 struct kvm_para_state {
+	struct kvm_cr3_cache cr3_cache;
 	struct kvm_multicall_entry queue[MAX_MULTICALL_NR];
 	int queue_index;
 	enum paravirt_lazy_mode mode;
-};
+} __attribute__ ((aligned(PAGE_SIZE)));
 
 static DEFINE_PER_CPU(struct kvm_para_state, para_state);
 
@@ -101,6 +103,98 @@ static void kvm_io_delay(void)
 {
 }
 
+static void kvm_new_cr3(unsigned long cr3)
+{
+	kvm_hypercall1(KVM_HYPERCALL_SET_CR3, cr3);
+}
+
+/*
+ * Special, register-to-cr3 instruction based hypercall API
+ * variant to the KVM host. This utilizes the cr3 filter capability
+ * of the hardware - if this works out then no VM exit happens,
+ * if a VM exit happens then KVM will get the virtual address too.
+ */
+static void kvm_write_cr3(unsigned long guest_cr3)
+{
+	struct kvm_para_state *para_state = &get_cpu_var(para_state);
+	struct kvm_cr3_cache *cache = &para_state->cr3_cache;
+	int idx;
+
+	/*
+	 * Check the cache (maintained by the host) for a matching
+	 * guest_cr3 => host_cr3 mapping. Use it if found:
+	 */
+	for (idx = 0; idx < cache->max_idx; idx++) {
+		if (cache->entry[idx].guest_cr3 == guest_cr3) {
+			/*
+			 * Cache-hit: we load the cached host-CR3 value.
+			 * This never causes any VM exit. (if it does then the
+			 * hypervisor could do nothing with this instruction
+			 * and the guest OS would be aborted)
+			 */
+			native_write_cr3(cache->entry[idx].host_cr3);
+			goto out;
+		}
+	}
+
+	/*
+	 * Cache-miss. Tell the host the new cr3 via hypercall (to avoid
+	 * aliasing problems with a cached host_cr3 == guest_cr3).
+	 */
+	kvm_new_cr3(guest_cr3);
+out:
+	put_cpu_var(para_state);
+}
+
+/*
+ * Avoid the VM exit upon cr3 load by using the cached
+ * ->active_mm->pgd value:
+ */
+static void kvm_flush_tlb_user(void)
+{
+	kvm_write_cr3(__pa(current->active_mm->pgd));
+}
+
+/*
+ * Disable global pages, do a flush, then enable global pages:
+ */
+static fastcall void kvm_flush_tlb_kernel(void)
+{
+	unsigned long orig_cr4 = read_cr4();
+
+	write_cr4(orig_cr4 & ~X86_CR4_PGE);
+	kvm_flush_tlb_user();
+	write_cr4(orig_cr4);
+}
+
+static void register_cr3_cache(void *cache)
+{
+	struct kvm_para_state *state;
+
+	state = &per_cpu(para_state, raw_smp_processor_id());
+	wrmsrl(KVM_MSR_SET_CR3_CACHE, __pa(&state->cr3_cache));
+}
+
+static unsigned __init kvm_patch(u8 type, u16 clobbers, void *ibuf,
+				 unsigned long addr, unsigned len)
+{
+	switch (type) {
+	case PARAVIRT_PATCH(pv_mmu_ops.write_cr3):
+		return paravirt_patch_default(type, clobbers, ibuf, addr, len);
+	default:
+		return native_patch(type, clobbers, ibuf, addr, len);
+	}
+}
+
+static void __init setup_guest_cr3_cache(void)
+{
+	on_each_cpu(register_cr3_cache, NULL, 0, 1);
+
+	pv_mmu_ops.write_cr3 = kvm_write_cr3;
+	pv_mmu_ops.flush_tlb_user = kvm_flush_tlb_user;
+	pv_mmu_ops.flush_tlb_kernel = kvm_flush_tlb_kernel;
+}
+
 static void kvm_mmu_write(void *dest, const void *src, size_t size)
 {
 	const uint8_t *p = src;
@@ -117,6 +211,28 @@ static void kvm_mmu_write(void *dest, co
 }
 
 /*
+ * CR3 cache initialization uses on_each_cpu(), so it can't
+ * happen at kvm_guest_init time.
+ */
+int __init kvm_cr3_cache_init(void)
+{
+	unsigned long flags;
+
+	if (!kvm_para_available())
+		return -ENOSYS;
+
+	if (kvm_para_has_feature(KVM_FEATURE_CR3_CACHE)) {
+		setup_guest_cr3_cache();
+		local_irq_save(flags);
+		apply_paravirt(__parainstructions, __parainstructions_end);
+		local_irq_restore(flags);
+	}
+
+	return 0;
+}
+module_init(kvm_cr3_cache_init);
+
+/*
  * We only need to hook operations that are MMU writes.  We hook these so that
  * we can use lazy MMU mode to batch these operations.  We could probably
  * improve the performance of the host code if we used some of the information
@@ -236,6 +352,9 @@ static void paravirt_ops_setup(void)
 		pv_mmu_ops.lazy_mode.enter = kvm_enter_lazy_mmu;
 		pv_mmu_ops.lazy_mode.leave = kvm_leave_lazy_mmu;
 	}
+
+	if (kvm_para_has_feature(KVM_FEATURE_CR3_CACHE))
+		pv_init_ops.patch = kvm_patch;
 }
 
 void __init kvm_guest_init(void)
arch/x86/kvm/mmu.c +124 -56

396 lines not on this page · raw message

arch/x86/kvm/mmu.h +2 -1
Index: kvm.paravirt/arch/x86/kvm/mmu.h
===================================================================
@@ -17,7 +17,8 @@ static inline void kvm_mmu_free_some_pag
 
 static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
 {
-	if (likely(vcpu->arch.mmu.root_hpa != INVALID_PAGE))
+	int idx = vcpu->arch.cr3_cache_idx;
+	if (likely(vcpu->arch.mmu.root_hpa[idx] != INVALID_PAGE))
 		return 0;
 
 	return kvm_mmu_load(vcpu);
arch/x86/kvm/paging_tmpl.h +2 -2
Index: kvm.paravirt/arch/x86/kvm/paging_tmpl.h
===================================================================
@@ -283,10 +283,10 @@ static u64 *FNAME(fetch)(struct kvm_vcpu
 	if (!is_present_pte(walker->ptes[walker->level - 1]))
 		return NULL;
 
-	shadow_addr = vcpu->arch.mmu.root_hpa;
+	shadow_addr = vcpu->arch.mmu.root_hpa[vcpu->arch.cr3_cache_idx];
 	level = vcpu->arch.mmu.shadow_root_level;
 	if (level == PT32E_ROOT_LEVEL) {
-		shadow_addr = vcpu->arch.mmu.pae_root[(addr >> 30) & 3];
+		shadow_addr = vcpu->arch.mmu.pae_root[vcpu->arch.cr3_cache_idx][(addr >> 30) & 3];
 		shadow_addr &= PT64_BASE_ADDR_MASK;
 		--level;
 	}
arch/x86/kvm/vmx.c +142 -1

218 lines not on this page · raw message

arch/x86/kvm/x86.c +15 -2
Index: kvm.paravirt/arch/x86/kvm/x86.c
===================================================================
@@ -80,6 +80,7 @@ struct kvm_stats_debugfs_item debugfs_en
 	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
 	{ "multicall", VCPU_STAT(multicall) },
 	{ "multicall_nr", VCPU_STAT(multicall_nr) },
+	{ "cr3_cache_synced", VCPU_STAT(cr3_cache_synced) },
 	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
 	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
 	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },
@@ -766,10 +767,13 @@ long kvm_arch_dev_ioctl(struct file *fil
 	}
 	case KVM_GET_PARA_FEATURES: {
 		__u32 para_features = KVM_PARA_FEATURES;
+
 		if (tdp_enabled) {
 			para_features &= ~(1UL << KVM_FEATURE_MMU_WRITE);
 			para_features &= ~(1UL << KVM_FEATURE_MULTICALL);
 		}
+		if (!kvm_x86_ops->cpu_has_cr3_cache())
+			para_features &= ~(1UL << KVM_FEATURE_CR3_CACHE);
 
 		r = -EFAULT;
 		if (copy_to_user(argp, &para_features, sizeof para_features))
@@ -2321,6 +2325,12 @@ static int kvm_hypercall_release_pt(stru
 	return 0;
 }
 
+static int kvm_hypercall_set_cr3(struct kvm_vcpu *vcpu, gpa_t cr3)
+{
+	set_cr3(vcpu, cr3);
+	return 0;
+}
+
 static int dispatch_hypercall(struct kvm_vcpu *vcpu, unsigned long nr,
 			      unsigned long a0, unsigned long a1,
 			      unsigned long a2, unsigned long a3)
@@ -2334,6 +2344,8 @@ static int dispatch_hypercall(struct kvm
 		return kvm_hypercall_flush_tlb(vcpu);
 	case KVM_HYPERCALL_RELEASE_PT:
 		return kvm_hypercall_release_pt(vcpu, a0);
+	case KVM_HYPERCALL_SET_CR3:
+		return kvm_hypercall_set_cr3(vcpu, a0);
 	}
 
 	return -KVM_ENOSYS;
@@ -3245,12 +3257,13 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *
 {
 	struct page *page;
 	struct kvm *kvm;
-	int r;
+	int r, i;
 
 	BUG_ON(vcpu->kvm == NULL);
 	kvm = vcpu->kvm;
 
-	vcpu->arch.mmu.root_hpa = INVALID_PAGE;
+	for (i = 0; i < KVM_CR3_CACHE_SIZE; i++)
+		vcpu->arch.mmu.root_hpa[i] = INVALID_PAGE;
 	if (!irqchip_in_kernel(kvm) || vcpu->vcpu_id == 0)
 		vcpu->arch.mp_state = VCPU_MP_STATE_RUNNABLE;
 	else
include/asm-x86/kvm_host.h +7 -2
Index: kvm.paravirt/include/asm-x86/kvm_host.h
===================================================================
@@ -181,11 +181,11 @@ struct kvm_mmu {
 	gpa_t (*gva_to_gpa)(struct kvm_vcpu *vcpu, gva_t gva);
 	void (*prefetch_page)(struct kvm_vcpu *vcpu,
 			      struct kvm_mmu_page *page);
-	hpa_t root_hpa;
+	hpa_t root_hpa[KVM_CR3_CACHE_SIZE];
 	int root_level;
 	int shadow_root_level;
 
-	u64 *pae_root;
+	u64 *pae_root[KVM_CR3_CACHE_SIZE];
 };
 
 struct kvm_vcpu_arch {
@@ -199,6 +199,9 @@ struct kvm_vcpu_arch {
 	unsigned long cr0;
 	unsigned long cr2;
 	unsigned long cr3;
+	struct kvm_cr3_cache *cr3_cache;
+	unsigned int cr3_cache_idx;
+	unsigned int cr3_cache_limit;
 	unsigned long cr4;
 	unsigned long cr8;
 	u64 pdptrs[4]; /* pae */
@@ -323,6 +326,7 @@ struct kvm_vcpu_stat {
 	u32 insn_emulation_fail;
 	u32 multicall;
 	u32 multicall_nr;
+	u32 cr3_cache_synced;
 };
 
 struct descriptor_table {
@@ -339,6 +343,7 @@ struct kvm_x86_ops {
 	int (*hardware_setup)(void);               /* __init */
 	void (*hardware_unsetup)(void);            /* __exit */
 	bool (*cpu_has_accelerated_tpr)(void);
+	bool (*cpu_has_cr3_cache)(void);
 
 	/* Create, but do not attach this VCPU */
 	struct kvm_vcpu *(*vcpu_create)(struct kvm *kvm, unsigned id);
include/asm-x86/kvm_para.h +17 -1
Index: kvm.paravirt/include/asm-x86/kvm_para.h
===================================================================
@@ -8,6 +8,7 @@
 #define KVM_FEATURE_NOP_IO_DELAY	0
 #define KVM_FEATURE_MMU_WRITE		1
 #define KVM_FEATURE_MULTICALL		2
+#define KVM_FEATURE_CR3_CACHE		3
 
 /* This CPUID returns a feature bitmap in eax.  Before enabling a particular
  * paravirtualization, the appropriate feature bit should be checked.
@@ -19,7 +20,10 @@
 
 #define KVM_PARA_FEATURES ((1UL << KVM_FEATURE_NOP_IO_DELAY)	|	\
 			   (1UL << KVM_FEATURE_MMU_WRITE)	|	\
-			   (1UL << KVM_FEATURE_MULTICALL))
+			   (1UL << KVM_FEATURE_MULTICALL)	|	\
+			   (1UL << KVM_FEATURE_CR3_CACHE))
+
+#define KVM_MSR_SET_CR3_CACHE 0x87655678
 
 struct kvm_multicall_entry
 {
@@ -118,4 +122,16 @@ static inline unsigned int kvm_arch_para
 
 #endif
 
+#define KVM_CR3_CACHE_SIZE 4
+
+struct kvm_cr3_cache_entry {
+	__u64 guest_cr3;
+	__u64 host_cr3;
+};
+
+struct kvm_cr3_cache {
+	struct kvm_cr3_cache_entry entry[KVM_CR3_CACHE_SIZE];
+	__u32 max_idx;
+};
+
 #endif
include/linux/kvm_para.h +1 -0
Index: kvm.paravirt/include/linux/kvm_para.h
===================================================================
@@ -20,6 +20,7 @@
 #define KVM_HYPERCALL_FLUSH_TLB		3
 #define KVM_HYPERCALL_RELEASE_PT	4
 #define KVM_HYPERCALL_MULTICALL		5
+#define KVM_HYPERCALL_SET_CR3		6
 
 /*
  * hypercalls use architecture specific
arch/x86/kvm/svm.c +6 -0
Index: kvm.paravirt/arch/x86/kvm/svm.c
===================================================================
@@ -1801,6 +1801,11 @@ static bool svm_cpu_has_accelerated_tpr(
 	return false;
 }
 
+static bool cpu_has_cr3_cache(void)
+{
+	return false;
+}
+
 static struct kvm_x86_ops svm_x86_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
@@ -1810,6 +1815,7 @@ static struct kvm_x86_ops svm_x86_ops = 
 	.hardware_enable = svm_hardware_enable,
 	.hardware_disable = svm_hardware_disable,
 	.cpu_has_accelerated_tpr = svm_cpu_has_accelerated_tpr,
+	.cpu_has_cr3_cache = cpu_has_cr3_cache,
 
 	.vcpu_create = svm_create_vcpu,
 	.vcpu_free = svm_free_vcpu,
Signature · 4 lines
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#6 lore raw

Re: [patch 0/5] KVM paravirt MMU updates and cr3 caching

In reply to a message not in the archive
Quoted 4 lines · Marcelo Tosatti

The following patchset, based on earlier work by Anthony and Ingo, adds paravirt_ops support for KVM guests enabling hypercall based pte updates, hypercall batching and cr3 caching.

Could you post performance results for each optimization? I'm particularly curious if the hypercall batching is very useful.

Regards,

Anthony Liguori

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#7 lore raw

Re: [patch 0/5] KVM paravirt MMU updates and cr3 caching

Quoted 8 lines · Anthony Liguori
Quoted 4 lines · Marcelo Tosatti

The following patchset, based on earlier work by Anthony and Ingo, adds paravirt_ops support for KVM guests enabling hypercall based pte updates, hypercall batching and cr3 caching.

Could you post performance results for each optimization? I'm particularly curious if the hypercall batching is very useful.

Batched hypercall pte updates give 8.5% performance improvement on kernel compile:

http://www.mail-archive.com/[email protected]/msg12395.html

I can get separate results tomorrow or Monday, but I'm sure batching plays a significant role. For the kernel compile test, there is an average of 5 pte updates per batched hypercall.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw

Re: [patch 2/5] KVM: hypercall based pte updates and TLB flushes

Quoted 5 lines · Marcelo Tosatti

Hypercall based pte updates are faster than faults, and also allow use of the lazy MMU mode to batch operations.

Don't report the feature if two dimensional paging is enabled.

Quoted 8 lines

+/*
+ * We only need to hook operations that are MMU writes. We hook these so that
+ * we can use lazy MMU mode to batch these operations. We could probably
+ * improve the performance of the host code if we used some of the information
+ * here to simplify processing of batched writes.
+ */

One option is, if the guest promises never to write to a page table directly, is to avoid write protecting guest page tables. I think the shadow code can handle it (since the gfn/spte relationship is maintained by shadow code, and doesn't require reading the guest page tables), but am not sure.

Signature · 7 lines
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#10 lore raw

Re: [patch 2/5] KVM: hypercall based pte updates and TLB flushes

Quoted 18 lines · Avi Kivity
Quoted 6 lines · Marcelo Tosatti

Hypercall based pte updates are faster than faults, and also allow use of the lazy MMU mode to batch operations.

Don't report the feature if two dimensional paging is enabled.

Quoted 10 lines

+/*
+ * We only need to hook operations that are MMU writes. We hook these so that
+ * we can use lazy MMU mode to batch these operations. We could probably
+ * improve the performance of the host code if we used some of the information
+ * here to simplify processing of batched writes.
+ */

Quoted 6 lines

One option is, if the guest promises never to write to a page table directly, is to avoid write protecting guest page tables. I think the shadow code can handle it (since the gfn/spte relationship is maintained by shadow code, and doesn't require reading the guest page tables), but am not sure.

In addition to reducing mmu work for write protection, this allows more efficient use of large pages.

Signature · 7 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#11 lore raw

Re: [patch 2/5] KVM: hypercall based pte updates and TLB flushes

Quoted 26 lines · Avi Kivity
Quoted 18 lines · Avi Kivity
Quoted 6 lines · Marcelo Tosatti

Hypercall based pte updates are faster than faults, and also allow use of the lazy MMU mode to batch operations.

Don't report the feature if two dimensional paging is enabled.

Quoted 10 lines

+/*
+ * We only need to hook operations that are MMU writes. We hook these so that
+ * we can use lazy MMU mode to batch these operations. We could probably
+ * improve the performance of the host code if we used some of the information
+ * here to simplify processing of batched writes.
+ */

Quoted 6 lines

One option is, if the guest promises never to write to a page table directly, is to avoid write protecting guest page tables. I think the shadow code can handle it (since the gfn/spte relationship is maintained by shadow code, and doesn't require reading the guest page tables), but am not sure.

Quoted 2 lines

In addition to reducing mmu work for write protection, this allows more efficient use of large pages.

Yes, and gets rid of the remote TLB flushing.

Issue is the paravirt_ops code in Linux does not cover all pte updates (bit updates, ptep_get_and_clear, etc).

The plan is to get the basic infrastructure merged into KVM first (which is a significant improvement already) and then later have paravirt_ops cover all updates, disabling write protection.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#12 lore raw

Re: [patch 2/5] KVM: hypercall based pte updates and TLB flushes

Quoted 7 lines · Marcelo Tosatti

Issue is the paravirt_ops code in Linux does not cover all pte updates (bit updates, ptep_get_and_clear, etc).

The plan is to get the basic infrastructure merged into KVM first (which is a significant improvement already) and then later have paravirt_ops cover all updates, disabling write protection.

Okay, sounds good.

Do you know if anyone is working on extending pv_ops as you describe?

Signature · 7 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#13 lore raw

Re: [patch 2/5] KVM: hypercall based pte updates and TLB flushes

Quoted 4 lines · Avi Kivity
Quoted 3 lines · Marcelo Tosatti

Issue is the paravirt_ops code in Linux does not cover all pte updates (bit updates, ptep_get_and_clear, etc).

Quoted 8 lines
Quoted 4 lines

The plan is to get the basic infrastructure merged into KVM first (which is a significant improvement already) and then later have paravirt_ops cover all updates, disabling write protection.

Okay, sounds good.

Do you know if anyone is working on extending pv_ops as you describe?

Not that I'm aware of.

I was planning to do it. The first part of that is a mask based bit update hypercall (ie. a non-broken version of the patch I posted earlier which Jeremy replied to).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#14 lore raw

Re: [patch 2/5] KVM: hypercall based pte updates and TLB flushes

In reply to a message not in the archive
Quoted 25 lines · Marcelo Tosatti

Hypercall based pte updates are faster than faults, and also allow use of the lazy MMU mode to batch operations.

Don't report the feature if two dimensional paging is enabled.

(quoted diff) +16 -0
+static int kvm_hypercall_mmu_write(struct kvm_vcpu *vcpu, gva_t addr,
+				   unsigned long size, unsigned long a0,
+				   unsigned long a1)
+{
+	gpa_t gpa;
+	u64 value;
+
+	if (mmu_topup_memory_caches(vcpu))
+		return -KVM_EFAULT;
+
+	down_read(&vcpu->kvm->slots_lock);
+	gpa = vcpu->arch.mmu.gva_to_gpa(vcpu, addr);
+	up_read(&vcpu->kvm->slots_lock);
+
+	if (gpa == UNMAPPED_GVA)
+		return -KVM_EFAULT;

Review

Quoted 10 lines
(quoted diff) +10 -0
+	if (size == 1) {
+		if (!emulator_write_phys(vcpu, gpa, &a0, sizeof(a0)))
+			return -KVM_EFAULT;
+	} else if (size == 2) {
+		if (!is_long_mode(vcpu) && is_pae(vcpu))
+			value = (u64)a1 << 32 | a0;
+		else
+			value = a0;
+		if (!emulator_write_phys(vcpu, gpa, &value, sizeof(value)))
+			return -KVM_EFAULT;

The size logic can be simplified if it is redefined to mean "size in guest longs". Actually, it can be inferred from the guest mode, and a1 is only required iff the guest is a 32-bit pae.

Signature · 7 lines
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#16 lore raw

Re: [patch 3/5] KVM: hypercall batching

Quoted 3 lines · Marcelo Tosatti

Batch pte updates and tlb flushes in lazy MMU mode.

I am slightly uneasy about generic hypercall batching. An alternative way to do it would be to define a kvm_mmu_op structure (with an embedded opcode) and have a single hypercall execute a number of these (and not have any hypercall for executing just one).

Quoted 18 lines
(quoted diff) +18 -0
+static int kvm_hypercall_multicall(struct kvm_vcpu *vcpu, gpa_t addr, u32 nents)
+{
+	int i, result = 0;
+
+	++vcpu->stat.multicall;
+	vcpu->stat.multicall_nr += nents;
+
+	for (i = 0; i < nents; i++) {
+		struct kvm_multicall_entry mc;
+		int ret;
+
+		down_read(&vcpu->kvm->slots_lock);
+		ret = kvm_read_guest(vcpu->kvm, addr, &mc, sizeof(mc));
+		up_read(&vcpu->kvm->slots_lock);
+		if (ret)
+			return -KVM_EFAULT;
+
+		ret = dispatch_hypercall(vcpu, mc.nr, mc.a0, mc.a1, mc.a2,

Review

Quoted 10 lines
(quoted diff) +10 -0
+					    mc.a3);
+		if (ret)
+			result = ret;
+		addr += sizeof(mc);
+	}
+	if (result < 0)
+		return -KVM_EINVAL;
+	return result;
+}
+

The return code of the hypercalls (if positive) is lost. In the case that one hypercall failed, which one exactly is also lost.

For mmu ops, it doesn't matter (since if any fails, the guest is toast). For a generic framework, it may matter. If we keep it generic, I suggest adding a field for the return code of each hypercall, and have the multicall return the number of executed hypercalls.

Signature · 7 lines
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#17 lore raw

Re: [patch 3/5] KVM: hypercall batching

Quoted 43 lines · Avi Kivity
Quoted 3 lines · Marcelo Tosatti

Batch pte updates and tlb flushes in lazy MMU mode.

I am slightly uneasy about generic hypercall batching. An alternative way to do it would be to define a kvm_mmu_op structure (with an embedded opcode) and have a single hypercall execute a number of these (and not have any hypercall for executing just one).

Quoted 19 lines

+static int kvm_hypercall_multicall(struct kvm_vcpu *vcpu, gpa_t addr, u32 nents)

(quoted diff) +17 -0
+{
+	int i, result = 0;
+
+	++vcpu->stat.multicall;
+	vcpu->stat.multicall_nr += nents;
+
+	for (i = 0; i < nents; i++) {
+		struct kvm_multicall_entry mc;
+		int ret;
+
+		down_read(&vcpu->kvm->slots_lock);
+		ret = kvm_read_guest(vcpu->kvm, addr, &mc, sizeof(mc));
+		up_read(&vcpu->kvm->slots_lock);
+		if (ret)
+			return -KVM_EFAULT;
+
+		ret = dispatch_hypercall(vcpu, mc.nr, mc.a0, mc.a1, mc.a2,

Review

Quoted 10 lines
(quoted diff) +10 -0
+					    mc.a3);
+		if (ret)
+			result = ret;
+		addr += sizeof(mc);
+	}
+	if (result < 0)
+		return -KVM_EINVAL;
+	return result;
+}
+

The return code of the hypercalls (if positive) is lost. In the case that one hypercall failed, which one exactly is also lost.

Quoted 4 lines

For mmu ops, it doesn't matter (since if any fails, the guest is toast). For a generic framework, it may matter. If we keep it generic, I suggest adding a field for the return code of each hypercall, and have the multicall return the number of executed hypercalls.

We need a "generic" mechanism which allows different operations in the same multicall. For example mixed bit pte updates and normal whole pte updates.

I've added an error code to each multicall entry and changed the hypercall itself to return the number of processed entries as you suggested.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#18 lore raw

Re: [patch 3/5] KVM: hypercall batching

In reply to a message not in the archive
Quoted 15 lines · Marcelo Tosatti

plain text document attachment (kvm-multicall) Batch pte updates and tlb flushes in lazy MMU mode.

Index: kvm.paravirt/arch/x86/kernel/kvm.c
===================================================================

arch/x86/kernel/kvm.c +1 -0
@@ -25,6 +25,74 @@
 #include <linux/kvm_para.h>
 #include <linux/cpu.h>
 #include <linux/mm.h>
+#include <linux/hardirq.h>

arch/x86/kernel/kvm.c line 38

Quoted 10 lines
arch/x86/kernel/kvm.c +10 -0
+
+#define MAX_MULTICALL_NR (PAGE_SIZE / sizeof(struct kvm_multicall_entry))
+
+struct kvm_para_state {
+	struct kvm_multicall_entry queue[MAX_MULTICALL_NR];
+	int queue_index;
+	enum paravirt_lazy_mode mode;
+};
+
+static DEFINE_PER_CPU(struct kvm_para_state, para_state);

AFAICS there is no guarantee about page-alignment here...

Quoted 17 lines
arch/x86/kernel/kvm.c +17 -0
+static int kvm_hypercall_multicall(struct kvm_vcpu *vcpu, gpa_t addr, u32 nents)
+{
+	int i, result = 0;
+
+	++vcpu->stat.multicall;
+	vcpu->stat.multicall_nr += nents;
+
+	for (i = 0; i < nents; i++) {
+		struct kvm_multicall_entry mc;
+		int ret;
+
+		down_read(&vcpu->kvm->slots_lock);
+		ret = kvm_read_guest(vcpu->kvm, addr, &mc, sizeof(mc));
+		up_read(&vcpu->kvm->slots_lock);
+		if (ret)
+			return -KVM_EFAULT;
+

arch/x86/kernel/kvm.c line 65

Quoted 10 lines
arch/x86/kernel/kvm.c +10 -0
+		ret = dispatch_hypercall(vcpu, mc.nr, mc.a0, mc.a1, mc.a2,
+					    mc.a3);
+		if (ret)
+			result = ret;
+		addr += sizeof(mc);
+	}
+	if (result < 0)
+		return -KVM_EINVAL;
+	return result;
+}

... but here you're assuming that 'queue' is physically contiguous, which is not necessarily true one you cross a page boundary.

Signature · 8 lines
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#19 lore raw

Re: [patch 3/5] KVM: hypercall batching

Quoted 22 lines · Hollis Blanchard
Quoted 20 lines · Marcelo Tosatti

plain text document attachment (kvm-multicall) Batch pte updates and tlb flushes in lazy MMU mode.

Index: kvm.paravirt/arch/x86/kernel/kvm.c
===================================================================

arch/x86/kernel/kvm.c +6 -0
@@ -25,6 +25,74 @@
 #include <linux/kvm_para.h>
 #include <linux/cpu.h>
 #include <linux/mm.h>
+#include <linux/hardirq.h>
+
+#define MAX_MULTICALL_NR (PAGE_SIZE / sizeof(struct kvm_multicall_entry))
+
+struct kvm_para_state {
+	struct kvm_multicall_entry queue[MAX_MULTICALL_NR];
Quoted 10 lines

arch/x86/kernel/kvm.c line 39

Quoted 6 lines
arch/x86/kernel/kvm.c +5 -0
+	int queue_index;
+	enum paravirt_lazy_mode mode;
+};
+
+static DEFINE_PER_CPU(struct kvm_para_state, para_state);
 

AFAICS there is no guarantee about page-alignment here...

Right.

Quoted 23 lines
Quoted 23 lines
arch/x86/kernel/kvm.c +23 -0
+static int kvm_hypercall_multicall(struct kvm_vcpu *vcpu, gpa_t addr, u32 nents)
+{
+	int i, result = 0;
+
+	++vcpu->stat.multicall;
+	vcpu->stat.multicall_nr += nents;
+
+	for (i = 0; i < nents; i++) {
+		struct kvm_multicall_entry mc;
+		int ret;
+
+		down_read(&vcpu->kvm->slots_lock);
+		ret = kvm_read_guest(vcpu->kvm, addr, &mc, sizeof(mc));
+		up_read(&vcpu->kvm->slots_lock);
+		if (ret)
+			return -KVM_EFAULT;
+
+		ret = dispatch_hypercall(vcpu, mc.nr, mc.a0, mc.a1, mc.a2,
+					    mc.a3);
+		if (ret)
+			result = ret;
+		addr += sizeof(mc);
+	}
Quoted 9 lines

arch/x86/kernel/kvm.c line 67

Quoted 5 lines
arch/x86/kernel/kvm.c +4 -0
+	if (result < 0)
+		return -KVM_EINVAL;
+	return result;
+}
 

... but here you're assuming that 'queue' is physically contiguous, which is not necessarily true one you cross a page boundary.

Kernel data is physically contiguous (true for per-cpu data as well?), so no there's issue here.

Signature · 7 lines
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
#20 lore raw

Re: [patch 3/5] KVM: hypercall batching

Am Montag, 18. Februar 2008 schrieb Avi Kivity:

Quoted 4 lines
Quoted 1 line

AFAICS there is no guarantee about page-alignment here...

Kernel data is physically contiguous (true for per-cpu data as well?), so no there's issue here.

Modules are loaded into vmalloc space, no? I think, if kvm is built as module, static module variables are not guaranteed to be contiguous.

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#21 lore raw

Re: [patch 3/5] KVM: hypercall batching

Quoted 6 lines · Christian Borntraeger

Am Montag, 18. Februar 2008 schrieb Avi Kivity:

Quoted 2 lines
Quoted 2 lines

AFAICS there is no guarantee about page-alignment here...

Quoted 7 lines
Quoted 3 lines

Kernel data is physically contiguous (true for per-cpu data as well?), so no there's issue here.

Modules are loaded into vmalloc space, no? I think, if kvm is built as module, static module variables are not guaranteed to be contiguous.

It's not a module (the code is part of pv-ops for kvm, not the kvm host itself).

Signature · 7 lines
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): 1203361276.3428.6.camel@basalt

#23 lore raw

Re: [patch 3/5] KVM: hypercall batching

Quoted 4 lines · Hollis Blanchard
Quoted 3 lines

Kernel data is physically contiguous (true for per-cpu data as well?), so no there's issue here.

Quoted 8 lines

So this is an addition to the ABI, that the data must be physically contiguous. That's a pretty subtle implicit requirement, and it's easy to resolve the issue by requiring page-aligned data in the guest in the first place.

Please don't forget that there are other OSes that could use this interface as well...

I think it's fairly straightforward that a (phys_addr, length) API implies physically contiguous memory. If the guest doesn't have physically contiguous memory, it can page align itself.

But you do raise an important issue, that our ABIs (host/user and host/guest) are seriously underdocumented.

Signature · 7 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#25 lore raw

Re: [patch 4/5] KVM: ignore zapped root pagetables

Quoted 8 lines · Marcelo Tosatti

Mark zapped root pagetables as invalid and ignore such pages during lookup.

This is a problem with the cr3-target feature, where a zapped root table fools the faulting code into creating a read-only mapping. The result is a lockup if the instruction can't be emulated.

Review static void kvm_mmu_zap_page(struct kvm · line 804

Quoted 9 lines
(quoted diff) +3 -1
@@ -796,8 +797,10 @@ static void kvm_mmu_zap_page(struct kvm 
 	if (!sp->root_count) {
 		hlist_del(&sp->hash_link);
 		kvm_mmu_free_page(kvm, sp);
-	} else
+	} else {
 		list_move(&sp->link, &kvm->arch.active_mmu_pages);
+		sp->role.invalid = 1;
+	}
 	kvm_mmu_reset_last_pte_updated(kvm)

There's an smp issue here. You're marking a shadow page as invalid, but it may be currently in use by another vcpu. So the shadow page and the guest page may be out of sync.

A fix is to send an IPI to all vcpus in such a situation, and request them to unload the mmu.

Also, we can't rely on memory pressure to flush out the invalid shadow pages, because for many workloads the shadow cache is large enough (the "mmu_recycled" counter never increments). So a check for (root_count == 0 && role.invalid) when decrementing root_count can help to zap those pages.

Signature · 7 lines
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/