public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* RFC/patch: a very trivial patch towards portability
@ 2007-10-09 16:09 Carsten Otte
       [not found] ` <1191946167.7292.3.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Carsten Otte @ 2007-10-09 16:09 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

We've seen various big patches regarding portability from Christian and
Xianto, but none was merged to date. Maintaining a large patch set on
top of a quickly moving code base is painful.
I thought it might be cool to try to throw trivial patches at Avi that
obviously don't break things and push towards portability. In case this
succeeds I'll keep throwing in trivial patches until we've split
everything proper.
This patch splits kvm_dev_ioctl into architecture independent and
architecture dependent ioctls. Those that are arch independent remain in
kvm_main.c, others are implemented by kvm_arch_dev_ioctl() in kvm_x86.c.
A header file named kvm_arch.h is being introduced that contains
prototypes for funtions in kvm_x86.c.

Comments? Is this a preferable approach? What needs to be done
different?

signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
Index: kvm/drivers/kvm/kvm_arch.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/kvm_arch.h	2007-10-09 16:42:09.000000000 +0200
@@ -0,0 +1,17 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * This header defines architecture specific interfaces
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef KVM_ARCH_H
+#define KVM_ARCH_H
+long kvm_arch_dev_ioctl(struct file *,
+			unsigned int, unsigned long);
+
+__init void kvm_arch_init(void);
+#endif
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-10-09 15:39:26.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-10-09 16:41:04.000000000 +0200
@@ -16,6 +16,7 @@
  */
 
 #include "kvm.h"
+#include "kvm_arch.h"
 #include "x86_emulate.h"
 #include "segment_descriptor.h"
 #include "irq.h"
@@ -44,7 +45,6 @@
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/io.h>
-#include <asm/uaccess.h>
 #include <asm/desc.h>
 
 MODULE_AUTHOR("Qumranet");
@@ -2478,43 +2478,6 @@
 EXPORT_SYMBOL_GPL(kvm_get_cs_db_l_bits);
 
 /*
- * List of msr numbers which we expose to userspace through KVM_GET_MSRS
- * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
- *
- * This list is modified at module load time to reflect the
- * capabilities of the host cpu.
- */
-static u32 msrs_to_save[] = {
-	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
-	MSR_K6_STAR,
-#ifdef CONFIG_X86_64
-	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
-#endif
-	MSR_IA32_TIME_STAMP_COUNTER,
-};
-
-static unsigned num_msrs_to_save;
-
-static u32 emulated_msrs[] = {
-	MSR_IA32_MISC_ENABLE,
-};
-
-static __init void kvm_init_msr_list(void)
-{
-	u32 dummy[2];
-	unsigned i, j;
-
-	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
-		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
-			continue;
-		if (j < i)
-			msrs_to_save[j] = msrs_to_save[i];
-		j++;
-	}
-	num_msrs_to_save = j;
-}
-
-/*
  * Adapt set_msr() to msr_io()'s calling convention
  */
 static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
@@ -3292,7 +3255,6 @@
 static long kvm_dev_ioctl(struct file *filp,
 			  unsigned int ioctl, unsigned long arg)
 {
-	void __user *argp = (void __user *)arg;
 	long r = -EINVAL;
 
 	switch (ioctl) {
@@ -3308,56 +3270,8 @@
 			goto out;
 		r = kvm_dev_ioctl_create_vm();
 		break;
-	case KVM_GET_MSR_INDEX_LIST: {
-		struct kvm_msr_list __user *user_msr_list = argp;
-		struct kvm_msr_list msr_list;
-		unsigned n;
-
-		r = -EFAULT;
-		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
-			goto out;
-		n = msr_list.nmsrs;
-		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
-		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
-			goto out;
-		r = -E2BIG;
-		if (n < num_msrs_to_save)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
-				 num_msrs_to_save * sizeof(u32)))
-			goto out;
-		if (copy_to_user(user_msr_list->indices
-				 + num_msrs_to_save * sizeof(u32),
-				 &emulated_msrs,
-				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
-			goto out;
-		r = 0;
-		break;
-	}
-	case KVM_CHECK_EXTENSION: {
-		int ext = (long)argp;
-
-		switch (ext) {
-		case KVM_CAP_IRQCHIP:
-		case KVM_CAP_HLT:
-		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
-			r = 1;
-			break;
-		default:
-			r = 0;
-			break;
-		}
-		break;
-	}
-	case KVM_GET_VCPU_MMAP_SIZE:
-		r = -EINVAL;
-		if (arg)
-			goto out;
-		r = 2 * PAGE_SIZE;
-		break;
 	default:
-		;
+		return kvm_arch_dev_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
@@ -3721,7 +3635,7 @@
 
 	kvm_init_debug();
 
-	kvm_init_msr_list();
+	kvm_arch_init();
 
 	bad_page = alloc_page(GFP_KERNEL);
 
Index: kvm/drivers/kvm/kvm_x86.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/kvm_x86.c	2007-10-09 16:47:55.000000000 +0200
@@ -0,0 +1,126 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * derived from drivers/kvm/kvm_main.c
+ *
+ * Copyright (C) 2006 Qumranet, Inc.
+ *
+ * Authors:
+ *   Avi Kivity   <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *   Yaniv Kamay  <yaniv-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "kvm.h"
+#include "kvm_arch.h"
+
+#include <asm/uaccess.h>
+
+static u32 emulated_msrs[] = {
+	MSR_IA32_MISC_ENABLE,
+};
+
+static unsigned num_msrs_to_save;
+
+/*
+ * List of msr numbers which we expose to userspace through KVM_GET_MSRS
+ * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
+ *
+ * This list is modified at module load time to reflect the
+ * capabilities of the host cpu.
+ */
+static u32 msrs_to_save[] = {
+	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
+	MSR_K6_STAR,
+#ifdef CONFIG_X86_64
+	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
+#endif
+	MSR_IA32_TIME_STAMP_COUNTER,
+};
+
+
+long kvm_arch_dev_ioctl(struct file *filp,
+			unsigned int ioctl, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	long r;
+
+	switch (ioctl) {
+	case KVM_GET_MSR_INDEX_LIST: {
+		struct kvm_msr_list __user *user_msr_list = argp;
+		struct kvm_msr_list msr_list;
+		unsigned n;
+
+		r = -EFAULT;
+		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
+			goto out;
+		n = msr_list.nmsrs;
+		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
+		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
+			goto out;
+		r = -E2BIG;
+		if (n < num_msrs_to_save)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
+				 num_msrs_to_save * sizeof(u32)))
+			goto out;
+		if (copy_to_user(user_msr_list->indices
+				 + num_msrs_to_save * sizeof(u32),
+				 &emulated_msrs,
+				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_CHECK_EXTENSION: {
+		int ext = (long)argp;
+
+		switch (ext) {
+		case KVM_CAP_IRQCHIP:
+		case KVM_CAP_HLT:
+		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
+			r = 1;
+			break;
+		default:
+			r = 0;
+			break;
+		}
+		break;
+	}
+	case KVM_GET_VCPU_MMAP_SIZE: {
+		r = -EINVAL;
+		if (arg)
+			goto out;
+		r = 2 * PAGE_SIZE;
+		break;
+	}
+	default:
+		r = -EINVAL;
+	}
+out:
+	return r;
+}
+
+static __init void kvm_init_msr_list(void)
+{
+	u32 dummy[2];
+	unsigned i, j;
+
+	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
+		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
+			continue;
+		if (j < i)
+			msrs_to_save[j] = msrs_to_save[i];
+		j++;
+	}
+	num_msrs_to_save = j;
+}
+
+__init void kvm_arch_init(void)
+{
+	kvm_init_msr_list();
+}
Index: kvm/drivers/kvm/Makefile
===================================================================
--- kvm.orig/drivers/kvm/Makefile	2007-10-09 18:17:03.000000000 +0200
+++ kvm/drivers/kvm/Makefile	2007-10-09 18:17:12.000000000 +0200
@@ -2,7 +2,7 @@
 # Makefile for Kernel-based Virtual Machine module
 #
 
-kvm-objs := kvm_main.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
+kvm-objs := kvm_main.o kvm_x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability
       [not found] ` <1191946167.7292.3.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
