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. > >> +static int sclp_service_call(CPUState *env, struct kvm_run *run, uint16_t > >> ipbh0) > >> +{ > >> + uint32_t sccb; > >> + uint64_t code; > >> + int r = 0; > >> + > >> + cpu_synchronize_state(env); > >> + sccb = env->regs[ipbh0 & 0xf]; > >> + code = env->regs[(ipbh0 & 0xf0) >> 4]; > >> + > >> + dprintf("sclp(0x%x, 0x%lx)\n", sccb, code); > >> + > >> + if (sccb & ~0x7ffffff8ul) { > >> + fprintf(stderr, "KVM: invalid sccb address 0x%x\n", sccb); > >> + r = -1; > >> + goto out; > >> + } > >> + > >> + switch(code) { > >> + case 0x00020001: > >> + case 0x00120001: > > > > What are those constants? > > If I knew I'd have defined some more verbose constants. I just took them 1:1 > from kuli. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net