On 02.12.2009, at 09:42, malc wrote: > On Wed, 2 Dec 2009, Alexander Graf wrote: > >> >> On 02.12.2009, at 09:12, Aurelien Jarno wrote: >> >>> On Mon, Nov 30, 2009 at 11:25:03PM +0100, Alexander Graf wrote: >>>> >>>> On 30.11.2009, at 19:18, Aurelien Jarno wrote: >>>> >>>>> On Thu, Nov 26, 2009 at 02:23:13PM +0100, Alexander Graf wrote: >>>>>> S390x was one of the first platforms that received support for KVM back >>>>>> in the >>>>>> day. Unfortunately until now there hasn't been a qemu implementation >>>>>> that would >>>>>> enable users to actually run guests. >>>>>> >>>>>> So let's include support for KVM S390x in qemu! >>>>> >>>>> Please find the comments below. >>>>> >>>>>> Signed-off-by: Alexander Graf <ag...@suse.de> >>>>>> --- >>>>>> configure | 4 +- >>>>>> target-s390x/kvm.c | 463 >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> 2 files changed, 466 insertions(+), 1 deletions(-) >>>>>> create mode 100644 target-s390x/kvm.c >>>>>> >>>>>> diff --git a/configure b/configure >>>>>> index b616e1a..cf466ec 100755 >>>>>> --- a/configure >>>>>> +++ b/configure >>>>>> @@ -1348,6 +1348,8 @@ EOF >>>>>> kvm_cflags="$kvm_cflags -I$kerneldir/arch/x86/include" >>>>>> elif test "$cpu" = "ppc" -a -d "$kerneldir/arch/powerpc/include" ; then >>>>>> kvm_cflags="$kvm_cflags -I$kerneldir/arch/powerpc/include" >>>>>> + elif test "$cpu" = "s390x" -a -d "$kerneldir/arch/s390/include" >>>>>> ; then >>>>>> + kvm_cflags="$kvm_cflags -I$kerneldir/arch/s390/include" >>>>>> elif test -d "$kerneldir/arch/$cpu/include" ; then >>>>>> kvm_cflags="$kvm_cflags -I$kerneldir/arch/$cpu/include" >>>>>> fi >>>>>> @@ -2360,7 +2362,7 @@ case "$target_arch2" in >>>>>> fi >>>>>> esac >>>>>> case "$target_arch2" in >>>>>> - i386|x86_64|ppcemb|ppc|ppc64) >>>>>> + i386|x86_64|ppcemb|ppc|ppc64|s390x) >>>>>> # Make sure the target and host cpus are compatible >>>>>> if test "$kvm" = "yes" -a "$target_softmmu" = "yes" -a \ >>>>>> \( "$target_arch2" = "$cpu" -o \ >>>>>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c >>>>>> new file mode 100644 >>>>>> index 0000000..d477664 >>>>>> --- /dev/null >>>>>> +++ b/target-s390x/kvm.c >>>>>> @@ -0,0 +1,463 @@ >>>>>> +/* >>>>>> + * QEMU S390x KVM implementation >>>>>> + * >>>>>> + * Copyright (c) 2009 Alexander Graf <ag...@suse.de> >>>>>> + * >>>>>> + * This library is free software; you can redistribute it and/or >>>>>> + * modify it under the terms of the GNU Lesser General Public >>>>>> + * License as published by the Free Software Foundation; either >>>>>> + * version 2 of the License, or (at your option) any later version. >>>>>> + * >>>>>> + * This library 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 >>>>>> + * Lesser General Public License for more details. >>>>>> + * >>>>>> + * You should have received a copy of the GNU Lesser General Public >>>>>> + * License along with this library; if not, see >>>>>> <http://www.gnu.org/licenses/>. >>>>>> + */ >>>>>> + >>>>>> +#include <sys/types.h> >>>>>> +#include <sys/ioctl.h> >>>>>> +#include <sys/mman.h> >>>>>> + >>>>>> +#include <linux/kvm.h> >>>>>> +#include <asm/ptrace.h> >>>>>> + >>>>>> +#include "qemu-common.h" >>>>>> +#include "qemu-timer.h" >>>>>> +#include "sysemu.h" >>>>>> +#include "kvm.h" >>>>>> +#include "cpu.h" >>>>>> +#include "device_tree.h" >>>>>> + >>>>>> +/* #define DEBUG_KVM */ >>>>>> + >>>>>> +#ifdef DEBUG_KVM >>>>>> +#define dprintf(fmt, ...) \ >>>>>> + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) >>>>>> +#else >>>>>> +#define dprintf(fmt, ...) \ >>>>>> + do { } while (0) >>>>>> +#endif >>>>>> + >>>>>> +#define IPA0_DIAG 0x8300 >>>>>> +#define IPA0_SIGP 0xae00 >>>>>> +#define IPA0_PRIV 0xb200 >>>>>> + >>>>>> +#define PRIV_SCLP_CALL 0x20 >>>>>> +#define DIAG_KVM_HYPERCALL 0x500 >>>>>> +#define DIAG_KVM_BREAKPOINT 0x501 >>>>>> + >>>>>> +#define SCP_LENGTH 0x00 >>>>>> +#define SCP_FUNCTION_CODE 0x02 >>>>>> +#define SCP_CONTROL_MASK 0x03 >>>>>> +#define SCP_RESPONSE_CODE 0x06 >>>>>> +#define SCP_MEM_CODE 0x08 >>>>>> +#define SCP_INCREMENT 0x0a >>>>>> + >>>>>> +#define ICPT_INSTRUCTION 0x04 >>>>>> +#define ICPT_WAITPSW 0x1c >>>>>> +#define ICPT_SOFT_INTERCEPT 0x24 >>>>>> +#define ICPT_CPU_STOP 0x28 >>>>>> +#define ICPT_IO 0x40 >>>>>> + >>>>>> +#define SIGP_RESTART 0x06 >>>>>> +#define SIGP_INITIAL_CPU_RESET 0x0b >>>>>> +#define SIGP_STORE_STATUS_ADDR 0x0e >>>>>> +#define SIGP_SET_ARCH 0x12 >>>>>> + >>>>>> + >>>>>> +int kvm_arch_init(KVMState *s, int smp_cpus) >>>>>> +{ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int kvm_arch_init_vcpu(CPUState *env) >>>>>> +{ >>>>>> + int ret = 0; >>>>>> + >>>>>> + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL) < 0) >>>>>> + perror("cannot init reset vcpu"); >>>>> >>>>> coding style. >>>>> >>>>>> + >>>>>> + return ret; >>>>>> +} >>>>>> + >>>>>> +void kvm_arch_reset_vcpu(CPUState *env) >>>>>> +{ >>>>>> +} >>>>> >>>>> Is there really nothing to do? If some code has to be added later, it >>>>> may be worth adding a comment. >>>> >>>> As you have probably realized by now, all reset code is missing. In fact, >>>> we don't even ever call the qemu reset functions to actually do a reset. >>> >>> Can you make it clean with an abort? >>> >>>>>> + >>>>>> +static void _kvm_s390_interrupt(CPUState *env, int type, uint32_t parm, >>>>>> uint64_t parm64, int vm) >>>>>> +{ >>>>> >>>>> Why such a name starting with an underscore? >>>> >>>> Because that's the internal function that gets used by the exported, >>>> properly named ones. Are there any conventions on how to declare private >>>> functions? >>> >>> I don't think there is any convention, but I know malc always complains >>> about not introducing names starting with an underscore. > > Yeah he does. > >> >> Hm - I just wanted to clearly show that this is an internal API, nobody >> should really have to call directly. But I'm open for other naming >> suggestions. > > Thing is, in 7.1.3#1 standard says (after explicitly reserving __ _[A-Z] > for any use): > -- All identifiers that begin with an underscore are > always reserved for use as identifiers with file scope > in both the ordinary and tag name spaces. > > And i could never really understand (or recall/comprehend when asked > and being given an answer) what this entails. (Anyone?)
I don't get the second part, but the first one clearly says "If you use a function beginning underscore, only use it within the file you're declaring it at". > So i would go with something imaginative like internal_do_not_use_kvm*, > but that's just me. You can go wild here, leading underscore doesn't look > attractive though. Well I could have gone with kvm_s390_interrupt_generic or kvm_s390_interrupt_internal. But I wanted to have a function name that doesn't exceed the 80 character limit all by itself ;-). Alex