@ 2007-10-09 16:17   ` Avi Kivity
       [not found]     ` <470BA98A.8090900-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  2007-10-09 16:28   ` Avi Kivity
  2007-10-10 12:37   ` RFC/patch: a very trivial patch towards portability V2 Carsten Otte
  2 siblings, 1 reply; 13+ messages in thread
From: Avi Kivity @ 2007-10-09 16:17 UTC (permalink / raw)
  To: Carsten Otte; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Carsten Otte wrote:
> We've seen various big patches regarding portability from Christian and
> Xianto, but none was merged to date. Maintaining a large patch set on
> top of a quickly moving code base is painful.
> I thought it might be cool to try to throw trivial patches at Avi that
> obviously don't break things and push towards portability. In case this
> succeeds I'll keep throwing in trivial patches until we've split
> everything proper.
> This patch splits kvm_dev_ioctl into architecture independent and
> architecture dependent ioctls. Those that are arch independent remain in
> kvm_main.c, others are implemented by kvm_arch_dev_ioctl() in kvm_x86.c.
> A header file named kvm_arch.h is being introduced that contains
> prototypes for funtions in kvm_x86.c.
>
> Comments? Is this a preferable approach? What needs to be done
> different?
>
>   

Small, reviewable, posted patches are definitely the best way forward.

> -	case KVM_CHECK_EXTENSION: {
> -		int ext = (long)argp;
> -
> -		switch (ext) {
> -		case KVM_CAP_IRQCHIP:
> -		case KVM_CAP_HLT:
> -		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
> -			r = 1;
> -			break;
> -		default:
> -			r = 0;
> -			break;
> -		}
> -		break;
> -	}
>   

CHECK_EXTENSION is hopefully a generic mechanism (even if the some of 
the actual extensions are not).  So there should be a switch in common 
code for the common extensions, and the default: target should call 
kvm_arch_check_extension() for further processing.

> -	case KVM_GET_VCPU_MMAP_SIZE:
> -		r = -EINVAL;
> -		if (arg)
> -			goto out;
> -		r = 2 * PAGE_SIZE;
> -		break;
>   

I would think this is generic too?  Isn't s390 interested in passing 
information to userspace via a mmap()ed region?

Note that mmio data is passed via that region.


>  
> Index: kvm/drivers/kvm/kvm_x86.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ kvm/drivers/kvm/kvm_x86.c	2007-10-09 16:47:55.000000000 +0200
>   

x86.c


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability
       [not found] ` <1191946167.7292.3.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  2007-10-09 16:17   ` Avi Kivity
@ 2007-10-09 16:28   ` Avi Kivity
       [not found]     ` <470BAC2B.40201-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  2007-10-10 12:37   ` RFC/patch: a very trivial patch towards portability V2 Carsten Otte
  2 siblings, 1 reply; 13+ messages in thread
From: Avi Kivity @ 2007-10-09 16:28 UTC (permalink / raw)
  To: Carsten Otte; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Carsten Otte wrote:
> A header file named kvm_arch.h is being introduced that contains
> prototypes for funtions in kvm_x86.c.
>
>   

What's the motivation for the new header?  So we have a list of 
arch-dependent functions?  Compiler-wise it could just as well remain in 
kvm.h.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability
       [not found]     ` <470BA98A.8090900-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2007-10-10  8:28       ` Carsten Otte
  0 siblings, 0 replies; 13+ messages in thread
From: Carsten Otte @ 2007-10-10  8:28 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Avi Kivity wrote:
> Small, reviewable, posted patches are definitely the best way forward.
Very well, will go that direction.
> 
>> -    case KVM_CHECK_EXTENSION: {
>> -        int ext = (long)argp;
>> -
>> -        switch (ext) {
>> -        case KVM_CAP_IRQCHIP:
>> -        case KVM_CAP_HLT:
>> -        case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
>> -            r = 1;
>> -            break;
>> -        default:
>> -            r = 0;
>> -            break;
>> -        }
>> -        break;
>> -    }
>>   
> 
> CHECK_EXTENSION is hopefully a generic mechanism (even if the some of 
> the actual extensions are not).  So there should be a switch in common 
> code for the common extensions, and the default: target should call 
> kvm_arch_check_extension() for further processing.
Agreed, the call itself should be generic. But the result is arch 
dependent. Will fix it.
> 
>> -    case KVM_GET_VCPU_MMAP_SIZE:
>> -        r = -EINVAL;
>> -        if (arg)
>> -            goto out;
>> -        r = 2 * PAGE_SIZE;
>> -        break;
>>   
> 
> I would think this is generic too?  Isn't s390 interested in passing 
> information to userspace via a mmap()ed region?
Yes, same as above: call is generic - result value not. Will fix this too.
> Note that mmio data is passed via that region.
Yes, I've seen that. Thanks for the heads-up.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability
       [not found]     ` <470BAC2B.40201-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2007-10-10  8:30       ` Carsten Otte
  0 siblings, 0 replies; 13+ messages in thread
From: Carsten Otte @ 2007-10-10  8:30 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Avi Kivity wrote:
> What's the motivation for the new header?  So we have a list of 
> arch-dependent functions?  Compiler-wise it could just as well remain in 
> kvm.h.
The motivation for a new header, is that it contains definitions 
specific to an architecture. These prototypes should go to kvm.h, they 
are common for all archs. Will fix it.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RFC/patch: a very trivial patch towards portability V2
       [not found] ` <1191946167.7292.3.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  2007-10-09 16:17   ` Avi Kivity
  2007-10-09 16:28   ` Avi Kivity
@ 2007-10-10 12:37   ` Carsten Otte
       [not found]     ` <1192019827.17745.9.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  2 siblings, 1 reply; 13+ messages in thread
From: Carsten Otte @ 2007-10-10 12:37 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Thanks to Avis review, this is an updated version of the patch that
splits kvm_dev_ioctl into architecture dependent and architecture
independent parts. I've changed everything Avi noted in his review
feedback. It is also rebased to fit on today's git with all that
reindentation. 
KVM_CHECK_EXTENSION and KVM_GET_VCPU_MMAP_SIZE are now implemented in
the common part. For now, kvm_main.c includes x86.h. After the split, I
think we should move that file into include/asm-arch/ so that kvm_main.c
automagically includes the correct header. Until we've actually got
another arch merged upstream, I think it is more convenient to have the
file in drivers/kvm so that we only need to convince Avi when changing
it.

signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
Index: kvm/drivers/kvm/kvm_x86.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/kvm_x86.h	2007-10-10 14:16:47.000000000 +0200
@@ -0,0 +1,15 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * This header defines architecture specific interfaces, x86 version
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef KVM_X86_H
+#define KVM_X86_H
+
+#define KVM_ARCH_VCPU_MMAP_SIZE (2 * PAGE_SIZE)
+#endif
Index: kvm/drivers/kvm/kvm.h
===================================================================
--- kvm.orig/drivers/kvm/kvm.h	2007-10-10 12:42:21.000000000 +0200
+++ kvm/drivers/kvm/kvm.h	2007-10-10 14:12:30.000000000 +0200
@@ -653,6 +653,11 @@
 
 int kvm_fix_hypercall(struct kvm_vcpu *vcpu);
 
+long kvm_arch_dev_ioctl(struct file *,
+			unsigned int, unsigned long);
+__init void kvm_arch_init(void);
+long kvm_arch_check_extension(int);
+
 static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
 				     u32 error_code)
 {
Index: kvm/drivers/kvm/Makefile
===================================================================
--- kvm.orig/drivers/kvm/Makefile	2007-10-10 11:35:01.000000000 +0200
+++ kvm/drivers/kvm/Makefile	2007-10-10 13:44:02.000000000 +0200
@@ -2,7 +2,7 @@
 # Makefile for Kernel-based Virtual Machine module
 #
 
-kvm-objs := kvm_main.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
+kvm-objs := kvm_main.o kvm_x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-10-10 11:34:07.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-10-10 14:17:08.000000000 +0200
@@ -16,6 +16,7 @@
  */
 
 #include "kvm.h"
+#include "kvm_x86.h"
 #include "x86_emulate.h"
 #include "segment_descriptor.h"
 #include "irq.h"
@@ -45,7 +46,6 @@
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/io.h>
-#include <asm/uaccess.h>
 #include <asm/desc.h>
 
 MODULE_AUTHOR("Qumranet");
@@ -2518,43 +2518,6 @@
 EXPORT_SYMBOL_GPL(kvm_get_cs_db_l_bits);
 
 /*
- * List of msr numbers which we expose to userspace through KVM_GET_MSRS
- * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
- *
- * This list is modified at module load time to reflect the
- * capabilities of the host cpu.
- */
-static u32 msrs_to_save[] = {
-	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
-	MSR_K6_STAR,
-#ifdef CONFIG_X86_64
-	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
-#endif
-	MSR_IA32_TIME_STAMP_COUNTER,
-};
-
-static unsigned num_msrs_to_save;
-
-static u32 emulated_msrs[] = {
-	MSR_IA32_MISC_ENABLE,
-};
-
-static __init void kvm_init_msr_list(void)
-{
-	u32 dummy[2];
-	unsigned i, j;
-
-	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
-		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
-			continue;
-		if (j < i)
-			msrs_to_save[j] = msrs_to_save[i];
-		j++;
-	}
-	num_msrs_to_save = j;
-}
-
-/*
  * Adapt set_msr() to msr_io()'s calling convention
  */
 static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
@@ -3366,57 +3329,20 @@
 			goto out;
 		r = kvm_dev_ioctl_create_vm();
 		break;
