Re: [patch] KVM: add MSR based hypercall API

[Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>]

* Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org> wrote:

> NOTE: i have not updated the cr3 patch to the hypercall API yet (hence 
> the aliasing bug is not fixed yet), i wanted to get this to you so 
> that we can think about the hypercall API.
> 
> Right now what i have is only good to test that the VMCALL instruction 
> works - but the API must look differently. I'd prefer a register-based 
> thing so that i can embedd hypercalls within Linux without having to 
> go to a wrapper function. Right now a "call hypercall_addr" is a 
> regparm-based function entry. I'd like to keep that - but not have a 
> fixed number of parameters but inlines/macros for all parameter 
> combinations: 1, 2, 3, 4, 5 param, picked up automatically via use. 
> I.e. a 2-param call would be:
> 
> 	hypercall(KVM_cr3_miss, cr3);
> 
> a 3-param call would be:
> 
> 	hypercall(KVM_api_call1, param1, param2);
> 
> a 1-param call would be:
> 
> 	hypercall(KVM_api_call2);

i.e. something like the patch below (ontop of the previous tarball, 
still ad-hoc, these things need to go into a header, and i only have the 
1 and 2 param macros done).

this is very tightly integrated into the natural instruction sequence of 
functions that call it:

 c01209d4 <test_hypercall>:
 c01209d4:       55                      push   %ebp
 c01209d5:       89 e5                   mov    %esp,%ebp
 c01209d7:       50                      push   %eax
 c01209d8:       50                      push   %eax
 c01209d9:       e8 06 fa ff ff          call   c01203e4 <hypercall_addr>
 c01209de:       c7 04 24 48 41 50 c0    movl   $0xc0504148,(%esp)
 c01209e5:       e8 1f a9 00 00          call   c012b309 <printk>
 c01209ea:       e8 f5 f9 ff ff          call   c01203e4 <hypercall_addr>
 c01209ef:       c7 04 24 67 41 50 c0    movl   $0xc0504167,(%esp)
 c01209f6:       e8 0e a9 00 00          call   c012b309 <printk>
 c01209fb:       c9                      leave
 c01209fc:       c3                      ret

this is as cheap as it gets, and it only clobbers the registers that are 
needed. Basically "call hypercall_addr" is equivalent to a syscall trap 
instruction.

	Ingo

Index: linux/arch/i386/kernel/paravirt.c
===================================================================
--- linux.orig/arch/i386/kernel/paravirt.c
+++ linux/arch/i386/kernel/paravirt.c
@@ -888,29 +888,40 @@ asm (
 	"		ret				\n"
 );
 
-extern unsigned char hypercall_addr[4];
+extern unsigned char hypercall_addr[6];
 
-
-static inline int
-kvm_hypercall(void *param1, void *param2, void *param3, void *param4)
-{
-	int ret = -1;
-
-	asm (" call hypercall_addr\n"
-		: "=g" (ret)
-		: "eax" (param1),
-		  "ecx" (param2),
-		  "edx" (param3),
-		  "ebp" (param4));
-
-	return ret;
-}
+#define hypercall1(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr)			\
+	);					\
+	__ret;					\
+})
+
+#define hypercall2(nr, p1)			\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr),			\
+		  "ecx" (p1)			\
+	);					\
+	__ret;					\
+})
 
 void test_hypercall(void)
 {
-	int ret = kvm_hypercall((void *)1, (void *)2, (void *)3, (void *)4);
+	int ret;
+
+	ret = hypercall1(1);
+	printk(KERN_DEBUG "hypercall test #1, ret: %d\n", ret);
 
-	printk(KERN_DEBUG "hypercall test, ret: %d\n", ret);
+	ret = hypercall2(1, 2);
+	printk(KERN_DEBUG "hypercall test #2, ret: %d\n", ret);
 }
 
 int kvm_guest_register_para(int cpu)
Index: linux/drivers/kvm/vmx.c
===================================================================
--- linux.orig/drivers/kvm/vmx.c
+++ linux/drivers/kvm/vmx.c
@@ -1032,7 +1032,7 @@ static int vmcs_setup_cr3_cache(struct k
 	cr3_target_values = (msr_val >> 16) & ((1 << 10) - 1);
 	printk(KERN_DEBUG " cr3 target values: %d\n", cr3_target_values);
 	if (cr3_target_values > KVM_CR3_CACHE_SIZE) {
-		printk(KERN_WARN "KVM: limiting cr3 cache size from %d to %d\n",
+		printk(KERN_WARNING "KVM: limiting cr3 cache size from %d to %d\n",
 			cr3_target_values, KVM_CR3_CACHE_SIZE);
 		cr3_target_values = KVM_CR3_CACHE_SIZE;
 	}

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV