From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Make S390x work in qemu-kvm Date: Tue, 15 Dec 2009 12:42:12 +0200 Message-ID: <4B276804.9080605@redhat.com> References: <1260286554-18253-1-git-send-email-agraf@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Alexander Graf Return-path: Received: from mx1.redhat.com ([209.132.183.28]:64830 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759626AbZLOKmQ (ORCPT ); Tue, 15 Dec 2009 05:42:16 -0500 In-Reply-To: <1260286554-18253-1-git-send-email-agraf@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: On 12/08/2009 05:35 PM, Alexander Graf wrote: > We now have S390x KVM support in qemu upstream. > > Unfortunately it doesn't work in qemu-kvm, because that has its own main > loop and slightly different calling conventions for the KVM helpers. > > So let's hack in some small compat ifdefs that make qemu-kvm work on S390x! > > > > > diff --git a/hw/msix.c b/hw/msix.c > index 6d598ee..b2c2857 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -11,6 +11,8 @@ > * the COPYING file in the top-level directory. > */ > > +#ifndef __s390x__ > + > This should be CONFIG_PCI or CONFIG_PCI_MSI. Also, better to hack it at the Makefile level. > diff --git a/hw/msix.h b/hw/msix.h > index a9f7993..643b3a1 100644 > --- a/hw/msix.h > +++ b/hw/msix.h > @@ -4,6 +4,37 @@ > #include "qemu-common.h" > #include "pci.h" > > +#ifdef __s390x__ > Ditto (minus Makefile). > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index cc21ee6..ea74955 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -179,7 +179,9 @@ static void s390_init(ram_addr_t ram_size, > exit(1); > } > > +#ifdef KVM_UPSTREAM > cpu_synchronize_state(env); > +#endif > env->psw.addr = KERN_IMAGE_START; > env->psw.mask = 0x0000000180000000UL; > } > @@ -236,6 +238,10 @@ static void s390_init(ram_addr_t ram_size, > qdev_prop_set_drive(dev, "drive", dinfo); > qdev_init_nofail(dev); > } > + > +#ifndef KVM_UPSTREAM > + kvm_arch_load_regs(env); > +#endif > } > Why isn't cpu_synchronize_state() suitable? > > static QEMUMachine s390_machine = { > diff --git a/kvm-tpr-opt.c b/kvm-tpr-opt.c > index 2565d79..e8c4185 100644 > --- a/kvm-tpr-opt.c > +++ b/kvm-tpr-opt.c > @@ -6,6 +6,8 @@ > * Licensed under the terms of the GNU GPL version 2 or higher. > */ > > +#ifndef __s390x__ > @Makefile. > diff --git a/kvm/include/linux/kvm.h b/kvm/include/linux/kvm.h > index db10887..2d241da 100644 > --- a/kvm/include/linux/kvm.h > +++ b/kvm/include/linux/kvm.h > Please post kvm header changes as a separate patch. > diff --git a/qemu-kvm-helper.c b/qemu-kvm-helper.c > index 9420eb1..d5b75bd 100644 > --- a/qemu-kvm-helper.c > +++ b/qemu-kvm-helper.c > @@ -30,7 +30,9 @@ void qemu_kvm_call_with_env(void (*func)(void *), void *data, CPUState *newenv) > > static void call_helper_cpuid(void *junk) > { > +#ifndef __s390x__ > helper_cpuid(); > +#endif > } > That just has to die (but not in your patches). It dates from the pre-tcg days. > @@ -157,7 +157,7 @@ static void init_slots(void) > > static int get_free_slot(kvm_context_t kvm) > { > - int i; > + int i = 0; > int tss_ext; > > #if defined(KVM_CAP_SET_TSS_ADDR)&& !defined(__s390__) > @@ -171,10 +171,12 @@ static int get_free_slot(kvm_context_t kvm) > * slot 0 to hold the extended memory, as the vmx will use the last 3 > * pages of this slot. > */ > +#ifndef __s390x__ > if (tss_ext> 0) > i = 0; > else > i = 1; > +#endif > That should conditioned on x86, not s390. While you're at it, drop the i = 0 assignment please. > > for (; i< KVM_MAX_NUM_MEM_REGIONS; ++i) > if (!slots[i].len) > @@ -450,6 +452,14 @@ static void kvm_create_vcpu(CPUState *env, int id) > env->kvm_fd = r; > env->kvm_state = kvm_state; > > +#ifdef __s390x__ > + r = kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, 0); > + if (r< 0) { > + fprintf(stderr, "kvm_s390_initial_reset: %m\n"); > + exit(1); > + } > +#endif > Isn't there an arch hook for this? TARGET_S390 or similar. > switch (run->exit_reason) { > case KVM_EXIT_UNKNOWN: > @@ -981,14 +990,6 @@ int kvm_run(CPUState *env) > case KVM_EXIT_SHUTDOWN: > r = handle_shutdown(kvm, env); > break; > -#if defined(__s390__) > - case KVM_EXIT_S390_SIEIC: > - r = kvm_s390_handle_intercept(kvm, env, run); > - break; > - case KVM_EXIT_S390_RESET: > - r = kvm_s390_handle_reset(kvm, env, run); > - break; > -#endif > Ditto. > > +#ifdef __s390x__ > +static > +#endif > Lovely. Why? > > int kvm_arch_init_vcpu(CPUState *env) > { > @@ -86,12 +88,27 @@ int kvm_arch_init_vcpu(CPUState *env) > return ret; > } > > +#ifdef KVM_UPSTREAM > void kvm_arch_reset_vcpu(CPUState *env) > +#else > +void kvm_arch_cpu_reset(CPUState *env) > +#endif > :( > index 0000000..9a6bcd6 > --- /dev/null > +++ b/target-s390x/libkvm.h > @@ -0,0 +1,26 @@ > +/* > + * This header is for functions& variables that will ONLY be > + * used inside libkvm for x86. > + * THESE ARE NOT EXPOSED TO THE USER AND ARE ONLY FOR USE > + * WITHIN LIBKVM. > + * > + * derived from libkvm.c > + * > + * Copyright (C) 2006 Qumranet, Inc. > + * > + * Authors: > + * Avi Kivity > + * Yaniv Kamay > + * > + * This work is licensed under the GNU LGPL license, version 2. > + */ > + > +#ifndef KVM_X86_H > +#define KVM_X86_H > + > +#define PAGE_SIZE 4096ul > +#define PAGE_MASK (~(PAGE_SIZE - 1)) > + > +#define smp_wmb() asm volatile("" ::: "memory") > + > +#endif > I thought we no longer include libkvm.h! -- error compiling committee.c: too many arguments to function