-	case KVM_GET_MSR_INDEX_LIST: {
-		struct kvm_msr_list __user *user_msr_list = argp;
-		struct kvm_msr_list msr_list;
-		unsigned n;
-
-		r = -EFAULT;
-		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
-			goto out;
-		n = msr_list.nmsrs;
-		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
-		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
-			goto out;
-		r = -E2BIG;
-		if (n < num_msrs_to_save)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
-				 num_msrs_to_save * sizeof(u32)))
-			goto out;
-		if (copy_to_user(user_msr_list->indices
-				 + num_msrs_to_save * sizeof(u32),
-				 &emulated_msrs,
-				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
-			goto out;
-		r = 0;
-		break;
-	}
 	case KVM_CHECK_EXTENSION: {
 		int ext = (long)argp;
 
-		switch (ext) {
-		case KVM_CAP_IRQCHIP:
-		case KVM_CAP_HLT:
-		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
-		case KVM_CAP_USER_MEMORY:
-			r = 1;
-			break;
-		default:
-			r = 0;
-			break;
-		}
+		r = kvm_arch_check_extension(ext);
 		break;
 	}
 	case KVM_GET_VCPU_MMAP_SIZE:
 		r = -EINVAL;
 		if (arg)
 			goto out;
-		r = 2 * PAGE_SIZE;
+		r = KVM_ARCH_VCPU_MMAP_SIZE;
 		break;
 	default:
-		;
+		return kvm_arch_dev_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
@@ -3780,7 +3706,7 @@
 
 	kvm_init_debug();
 
-	kvm_init_msr_list();
+	kvm_arch_init();
 
 	bad_page = alloc_page(GFP_KERNEL);
 
Index: kvm/drivers/kvm/kvm_x86.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/kvm_x86.c	2007-10-10 14:19:54.000000000 +0200
@@ -0,0 +1,115 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * derived from drivers/kvm/kvm_main.c
+ *
+ * Copyright (C) 2006 Qumranet, Inc.
+ *
+ * Authors:
+ *   Avi Kivity   <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *   Yaniv Kamay  <yaniv-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "kvm.h"
+
+#include <asm/uaccess.h>
+
+/*
+ * List of msr numbers which we expose to userspace through KVM_GET_MSRS
+ * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
+ *
+ * This list is modified at module load time to reflect the
+ * capabilities of the host cpu.
+ */
+static u32 msrs_to_save[] = {
+	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
+	MSR_K6_STAR,
+#ifdef CONFIG_X86_64
+	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
+#endif
+	MSR_IA32_TIME_STAMP_COUNTER,
+};
+
+static unsigned num_msrs_to_save;
+
+static u32 emulated_msrs[] = {
+	MSR_IA32_MISC_ENABLE,
+};
+
+long kvm_arch_dev_ioctl(struct file *filp,
+			unsigned int ioctl, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	long r;
+
+	switch (ioctl) {
+	case KVM_GET_MSR_INDEX_LIST: {
+		struct kvm_msr_list __user *user_msr_list = argp;
+		struct kvm_msr_list msr_list;
+		unsigned n;
+
+		r = -EFAULT;
+		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
+			goto out;
+		n = msr_list.nmsrs;
+		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
+		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
+			goto out;
+		r = -E2BIG;
+		if (n < num_msrs_to_save)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
+				 num_msrs_to_save * sizeof(u32)))
+			goto out;
+		if (copy_to_user(user_msr_list->indices
+				 + num_msrs_to_save * sizeof(u32),
+				 &emulated_msrs,
+				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
+			goto out;
+		r = 0;
+		break;
+	}
+	default:
+		r = -EINVAL;
+	}
+out:
+	return r;
+}
+
+long kvm_arch_check_extension(int ext)
+{
+	switch (ext) {
+	case KVM_CAP_IRQCHIP:
+	case KVM_CAP_HLT:
+	case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
+	case KVM_CAP_USER_MEMORY:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static __init void kvm_init_msr_list(void)
+{
+	u32 dummy[2];
+	unsigned i, j;
+
+	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
+		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
+			continue;
+		if (j < i)
+			msrs_to_save[j] = msrs_to_save[i];
+		j++;
+	}
+	num_msrs_to_save = j;
+}
+
+__init void kvm_arch_init(void)
+{
+	kvm_init_msr_list();
+}



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RFC/patch: a very trivial patch towards portability V2.1
       [not found]     ` <1192019827.17745.9.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
@ 2007-10-10 12:45       ` Carsten Otte
       [not found]         ` <1192020355.17877.4.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Carsten Otte @ 2007-10-10 12:45 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Christian Borntraeger pointed out that Avi requested to rename kvm_x86.c
to x86.c. The patch below is the same as V2, but the files kvm_x86.[ch]
have been renamed to x86.[ch], and the #includes have been updated.

signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
Index: kvm/drivers/kvm/kvm.h
===================================================================
--- kvm.orig/drivers/kvm/kvm.h	2007-10-10 12:42:21.000000000 +0200
+++ kvm/drivers/kvm/kvm.h	2007-10-10 14:12:30.000000000 +0200
@@ -653,6 +653,11 @@
 
 int kvm_fix_hypercall(struct kvm_vcpu *vcpu);
 
+long kvm_arch_dev_ioctl(struct file *,
+			unsigned int, unsigned long);
+__init void kvm_arch_init(void);
+long kvm_arch_check_extension(int);
+
 static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
 				     u32 error_code)
 {
Index: kvm/drivers/kvm/Makefile
===================================================================
--- kvm.orig/drivers/kvm/Makefile	2007-10-10 11:35:01.000000000 +0200
+++ kvm/drivers/kvm/Makefile	2007-10-10 14:36:36.000000000 +0200
@@ -2,7 +2,7 @@
 # Makefile for Kernel-based Virtual Machine module
 #
 
-kvm-objs := kvm_main.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
+kvm-objs := kvm_main.o x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-10-10 11:34:07.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-10-10 14:36:28.000000000 +0200
@@ -16,6 +16,7 @@
  */
 
 #include "kvm.h"
+#include "x86.h"
 #include "x86_emulate.h"
 #include "segment_descriptor.h"
 #include "irq.h"
@@ -45,7 +46,6 @@
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/io.h>
-#include <asm/uaccess.h>
 #include <asm/desc.h>
 
 MODULE_AUTHOR("Qumranet");
@@ -2518,43 +2518,6 @@
 EXPORT_SYMBOL_GPL(kvm_get_cs_db_l_bits);
 
 /*
- * List of msr numbers which we expose to userspace through KVM_GET_MSRS
- * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
- *
- * This list is modified at module load time to reflect the
- * capabilities of the host cpu.
- */
-static u32 msrs_to_save[] = {
-	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
-	MSR_K6_STAR,
-#ifdef CONFIG_X86_64
-	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
-#endif
-	MSR_IA32_TIME_STAMP_COUNTER,
-};
-
-static unsigned num_msrs_to_save;
-
-static u32 emulated_msrs[] = {
-	MSR_IA32_MISC_ENABLE,
-};
-
-static __init void kvm_init_msr_list(void)
-{
-	u32 dummy[2];
-	unsigned i, j;
-
-	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
-		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
-			continue;
-		if (j < i)
-			msrs_to_save[j] = msrs_to_save[i];
-		j++;
-	}
-	num_msrs_to_save = j;
-}
-
-/*
  * Adapt set_msr() to msr_io()'s calling convention
  */
 static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
@@ -3366,57 +3329,20 @@
 			goto out;
 		r = kvm_dev_ioctl_create_vm();
 		break;
-	case KVM_GET_MSR_INDEX_LIST: {
-		struct kvm_msr_list __user *user_msr_list = argp;
-		struct kvm_msr_list msr_list;
-		unsigned n;
-
-		r = -EFAULT;
-		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
-			goto out;
-		n = msr_list.nmsrs;
-		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
-		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
-			goto out;
-		r = -E2BIG;
-		if (n < num_msrs_to_save)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
-				 num_msrs_to_save * sizeof(u32)))
-			goto out;
-		if (copy_to_user(user_msr_list->indices
-				 + num_msrs_to_save * sizeof(u32),
-				 &emulated_msrs,
-				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
-			goto out;
-		r = 0;
-		break;
-	}
 	case KVM_CHECK_EXTENSION: {
 		int ext = (long)argp;
 
-		switch (ext) {
-		case KVM_CAP_IRQCHIP:
-		case KVM_CAP_HLT:
-		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
-		case KVM_CAP_USER_MEMORY:
-			r = 1;
-			break;
-		default:
-			r = 0;
-			break;
-		}
+		r = kvm_arch_check_extension(ext);
 		break;
 	}
 	case KVM_GET_VCPU_MMAP_SIZE:
 		r = -EINVAL;
 		if (arg)
 			goto out;
-		r = 2 * PAGE_SIZE;
+		r = KVM_ARCH_VCPU_MMAP_SIZE;
 		break;
 	default:
-		;
+		return kvm_arch_dev_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
@@ -3780,7 +3706,7 @@
 
 	kvm_init_debug();
 
-	kvm_init_msr_list();
+	kvm_arch_init();
 
 	bad_page = alloc_page(GFP_KERNEL);
 
Index: kvm/drivers/kvm/x86.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/x86.c	2007-10-10 14:19:54.000000000 +0200
@@ -0,0 +1,115 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * derived from drivers/kvm/kvm_main.c
+ *
+ * Copyright (C) 2006 Qumranet, Inc.
+ *
+ * Authors:
+ *   Avi Kivity   <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *   Yaniv Kamay  <yaniv-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "kvm.h"
+
+#include <asm/uaccess.h>
+
+/*
+ * List of msr numbers which we expose to userspace through KVM_GET_MSRS
+ * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
+ *
+ * This list is modified at module load time to reflect the
+ * capabilities of the host cpu.
+ */
+static u32 msrs_to_save[] = {
+	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
+	MSR_K6_STAR,
+#ifdef CONFIG_X86_64
+	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
+#endif
+	MSR_IA32_TIME_STAMP_COUNTER,
+};
+
+static unsigned num_msrs_to_save;
+
+static u32 emulated_msrs[] = {
+	MSR_IA32_MISC_ENABLE,
+};
+
+long kvm_arch_dev_ioctl(struct file *filp,
+			unsigned int ioctl, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	long r;
+
+	switch (ioctl) {
+	case KVM_GET_MSR_INDEX_LIST: {
+		struct kvm_msr_list __user *user_msr_list = argp;
+		struct kvm_msr_list msr_list;
+		unsigned n;
+
+		r = -EFAULT;
+		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
+			goto out;
+		n = msr_list.nmsrs;
+		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
+		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
+			goto out;
+		r = -E2BIG;
+		if (n < num_msrs_to_save)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
+				 num_msrs_to_save * sizeof(u32)))
+			goto out;
+		if (copy_to_user(user_msr_list->indices
+				 + num_msrs_to_save * sizeof(u32),
+				 &emulated_msrs,
+				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
+			goto out;
+		r = 0;
+		break;
+	}
+	default:
+		r = -EINVAL;
+	}
+out:
+	return r;
+}
+
+long kvm_arch_check_extension(int ext)
+{
+	switch (ext) {
+	case KVM_CAP_IRQCHIP:
+	case KVM_CAP_HLT:
+	case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
+	case KVM_CAP_USER_MEMORY:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static __init void kvm_init_msr_list(void)
+{
+	u32 dummy[2];
+	unsigned i, j;
+
+	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
+		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
+			continue;
+		if (j < i)
+			msrs_to_save[j] = msrs_to_save[i];
+		j++;
+	}
+	num_msrs_to_save = j;
+}
+
+__init void kvm_arch_init(void)
+{
+	kvm_init_msr_list();
+}
Index: kvm/drivers/kvm/x86.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/x86.h	2007-10-10 14:16:47.000000000 +0200
@@ -0,0 +1,15 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * This header defines architecture specific interfaces, x86 version
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef KVM_X86_H
+#define KVM_X86_H
+
+#define KVM_ARCH_VCPU_MMAP_SIZE (2 * PAGE_SIZE)
+#endif



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability V2.1
       [not found]         ` <1192020355.17877.4.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
@ 2007-10-10 13:21           ` Avi Kivity
       [not found]             ` <470CD1F7.1030400-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  2007-10-10 15:16           ` RFC/patch: a very trivial patch towards portability V3 Carsten Otte
  1 sibling, 1 reply; 13+ messages in thread
From: Avi Kivity @ 2007-10-10 13:21 UTC (permalink / raw)
  To: Carsten Otte; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Carsten Otte wrote:
> Christian Borntraeger pointed out that Avi requested to rename kvm_x86.c
> to x86.c. The patch below is the same as V2, but the files kvm_x86.[ch]
> have been renamed to x86.[ch], and the #includes have been updated.
>
> signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> ---
> Index: kvm/drivers/kvm/kvm.h
> ===================================================================
> --- kvm.orig/drivers/kvm/kvm.h	2007-10-10 12:42:21.000000000 +0200
> +++ kvm/drivers/kvm/kvm.h	2007-10-10 14:12:30.000000000 +0200
> @@ -653,6 +653,11 @@
>  
>  int kvm_fix_hypercall(struct kvm_vcpu *vcpu);
>  
> +long kvm_arch_dev_ioctl(struct file *,
> +			unsigned int, unsigned long);
> +__init void kvm_arch_init(void);
> +long kvm_arch_check_extension(int);
>   

Argument names in prototypes please.

>  	case KVM_CHECK_EXTENSION: {
>  		int ext = (long)argp;
>  
> -		switch (ext) {
> -		case KVM_CAP_IRQCHIP:
> -		case KVM_CAP_HLT:
> -		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
> -		case KVM_CAP_USER_MEMORY:
> -			r = 1;
> -			break;
> -		default:
> -			r = 0;
> -			break;
> -		}
> +		r = kvm_arch_check_extension(ext);
>  		break;
>
>   


Reflecting some more, do we want different API availablility for 
different archs?  We can make this generic, and ignore arch-specific 
extensions.

But on the other hand, things like KVM_CAP_IRQCHIP _are_ applicable to 
powerpc, but may not be implemented from day 1... should we do

case KVM_CAP_IRQCHIP:
    return KVM_ARCH_HAS_IRQCHIP;

?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability V2.1
       [not found]             ` <470CD1F7.1030400-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2007-10-10 13:59               ` Carsten Otte
  0 siblings, 0 replies; 13+ messages in thread
From: Carsten Otte @ 2007-10-10 13:59 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Avi Kivity wrote:
> Argument names in prototypes please.
Will do that.

>>  	case KVM_CHECK_EXTENSION: {
>>  		int ext = (long)argp;
>>  
>> -		switch (ext) {
>> -		case KVM_CAP_IRQCHIP:
>> -		case KVM_CAP_HLT:
>> -		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
>> -		case KVM_CAP_USER_MEMORY:
>> -			r = 1;
>> -			break;
>> -		default:
>> -			r = 0;
>> -			break;
>> -		}
>> +		r = kvm_arch_check_extension(ext);
>>  		break;
>>
>>   
> 
> 
> Reflecting some more, do we want different API availablility for 
> different archs?  We can make this generic, and ignore arch-specific 
> extensions.
> 
> But on the other hand, things like KVM_CAP_IRQCHIP _are_ applicable to 
> powerpc, but may not be implemented from day 1... should we do
> 
> case KVM_CAP_IRQCHIP:
>     return KVM_ARCH_HAS_IRQCHIP;
> 
> ?
Will do that too.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RFC/patch: a very trivial patch towards portability V3
       [not found]         ` <1192020355.17877.4.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  2007-10-10 13:21           ` Avi Kivity
@ 2007-10-10 15:16           ` Carsten Otte
       [not found]             ` <1192029379.18392.7.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  1 sibling, 1 reply; 13+ messages in thread
From: Carsten Otte @ 2007-10-10 15:16 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Thanks to Avi's continued review, we've got even more common code this
time: KVM_CHECK_EXTENSION ioctl is now completely handled in kvm_main.c
instead of having arch callbacks to check extensions. The architectures
are expected to setup a bit mask named KVM_ARCH_EXTENSIONS with
information about capabilities they support. Possible valus for the bit
masks are defined in drivers/kvm/kvm.h (KVM_ARCH_HAS_*), which rely on
KVM_CAP_* definitions in include/linux/kvm.h.
Function prototypes in drivers/kvm/kvm.h have been fixed: they have
argument names now.

signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
Index: kvm/drivers/kvm/kvm.h
===================================================================
--- kvm.orig/drivers/kvm/kvm.h	2007-10-10 12:42:21.000000000 +0200
+++ kvm/drivers/kvm/kvm.h	2007-10-10 17:00:12.000000000 +0200
@@ -64,6 +64,12 @@
 
 #define KVM_PIO_PAGE_OFFSET 1
 
+#define KVM_ARCH_HAS_IRQCHIP                  (1ull << KVM_CAP_IRQCHIP)
+#define KVM_ARCH_HAS_HLT	              (1ull << KVM_CAP_HLT)
+#define KVM_ARCH_HAS_MMU_SHADOW_CACHE_CONTROL \ 
+        (1ull << KVM_CAP_MMU_SHADOW_CACHE_CONTROL)
+#define KVM_ARCH_HAS_USER_MEMORY              (1ull << KVM_CAP_USER_MEMORY)
+
 /*
  * vcpu->requests bit members
  */
@@ -653,6 +659,10 @@
 
 int kvm_fix_hypercall(struct kvm_vcpu *vcpu);
 
+long kvm_arch_dev_ioctl(struct file *filp,
+			unsigned int ioctl, unsigned long arg);
+__init void kvm_arch_init(void);
+
 static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
 				     u32 error_code)
 {
Index: kvm/drivers/kvm/Makefile
===================================================================
--- kvm.orig/drivers/kvm/Makefile	2007-10-10 11:35:01.000000000 +0200
+++ kvm/drivers/kvm/Makefile	2007-10-10 14:36:36.000000000 +0200
@@ -2,7 +2,7 @@
 # Makefile for Kernel-based Virtual Machine module
 #
 
-kvm-objs := kvm_main.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
+kvm-objs := kvm_main.o x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-10-10 11:34:07.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-10-10 16:57:05.000000000 +0200
@@ -16,6 +16,7 @@
  */
 
 #include "kvm.h"
+#include "x86.h"
 #include "x86_emulate.h"
 #include "segment_descriptor.h"
 #include "irq.h"
@@ -45,7 +46,6 @@
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/io.h>
-#include <asm/uaccess.h>
 #include <asm/desc.h>
 
 MODULE_AUTHOR("Qumranet");
@@ -2518,43 +2518,6 @@
 EXPORT_SYMBOL_GPL(kvm_get_cs_db_l_bits);
 
 /*
- * List of msr numbers which we expose to userspace through KVM_GET_MSRS
- * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
- *
- * This list is modified at module load time to reflect the
- * capabilities of the host cpu.
- */
-static u32 msrs_to_save[] = {
-	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
-	MSR_K6_STAR,
-#ifdef CONFIG_X86_64
-	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
-#endif
-	MSR_IA32_TIME_STAMP_COUNTER,
-};
-
-static unsigned num_msrs_to_save;
-
-static u32 emulated_msrs[] = {
-	MSR_IA32_MISC_ENABLE,
-};
-
-static __init void kvm_init_msr_list(void)
-{
-	u32 dummy[2];
-	unsigned i, j;
-
-	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
-		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
-			continue;
-		if (j < i)
-			msrs_to_save[j] = msrs_to_save[i];
-		j++;
-	}
-	num_msrs_to_save = j;
-}
-
-/*
  * Adapt set_msr() to msr_io()'s calling convention
  */
 static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
@@ -3366,57 +3329,25 @@
 			goto out;
 		r = kvm_dev_ioctl_create_vm();
 		break;
-	case KVM_GET_MSR_INDEX_LIST: {
-		struct kvm_msr_list __user *user_msr_list = argp;
-		struct kvm_msr_list msr_list;
-		unsigned n;
-
-		r = -EFAULT;
-		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
-			goto out;
-		n = msr_list.nmsrs;
-		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
-		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
-			goto out;
-		r = -E2BIG;
-		if (n < num_msrs_to_save)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
-				 num_msrs_to_save * sizeof(u32)))
-			goto out;
-		if (copy_to_user(user_msr_list->indices
-				 + num_msrs_to_save * sizeof(u32),
-				 &emulated_msrs,
-				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
-			goto out;
-		r = 0;
-		break;
-	}
 	case KVM_CHECK_EXTENSION: {
 		int ext = (long)argp;
 
-		switch (ext) {
-		case KVM_CAP_IRQCHIP:
-		case KVM_CAP_HLT:
-		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
-		case KVM_CAP_USER_MEMORY:
-			r = 1;
-			break;
-		default:
-			r = 0;
+		r = 0;
+		if ((ext < 0) || (ext > sizeof(unsigned long long) - 1)) {
 			break;
 		}
+		if (KVM_ARCH_EXTENSIONS & (1ull << ext))
+			r = 1;
 		break;
 	}
 	case KVM_GET_VCPU_MMAP_SIZE:
 		r = -EINVAL;
 		if (arg)
 			goto out;
-		r = 2 * PAGE_SIZE;
+		r = KVM_ARCH_VCPU_MMAP_SIZE;
 		break;
 	default:
-		;
+		return kvm_arch_dev_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
@@ -3780,7 +3711,7 @@
 
 	kvm_init_debug();
 
-	kvm_init_msr_list();
+	kvm_arch_init();
 
 	bad_page = alloc_page(GFP_KERNEL);
 
Index: kvm/drivers/kvm/x86.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/x86.c	2007-10-10 17:00:13.000000000 +0200
@@ -0,0 +1,102 @@
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * derived from drivers/kvm/kvm_main.c
+ *
+ * Copyright (C) 2006 Qumranet, Inc.
+ *
+ * Authors:
+ *   Avi Kivity   <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *   Yaniv Kamay  <yaniv-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "kvm.h"
+
+#include <asm/uaccess.h>
+
+/*
+ * List of msr numbers which we expose to userspace through KVM_GET_MSRS
+ * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
+ *
+ * This list is modified at module load time to reflect the
+ * capabilities of the host cpu.
+ */
+static u32 msrs_to_save[] = {
+	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
+	MSR_K6_STAR,
+#ifdef CONFIG_X86_64
+	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
+#endif
+	MSR_IA32_TIME_STAMP_COUNTER,
+};
+
+static unsigned num_msrs_to_save;
+
+static u32 emulated_msrs[] = {
+	MSR_IA32_MISC_ENABLE,
+};
+
+long kvm_arch_dev_ioctl(struct file *filp,
+			unsigned int ioctl, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	long r;
+
+	switch (ioctl) {
+	case KVM_GET_MSR_INDEX_LIST: {
+		struct kvm_msr_list __user *user_msr_list = argp;
+		struct kvm_msr_list msr_list;
+		unsigned n;
+
+		r = -EFAULT;
+		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
+			goto out;
+		n = msr_list.nmsrs;
+		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
+		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
+			goto out;
+		r = -E2BIG;
+		if (n < num_msrs_to_save)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
+				 num_msrs_to_save * sizeof(u32)))
+			goto out;
+		if (copy_to_user(user_msr_list->indices
+				 + num_msrs_to_save * sizeof(u32),
+				 &emulated_msrs,
+				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
+			goto out;
+		r = 0;
+		break;
+	}
+	default:
+		r = -EINVAL;
+	}
+out:
+	return r;
+}
+
+static __init void kvm_init_msr_list(void)
+{
+	u32 dummy[2];
+	unsigned i, j;
+
+	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
+		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
+			continue;
+		if (j < i)
+			msrs_to_save[j] = msrs_to_save[i];
+		j++;
+	}
+	num_msrs_to_save = j;
+}
+
+__init void kvm_arch_init(void)
+{
+	kvm_init_msr_list();
+}
Index: kvm/drivers/kvm/x86.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/drivers/kvm/x86.h	2007-10-10 16:47:02.000000000 +0200
@@ -0,0 +1,22 @@
+#/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * This header defines architecture specific interfaces, x86 version
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef KVM_X86_H
+#define KVM_X86_H
+
+#include "kvm.h"
+
+#define KVM_ARCH_VCPU_MMAP_SIZE (2 * PAGE_SIZE)
+
+#define KVM_ARCH_EXTENSIONS (KVM_ARCH_HAS_IRQCHIP & \
+                             KVM_ARCH_HAS_HLT & \
+                             KVM_ARCH_HAS_MMU_SHADOW_CACHE_CONTROL & \
+                             KVM_ARCH_HAS_USER_MEMORY)
+#endif



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability V3
       [not found]             ` <1192029379.18392.7.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
@ 2007-10-11  9:15               ` Christian Ehrhardt
  2007-10-11 11:09               ` Avi Kivity
  1 sibling, 0 replies; 13+ messages in thread
From: Christian Ehrhardt @ 2007-10-11  9:15 UTC (permalink / raw)
  To: Carsten Otte,
	kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

To give a statement from other architectures I wanted to say that this patch is fine for the power port.
>From my view it is the small and easy-to-review flavor of the big patch series I once sent to the list.
And because Carsten and I sit in the same office these patches are usually reviewed/discussed from the power perspective before they are submitted.
Applying these patches now (I expect that Carsten will submit more) will reduce the size of the patches that we eventually need integrate our architecture.

Grüsse / regards,
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Carsten Otte wrote:
> Thanks to Avi's continued review, we've got even more common code this
> time: KVM_CHECK_EXTENSION ioctl is now completely handled in kvm_main.c
> instead of having arch callbacks to check extensions. The architectures
> are expected to setup a bit mask named KVM_ARCH_EXTENSIONS with
> information about capabilities they support. Possible valus for the bit
> masks are defined in drivers/kvm/kvm.h (KVM_ARCH_HAS_*), which rely on
> KVM_CAP_* definitions in include/linux/kvm.h.
> Function prototypes in drivers/kvm/kvm.h have been fixed: they have
> argument names now.
> 
> signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> ---
> Index: kvm/drivers/kvm/kvm.h
> ===================================================================
> --- kvm.orig/drivers/kvm/kvm.h	2007-10-10 12:42:21.000000000 +0200
> +++ kvm/drivers/kvm/kvm.h	2007-10-10 17:00:12.000000000 +0200
> @@ -64,6 +64,12 @@
> 
>  #define KVM_PIO_PAGE_OFFSET 1
> 
> +#define KVM_ARCH_HAS_IRQCHIP                  (1ull << KVM_CAP_IRQCHIP)
> +#define KVM_ARCH_HAS_HLT	              (1ull << KVM_CAP_HLT)
> +#define KVM_ARCH_HAS_MMU_SHADOW_CACHE_CONTROL \ 
> +        (1ull << KVM_CAP_MMU_SHADOW_CACHE_CONTROL)
> +#define KVM_ARCH_HAS_USER_MEMORY              (1ull << KVM_CAP_USER_MEMORY)
> +
>  /*
>   * vcpu->requests bit members
>   */
> @@ -653,6 +659,10 @@
> 
>  int kvm_fix_hypercall(struct kvm_vcpu *vcpu);
> 
> +long kvm_arch_dev_ioctl(struct file *filp,
> +			unsigned int ioctl, unsigned long arg);
> +__init void kvm_arch_init(void);
> +
>  static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
>  				     u32 error_code)
>  {
> Index: kvm/drivers/kvm/Makefile
> ===================================================================
> --- kvm.orig/drivers/kvm/Makefile	2007-10-10 11:35:01.000000000 +0200
> +++ kvm/drivers/kvm/Makefile	2007-10-10 14:36:36.000000000 +0200
> @@ -2,7 +2,7 @@
>  # Makefile for Kernel-based Virtual Machine module
>  #
> 
> -kvm-objs := kvm_main.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
> +kvm-objs := kvm_main.o x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o ioapic.o
>  obj-$(CONFIG_KVM) += kvm.o
>  kvm-intel-objs = vmx.o
>  obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
> Index: kvm/drivers/kvm/kvm_main.c
> ===================================================================
> --- kvm.orig/drivers/kvm/kvm_main.c	2007-10-10 11:34:07.000000000 +0200
> +++ kvm/drivers/kvm/kvm_main.c	2007-10-10 16:57:05.000000000 +0200
> @@ -16,6 +16,7 @@
>   */
> 
>  #include "kvm.h"
> +#include "x86.h"
>  #include "x86_emulate.h"
>  #include "segment_descriptor.h"
>  #include "irq.h"
> @@ -45,7 +46,6 @@
>  #include <asm/processor.h>
>  #include <asm/msr.h>
>  #include <asm/io.h>
> -#include <asm/uaccess.h>
>  #include <asm/desc.h>
> 
>  MODULE_AUTHOR("Qumranet");
> @@ -2518,43 +2518,6 @@
>  EXPORT_SYMBOL_GPL(kvm_get_cs_db_l_bits);
> 
>  /*
> - * List of msr numbers which we expose to userspace through KVM_GET_MSRS
> - * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
> - *
> - * This list is modified at module load time to reflect the
> - * capabilities of the host cpu.
> - */
> -static u32 msrs_to_save[] = {
> -	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> -	MSR_K6_STAR,
> -#ifdef CONFIG_X86_64
> -	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
> -#endif
> -	MSR_IA32_TIME_STAMP_COUNTER,
> -};
> -
> -static unsigned num_msrs_to_save;
> -
> -static u32 emulated_msrs[] = {
> -	MSR_IA32_MISC_ENABLE,
> -};
> -
> -static __init void kvm_init_msr_list(void)
> -{
> -	u32 dummy[2];
> -	unsigned i, j;
> -
> -	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
> -		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
> -			continue;
> -		if (j < i)
> -			msrs_to_save[j] = msrs_to_save[i];
> -		j++;
> -	}
> -	num_msrs_to_save = j;
> -}
> -
> -/*
>   * Adapt set_msr() to msr_io()'s calling convention
>   */
>  static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
> @@ -3366,57 +3329,25 @@
>  			goto out;
>  		r = kvm_dev_ioctl_create_vm();
>  		break;
> -	case KVM_GET_MSR_INDEX_LIST: {
> -		struct kvm_msr_list __user *user_msr_list = argp;
> -		struct kvm_msr_list msr_list;
> -		unsigned n;
> -
> -		r = -EFAULT;
> -		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
> -			goto out;
> -		n = msr_list.nmsrs;
> -		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
> -		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
> -			goto out;
> -		r = -E2BIG;
> -		if (n < num_msrs_to_save)
> -			goto out;
> -		r = -EFAULT;
> -		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
> -				 num_msrs_to_save * sizeof(u32)))
> -			goto out;
> -		if (copy_to_user(user_msr_list->indices
> -				 + num_msrs_to_save * sizeof(u32),
> -				 &emulated_msrs,
> -				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
> -			goto out;
> -		r = 0;
> -		break;
> -	}
>  	case KVM_CHECK_EXTENSION: {
>  		int ext = (long)argp;
> 
> -		switch (ext) {
> -		case KVM_CAP_IRQCHIP:
> -		case KVM_CAP_HLT:
> -		case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
> -		case KVM_CAP_USER_MEMORY:
> -			r = 1;
> -			break;
> -		default:
> -			r = 0;
> +		r = 0;
> +		if ((ext < 0) || (ext > sizeof(unsigned long long) - 1)) {
>  			break;
>  		}
> +		if (KVM_ARCH_EXTENSIONS & (1ull << ext))
> +			r = 1;
>  		break;
>  	}
>  	case KVM_GET_VCPU_MMAP_SIZE:
>  		r = -EINVAL;
>  		if (arg)
>  			goto out;
> -		r = 2 * PAGE_SIZE;
> +		r = KVM_ARCH_VCPU_MMAP_SIZE;
>  		break;
>  	default:
> -		;
> +		return kvm_arch_dev_ioctl(filp, ioctl, arg);
>  	}
>  out:
>  	return r;
> @@ -3780,7 +3711,7 @@
> 
>  	kvm_init_debug();
> 
> -	kvm_init_msr_list();
> +	kvm_arch_init();
> 
>  	bad_page = alloc_page(GFP_KERNEL);
> 
> Index: kvm/drivers/kvm/x86.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ kvm/drivers/kvm/x86.c	2007-10-10 17:00:13.000000000 +0200
> @@ -0,0 +1,102 @@
> +/*
> + * Kernel-based Virtual Machine driver for Linux
> + *
> + * derived from drivers/kvm/kvm_main.c
> + *
> + * Copyright (C) 2006 Qumranet, Inc.
> + *
> + * Authors:
> + *   Avi Kivity   <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> + *   Yaniv Kamay  <yaniv-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "kvm.h"
> +
> +#include <asm/uaccess.h>
> +
> +/*
> + * List of msr numbers which we expose to userspace through KVM_GET_MSRS
> + * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
> + *
> + * This list is modified at module load time to reflect the
> + * capabilities of the host cpu.
> + */
> +static u32 msrs_to_save[] = {
> +	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
> +	MSR_K6_STAR,
> +#ifdef CONFIG_X86_64
> +	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
> +#endif
> +	MSR_IA32_TIME_STAMP_COUNTER,
> +};
> +
> +static unsigned num_msrs_to_save;
> +
> +static u32 emulated_msrs[] = {
> +	MSR_IA32_MISC_ENABLE,
> +};
> +
> +long kvm_arch_dev_ioctl(struct file *filp,
> +			unsigned int ioctl, unsigned long arg)
> +{
> +	void __user *argp = (void __user *)arg;
> +	long r;
> +
> +	switch (ioctl) {
> +	case KVM_GET_MSR_INDEX_LIST: {
> +		struct kvm_msr_list __user *user_msr_list = argp;
> +		struct kvm_msr_list msr_list;
> +		unsigned n;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
> +			goto out;
> +		n = msr_list.nmsrs;
> +		msr_list.nmsrs = num_msrs_to_save + ARRAY_SIZE(emulated_msrs);
> +		if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
> +			goto out;
> +		r = -E2BIG;
> +		if (n < num_msrs_to_save)
> +			goto out;
> +		r = -EFAULT;
> +		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
> +				 num_msrs_to_save * sizeof(u32)))
> +			goto out;
> +		if (copy_to_user(user_msr_list->indices
> +				 + num_msrs_to_save * sizeof(u32),
> +				 &emulated_msrs,
> +				 ARRAY_SIZE(emulated_msrs) * sizeof(u32)))
> +			goto out;
> +		r = 0;
> +		break;
> +	}
> +	default:
> +		r = -EINVAL;
> +	}
> +out:
> +	return r;
> +}
> +
> +static __init void kvm_init_msr_list(void)
> +{
> +	u32 dummy[2];
> +	unsigned i, j;
> +
> +	for (i = j = 0; i < ARRAY_SIZE(msrs_to_save); i++) {
> +		if (rdmsr_safe(msrs_to_save[i], &dummy[0], &dummy[1]) < 0)
> +			continue;
> +		if (j < i)
> +			msrs_to_save[j] = msrs_to_save[i];
> +		j++;
> +	}
> +	num_msrs_to_save = j;
> +}
> +
> +__init void kvm_arch_init(void)
> +{
> +	kvm_init_msr_list();
> +}
> Index: kvm/drivers/kvm/x86.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ kvm/drivers/kvm/x86.h	2007-10-10 16:47:02.000000000 +0200
> @@ -0,0 +1,22 @@
> +#/*
> + * Kernel-based Virtual Machine driver for Linux
> + *
> + * This header defines architecture specific interfaces, x86 version
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef KVM_X86_H
> +#define KVM_X86_H
> +
> +#include "kvm.h"
> +
> +#define KVM_ARCH_VCPU_MMAP_SIZE (2 * PAGE_SIZE)
> +
> +#define KVM_ARCH_EXTENSIONS (KVM_ARCH_HAS_IRQCHIP & \
> +                             KVM_ARCH_HAS_HLT & \
> +                             KVM_ARCH_HAS_MMU_SHADOW_CACHE_CONTROL & \
> +                             KVM_ARCH_HAS_USER_MEMORY)
> +#endif
> 
> 
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability V3
       [not found]             ` <1192029379.18392.7.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
  2007-10-11  9:15               ` Christian Ehrhardt
