KVM for PowerPC: simpler TLB handling, better page management

RFC PATCH In review Hollis Blanchard 2 participants 3 messages Jan 25, 2008 – Feb 1, 2008

Subsystems (3)

KVM PowerPC KVM PowerPC

lore

Share & tools

lore

Hollis Blanchard

author

2008-01-25 01:08 UTC #0 lore raw

RFC PATCH 1/1 KVM for PowerPC: simpler TLB handling, better page management

Here's the updated version of PowerPC support. Still rough, but somewhat functional and getting better. :)

Changes:
* Big simplification in the TLB context switching: I threw out all that trampoline stuff. The trampoline allowed us to consume only 1 TLB entry when the guest was running, but was also led to very convoluted code. Now we leave the kernel's pinned entries present even when the guest is running. This will make things somewhat slower, since the guest has a couple fewer entries available. We can revisit this topic if necessary when we start looking at performance issues. (As a result, we are now incompatible with CONFIG_EARLY_DEBUG_44x on the host. That option was leaking a pinned AS=1 entry into the guest.)
* Stop leaking page refcounts. Basically every mapping in the shadow TLB (approx. 60) has had get_user_pages() called for it, and now when we evict it, we actually call put_page(). It might even work with swapping and highmem, who knows...
* I've also renamed a few functions that are 440-specific in anticipation of some e500 refactoring.

Going forward, I would like to get some sort of IO working besides console before submitting this for inclusion upstream. (So far we've been getting by with an initramfs.) IO support will depend on interrupt delivery, so that's the current focus.

Once we do have IO working, I feel pretty good about submitting this for serious review. Ultimately Avi will be the final approver, but I plan to copy linuxppc-dev for architectural and design comments.

PowerPC 440 KVM implementation.

Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>

MAINTAINERS +7 -0

@@ -2219,6 +2219,13 @@ W:	kvm.sourceforge.net
 W:	kvm.sourceforge.net
 S:	Supported
 
+KERNEL VIRTUAL MACHINE (KVM) FOR POWERPC
+P:	Hollis Blanchard
+M:	hollisb-r/[email protected]
+L:	[email protected]
+W:	kvm.qumranet.com
+S:	Supported
+
 KEXEC
 P:	Eric Biederman
 M:	ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]

arch/powerpc/Kconfig +1 -0

@@ -694,3 +694,4 @@ config PPC_LIB_RHEAP
 config PPC_LIB_RHEAP
 	bool
 
+source "arch/powerpc/kvm/Kconfig"

arch/powerpc/Kconfig.debug +1 -0

@@ -151,6 +151,7 @@ config BOOTX_TEXT
 
 config PPC_EARLY_DEBUG
 	bool "Early debugging (dangerous)"
+	depends on !KVM
 
 choice
 	prompt "Early debugging console"

arch/powerpc/Makefile +1 -0

@@ -147,6 +147,7 @@ core-y				+= arch/powerpc/kernel/ \
 				   arch/powerpc/platforms/
 core-$(CONFIG_MATH_EMULATION)	+= arch/powerpc/math-emu/
 core-$(CONFIG_XMON)		+= arch/powerpc/xmon/
+core-$(CONFIG_KVM) 		+= arch/powerpc/kvm/
 
 drivers-$(CONFIG_OPROFILE)	+= arch/powerpc/oprofile/

arch/powerpc/kernel/asm-offsets.c +27 -0

@@ -22,6 +22,7 @@
 #include <linux/mman.h>
 #include <linux/mm.h>
 #include <linux/suspend.h>
+#include <linux/kvm_host.h>
 #ifdef CONFIG_PPC64
 #include <linux/time.h>
 #include <linux/hardirq.h>

@@ -328,5 +329,31 @@ int main(void)
 
 	DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE);
 
