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.
>> +
>> +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?
>> +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.