@ 2007-10-11 11:09               ` Avi Kivity
       [not found]                 ` <470E046C.2030404-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  1 sibling, 1 reply; 13+ messages in thread
From: Avi Kivity @ 2007-10-11 11:09 UTC (permalink / raw)
  To: Carsten Otte; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Carsten Otte wrote:
> Thanks to Avi's continued review, we've got even more common code this
> time: KVM_CHECK_EXTENSION ioctl is now completely handled in kvm_main.c
> instead of having arch callbacks to check extensions. The architectures
> are expected to setup a bit mask named KVM_ARCH_EXTENSIONS with
> information about capabilities they support. Possible valus for the bit
> masks are defined in drivers/kvm/kvm.h (KVM_ARCH_HAS_*), which rely on
> KVM_CAP_* definitions in include/linux/kvm.h.
> Function prototypes in drivers/kvm/kvm.h have been fixed: they have
> argument names now.
>
>   


The new capability bitmap moves the patch out of the "very trivial" 
realm.  I removed those hunks and applied.

Send more, the approach is clearly right.  Leave things which require 
changes (like the capability bitmap) to the end, there's more than 
enough stuff for now.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: RFC/patch: a very trivial patch towards portability V3
       [not found]                 ` <470E046C.2030404-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2007-10-11 11:36                   ` Carsten Otte
  0 siblings, 0 replies; 13+ messages in thread