+#ifdef CONFIG_KVM
+	DEFINE(TLBE_BYTES, sizeof(struct tlbe));
+
+	DEFINE(VCPU_HOST_STACK, offsetof(struct kvm_vcpu, arch.host_stack));
+	DEFINE(VCPU_HOST_PID, offsetof(struct kvm_vcpu, arch.host_pid));
+	DEFINE(VCPU_HOST_TLB, offsetof(struct kvm_vcpu, arch.host_tlb));
+	DEFINE(VCPU_SHADOW_TLB, offsetof(struct kvm_vcpu, arch.shadow_tlb));
+	DEFINE(VCPU_GPRS, offsetof(struct kvm_vcpu, arch.gpr));
+	DEFINE(VCPU_LR, offsetof(struct kvm_vcpu, arch.lr));
+	DEFINE(VCPU_CR, offsetof(struct kvm_vcpu, arch.cr));
+	DEFINE(VCPU_XER, offsetof(struct kvm_vcpu, arch.xer));
+	DEFINE(VCPU_CTR, offsetof(struct kvm_vcpu, arch.ctr));
+	DEFINE(VCPU_PC, offsetof(struct kvm_vcpu, arch.pc));
+	DEFINE(VCPU_GUEST_MSR, offsetof(struct kvm_vcpu, arch.guest_msr));
+	DEFINE(VCPU_SHADOW_MSR, offsetof(struct kvm_vcpu, arch.shadow_msr));
+	DEFINE(VCPU_SPRG4, offsetof(struct kvm_vcpu, arch.sprg4));
+	DEFINE(VCPU_SPRG5, offsetof(struct kvm_vcpu, arch.sprg5));
+	DEFINE(VCPU_SPRG6, offsetof(struct kvm_vcpu, arch.sprg6));
+	DEFINE(VCPU_SPRG7, offsetof(struct kvm_vcpu, arch.sprg7));
+	DEFINE(VCPU_PID, offsetof(struct kvm_vcpu, arch.pid));
+
+	DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
+	DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
+	DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
+#endif
+
 	return 0;
 }

arch/powerpc/kvm/44x_tlb.c new file +236 -0

236 lines not on this page · raw message

arch/powerpc/kvm/44x_tlb.h new file +91 -0

91 lines not on this page · raw message

arch/powerpc/kvm/Kconfig new file +45 -0

45 lines not on this page · raw message

arch/powerpc/kvm/Makefile new file +15 -0

new file mode 100644
@@ -0,0 +1,15 @@
+#
+# Makefile for Kernel-based Virtual Machine module
+#
+
+EXTRA_CFLAGS += -Ivirt/kvm -Iarch/powerpc/kvm
+
+common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o)
+
+kvm-objs := $(common-objs)
+obj-$(CONFIG_KVM) += kvm.o
+
+kvm-powerpc-objs := powerpc.o emulate.o 44x_tlb.o booke_interrupts.o
+obj-$(CONFIG_KVM_POWERPC) += kvm-powerpc.o
+
+AFLAGS_booke_interrupts.o := -I$(obj)

arch/powerpc/kvm/booke_interrupts.S new file +430 -0

430 lines not on this page · raw message

arch/powerpc/kvm/emulate.c new file +830 -0

830 lines not on this page · raw message

arch/powerpc/kvm/powerpc.c new file +785 -0

785 lines not on this page · raw message

include/asm-powerpc/kvm.h new file +56 -0

56 lines not on this page · raw message

include/asm-powerpc/kvm_asm.h new file +53 -0

53 lines not on this page · raw message

include/asm-powerpc/kvm_host.h new file +169 -0

169 lines not on this page · raw message

include/asm-powerpc/kvm_para.h new file +38 -0

new file mode 100644
@@ -0,0 +1,38 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * Copyright IBM Corp. 2008
+ *
+ * Authors: Hollis Blanchard <hollisb-r/[email protected]>
+ */
+
+#ifndef __POWERPC_KVM_PARA_H__
+#define __POWERPC_KVM_PARA_H__
+
+#ifdef __KERNEL__
+
+static inline int kvm_para_available(void)
+{
+	return 0;
+}
+
+static inline unsigned int kvm_arch_para_features(void)
+{
+	return 0;
+}
+
+#endif /* __KERNEL__ */
+
+#endif /* __POWERPC_KVM_PARA_H__ */
+

include/asm-powerpc/mmu-44x.h +2 -0

@@ -53,6 +53,8 @@
 
 #ifndef __ASSEMBLY__
 
+extern unsigned int tlb_44x_hwater;
+
 typedef unsigned long long phys_addr_t;
 
 typedef struct {

include/linux/kvm.h +7 -0

@@ -73,6 +73,7 @@ struct kvm_irqchip {
 #define KVM_EXIT_INTR             10
 #define KVM_EXIT_SET_TPR          11
 #define KVM_EXIT_TPR_ACCESS       12
+#define KVM_EXIT_DCR              13
 
 /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
 struct kvm_run {

@@ -137,6 +138,12 @@ struct kvm_run {
 			__u32 is_write;
 			__u32 pad;
 		} tpr_access;
+		/* KVM_EXIT_DCR */
+		struct {
+			__u32 dcrn;
+			__u32 data;
+			__u8  is_write;
+		} dcr;
 		/* Fix the size of the union. */
 		char padding[256];
 	};

Signature · 9 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/

Nathan Lynch

2008-02-01 19:07 UTC #1 lore raw

Quoted 11 lines · Hollis Blanchard

arch/powerpc/kernel/asm-offsets.c +1 -0

@@ -22,6 +22,7 @@
 #include <linux/mman.h>
 #include <linux/mm.h>
 #include <linux/suspend.h>
+#include <linux/kvm_host.h>
 #ifdef CONFIG_PPC64
 #include <linux/time.h>
 #include <linux/hardirq.h>

arch/powerpc/kernel/asm-offsets.c int main(void) · line 338

Quoted 10 lines

arch/powerpc/kernel/asm-offsets.c +7 -0

@@ -328,5 +329,31 @@ int main(void)
 
 	DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE);
 
+#ifdef CONFIG_KVM
+	DEFINE(TLBE_BYTES, sizeof(struct tlbe));
+
+	DEFINE(VCPU_HOST_STACK, offsetof(struct kvm_vcpu, arch.host_stack));
+	DEFINE(VCPU_HOST_PID, offsetof(struct kvm_vcpu, arch.host_pid));
+	DEFINE(VCPU_HOST_TLB, offsetof(struct kvm_vcpu, arch.host_tlb));
+	DEFINE(VCPU_SHADOW_TLB, offsetof(struct kvm_vcpu, arch.shadow_tlb));

I found that if CONFIG_KVM=m these definitions don't get picked up when generating asm-offsets.h, which causes the build to break:

AS arch/powerpc/kvm/booke_interrupts.o
arch/powerpc/kvm/booke_interrupts.S: Assembler messages:
arch/powerpc/kvm/booke_interrupts.S:347: Error: unsupported relocation against VCPU_HOST_TLB arch/powerpc/kvm/booke_interrupts.S:348: Error: unsupported relocation against VCPU_SHADOW_TLB
make[1]: *** [arch/powerpc/kvm/booke_interrupts.o] Error 1

Changing it to ifdef CONFIG_KVM_POWERPC (which is a bool) seems to do the right thing.

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

Hollis Blanchard

author

2008-02-01 19:17 UTC #2 lore raw

Quoted 34 lines · Nathan Lynch

Quoted 11 lines · Hollis Blanchard
arch/powerpc/kernel/asm-offsets.c +1 -0
@@ -22,6 +22,7 @@
 #include <linux/mman.h>
 #include <linux/mm.h>
 #include <linux/suspend.h>
+#include <linux/kvm_host.h>
 #ifdef CONFIG_PPC64
 #include <linux/time.h>
 #include <linux/hardirq.h>
arch/powerpc/kernel/asm-offsets.c int main(void) · line 338
Quoted 10 lines
arch/powerpc/kernel/asm-offsets.c +7 -0
@@ -328,5 +329,31 @@ int main(void)
 
 	DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE);
 
+#ifdef CONFIG_KVM
+	DEFINE(TLBE_BYTES, sizeof(struct tlbe));
+
+	DEFINE(VCPU_HOST_STACK, offsetof(struct kvm_vcpu, arch.host_stack));
+	DEFINE(VCPU_HOST_PID, offsetof(struct kvm_vcpu, arch.host_pid));
+	DEFINE(VCPU_HOST_TLB, offsetof(struct kvm_vcpu, arch.host_tlb));
+	DEFINE(VCPU_SHADOW_TLB, offsetof(struct kvm_vcpu, arch.shadow_tlb));
I found that if CONFIG_KVM=m these definitions don't get picked up when generating asm-offsets.h, which causes the build to break:
AS arch/powerpc/kvm/booke_interrupts.o
arch/powerpc/kvm/booke_interrupts.S: Assembler messages:
arch/powerpc/kvm/booke_interrupts.S:347: Error: unsupported relocation against VCPU_HOST_TLB arch/powerpc/kvm/booke_interrupts.S:348: Error: unsupported relocation against VCPU_SHADOW_TLB
make[1]: *** [arch/powerpc/kvm/booke_interrupts.o] Error 1

Quoted 2 lines

Changing it to ifdef CONFIG_KVM_POWERPC (which is a bool) seems to do the right thing.

Hmm... building as a module is something we've struggled with in the past, so I just left KVM_POWERPC as a bool because I didn't want to mess with it.

With PowerPC's asm-offsets dependency, is it really possible to build a standalone module if the kernel didn't have KVM=y?

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/