Re: RFC/patch: a very trivial patch towards portability V3

[Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@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/