From: Carsten Otte @ 2007-10-11 11:36 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Avi Kivity wrote:
> The new capability bitmap moves the patch out of the "very trivial" 
> realm.  I removed those hunks and applied.
Thanks.

> Send more, the approach is clearly right.  Leave things which require 
> changes (like the capability bitmap) to the end, there's more than 
> enough stuff for now.
Okay, will come up with the next function to split soon.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2007-10-11 11:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-09 16:09 RFC/patch: a very trivial patch towards portability Carsten Otte
     [not found] ` <1191946167.7292.3.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
2007-10-09 16:17   ` Avi Kivity
     [not found]     ` <470BA98A.8090900-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-10-10  8:28       ` Carsten Otte
2007-10-09 16:28   ` Avi Kivity
     [not found]     ` <470BAC2B.40201-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-10-10  8:30       ` Carsten Otte
2007-10-10 12:37   ` RFC/patch: a very trivial patch towards portability V2 Carsten Otte
     [not found]     ` <1192019827.17745.9.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
2007-10-10 12:45       ` RFC/patch: a very trivial patch towards portability V2.1 Carsten Otte
     [not found]         ` <1192020355.17877.4.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
2007-10-10 13:21           ` Avi Kivity
     [not found]             ` <470CD1F7.1030400-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-10-10 13:59               ` Carsten Otte
2007-10-10 15:16           ` RFC/patch: a very trivial patch towards portability V3 Carsten Otte
     [not found]             ` <1192029379.18392.7.camel-WIxn4w2hgUz3YA32ykw5MLlKpX0K8NHHQQ4Iyu8u01E@public.gmane.org>
2007-10-11  9:15               ` Christian Ehrhardt
2007-10-11 11:09               ` Avi Kivity
     [not found]                 ` <470E046C.2030404-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-10-11 11:36                   ` Carsten Otte

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox