* [PATCH] Make S390x work in qemu-kvm
@ 2009-12-08 15:35 Alexander Graf
2009-12-15 10:42 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2009-12-08 15:35 UTC (permalink / raw)
To: kvm
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!
Signed-off-by: Alexander Graf <agraf@suse.de>
---
configure | 4 ++
hw/msix.c | 4 ++
hw/msix.h | 33 +++++++++++++
hw/s390-virtio.c | 6 +++
kvm-tpr-opt.c | 3 +
kvm/include/linux/kvm.h | 10 +++-
kvm/include/s390/asm/kvm.h | 44 ++++++++++++++++++
qemu-kvm-helper.c | 2 +
qemu-kvm.c | 32 ++++++++-----
qemu-kvm.h | 8 ---
target-s390x/kvm.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
target-s390x/libkvm.h | 26 +++++++++++
12 files changed, 256 insertions(+), 24 deletions(-)
create mode 100644 kvm/include/s390/asm/kvm.h
create mode 100644 target-s390x/libkvm.h
diff --git a/configure b/configure
index e804a92..b6381ae 100755
--- a/configure
+++ b/configure
@@ -188,6 +188,7 @@ case "$cpu" in
;;
s390x)
cpu="s390x"
+ target_list="s390x-softmmu"
;;
sparc|sun4[cdmuv])
cpu="sparc"
@@ -1430,6 +1431,9 @@ EOF
ppc)
kvm_arch="powerpc"
;;
+ s390x)
+ kvm_arch="s390"
+ ;;
*)
kvm_arch="$cpu"
;;
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__
+
#include "hw.h"
#include "msix.h"
#include "pci.h"
@@ -559,3 +561,5 @@ void msix_unuse_all_vectors(PCIDevice *dev)
return;
msix_free_irq_entries(dev);
}
+
+#endif
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__
+
+static int msix_init(PCIDevice *pdev, unsigned short nentries,
+ unsigned bar_nr, unsigned bar_size) { return 0; }
+
+static void msix_write_config(PCIDevice *pci_dev, uint32_t address,
+ uint32_t val, int len) { }
+
+static void msix_mmio_map(PCIDevice *pci_dev, int region_num,
+ pcibus_t addr, pcibus_t size, int type) { }
+
+static int msix_uninit(PCIDevice *d) { return 0; }
+
+static void msix_save(PCIDevice *dev, QEMUFile *f) { }
+static void msix_load(PCIDevice *dev, QEMUFile *f) { }
+
+static int msix_enabled(PCIDevice *dev) { return 0; }
+static int msix_present(PCIDevice *dev) { return 0; }
+
+static uint32_t msix_bar_size(PCIDevice *dev) { return 0; }
+
+static int msix_vector_use(PCIDevice *dev, unsigned vector) { return 0; }
+static void msix_vector_unuse(PCIDevice *dev, unsigned vector) { }
+static void msix_unuse_all_vectors(PCIDevice *dev) { }
+
+static void msix_notify(PCIDevice *dev, unsigned vector) { }
+
+static void msix_reset(PCIDevice *dev) { }
+
+#else
+
int msix_init(PCIDevice *pdev, unsigned short nentries,
unsigned bar_nr, unsigned bar_size);
@@ -34,3 +65,5 @@ void msix_reset(PCIDevice *dev);
extern int msix_supported;
#endif
+
+#endif
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
}
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__
+
#include "config.h"
#include "config-host.h"
@@ -401,3 +403,4 @@ void kvm_tpr_opt_setup(void)
register_ioport_write(0x7e, 2, 2, vtpr_ioport_write16, NULL);
}
+#endif
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
@@ -181,6 +181,11 @@ struct kvm_run {
__u64 cr8;
__u64 apic_base;
+#ifdef __KVM_S390
+ /* the processor status word for s390 */
+ __u64 psw_mask; /* psw upper half */
+ __u64 psw_addr; /* psw lower half */
+#endif
union {
/* KVM_EXIT_UNKNOWN */
struct {
@@ -232,8 +237,6 @@ struct kvm_run {
/* KVM_EXIT_S390_SIEIC */
struct {
__u8 icptcode;
- __u64 mask; /* psw upper half */
- __u64 addr; /* psw lower half */
__u16 ipa;
__u32 ipb;
} s390_sieic;
@@ -309,7 +312,7 @@ struct kvm_dirty_log {
__u32 slot;
__u32 padding1;
union {
- void *dirty_bitmap; /* one bit per page */
+ void __user *dirty_bitmap; /* one bit per page */
__u64 padding2;
};
};
@@ -492,6 +495,7 @@ struct kvm_ioeventfd {
#ifdef __KVM_HAVE_VCPU_EVENTS
#define KVM_CAP_VCPU_EVENTS 41
#endif
+#define KVM_CAP_S390_PSW 42
#ifdef KVM_CAP_IRQ_ROUTING
diff --git a/kvm/include/s390/asm/kvm.h b/kvm/include/s390/asm/kvm.h
new file mode 100644
index 0000000..82b32a1
--- /dev/null
+++ b/kvm/include/s390/asm/kvm.h
@@ -0,0 +1,44 @@
+#ifndef __LINUX_KVM_S390_H
+#define __LINUX_KVM_S390_H
+/*
+ * asm-s390/kvm.h - KVM s390 specific structures and definitions
+ *
+ * Copyright IBM Corp. 2008
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2 only)
+ * as published by the Free Software Foundation.
+ *
+ * Author(s): Carsten Otte <cotte@de.ibm.com>
+ * Christian Borntraeger <borntraeger@de.ibm.com>
+ */
+#include <linux/types.h>
+
+#define __KVM_S390
+
+/* for KVM_GET_REGS and KVM_SET_REGS */
+struct kvm_regs {
+ /* general purpose regs for s390 */
+ __u64 gprs[16];
+};
+
+/* for KVM_GET_SREGS and KVM_SET_SREGS */
+struct kvm_sregs {
+ __u32 acrs[16];
+ __u64 crs[16];
+};
+
+/* for KVM_GET_FPU and KVM_SET_FPU */
+struct kvm_fpu {
+ __u32 fpc;
+ __u64 fprs[16];
+};
+
+struct kvm_debug_exit_arch {
+};
+
+/* for KVM_SET_GUEST_DEBUG */
+struct kvm_guest_debug_arch {
+};
+
+#endif
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
}
void qemu_kvm_cpuid_on_env(CPUState *env)
diff --git a/qemu-kvm.c b/qemu-kvm.c
index c7fbce8..17c791b 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -66,7 +66,7 @@ pthread_cond_t qemu_pause_cond = PTHREAD_COND_INITIALIZER;
pthread_cond_t qemu_work_cond = PTHREAD_COND_INITIALIZER;
__thread CPUState *current_env;
-static int qemu_system_ready;
+int qemu_system_ready;
#define SIG_IPI (SIGRTMIN+4)
@@ -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
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
+
mmap_size = kvm_ioctl(kvm_state, KVM_GET_VCPU_MMAP_SIZE, 0);
if (mmap_size < 0) {
fprintf(stderr, "get vcpu mmap size: %m\n");
@@ -939,12 +949,11 @@ int kvm_run(CPUState *env)
}
#endif
-#if !defined(__s390__)
if (r == -1) {
r = handle_io_window(kvm);
goto more;
}
-#endif
+
if (1) {
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
case KVM_EXIT_INTERNAL_ERROR:
r = kvm_handle_internal_error(kvm, env, run);
break;
@@ -1127,6 +1128,9 @@ int kvm_destroy_memory_region_works(kvm_context_t kvm)
return ret;
}
+#ifdef __s390x__
+static
+#endif
int kvm_reinject_control(kvm_context_t kvm, int pit_reinject)
{
#ifdef KVM_CAP_REINJECT_CONTROL
@@ -1627,7 +1631,7 @@ static void kvm_do_save_mpstate(void *_env)
CPUState *env = _env;
kvm_arch_save_mpstate(env);
-#ifdef KVM_CAP_MP_STATE
+#if defined(KVM_CAP_MP_STATE) && !defined(__s390x__)
if (kvm_irqchip_in_kernel())
env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
#endif
@@ -2293,11 +2297,13 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size,
return;
#endif
while (size > 0) {
+#ifdef TARGET_I386
p = find_mapping(start_addr);
if (p) {
kvm_unregister_memory_area(kvm_context, p->phys, p->len);
drop_mapping(p->phys);
}
+#endif
start_addr += TARGET_PAGE_SIZE;
if (size > TARGET_PAGE_SIZE) {
size -= TARGET_PAGE_SIZE;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 74f3746..a20d89f 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -670,14 +670,6 @@ int kvm_enable_vapic(CPUState *env, uint64_t vapic);
#endif
-#if defined(__s390__)
-int kvm_s390_initial_reset(kvm_context_t kvm, int slot);
-int kvm_s390_interrupt(kvm_context_t kvm, int slot,
- struct kvm_s390_interrupt *kvmint);
-int kvm_s390_set_initial_psw(kvm_context_t kvm, int slot, psw_t psw);
-int kvm_s390_store_status(kvm_context_t kvm, int slot, unsigned long addr);
-#endif
-
#ifdef KVM_CAP_DEVICE_ASSIGNMENT
/*!
* \brief Notifies host kernel about a PCI device to be assigned to a guest
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b6aac42..0410c53 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -70,10 +70,12 @@
#define SCLP_CMDW_READ_SCP_INFO 0x00020001
#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
+#ifdef KVM_UPSTREAM
int kvm_arch_init(KVMState *s, int smp_cpus)
{
return 0;
}
+#endif
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
{
/* FIXME: add code to reset vcpu. */
}
+#ifdef KVM_UPSTREAM
int kvm_arch_put_registers(CPUState *env)
+#else
+int _kvm_arch_load_regs(CPUState *env);
+
+void kvm_arch_load_regs(CPUState *env)
+{
+ _kvm_arch_load_regs(env);
+}
+
+int _kvm_arch_load_regs(CPUState *env)
+#endif
{
struct kvm_regs regs;
int ret;
@@ -117,7 +134,18 @@ int kvm_arch_put_registers(CPUState *env)
return ret;
}
+#ifdef KVM_UPSTREAM
int kvm_arch_get_registers(CPUState *env)
+#else
+int _kvm_arch_save_regs(CPUState *env);
+
+void kvm_arch_save_regs(CPUState *env)
+{
+ _kvm_arch_save_regs(env);
+}
+
+int _kvm_arch_save_regs(CPUState *env)
+#endif
{
uint32_t ret;
struct kvm_regs regs;
@@ -180,6 +208,10 @@ static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
{
struct kvm_s390_interrupt kvmint;
int r;
+ extern int qemu_system_ready;
+
+ if (!qemu_system_ready)
+ return;
if (!env->kvm_state) {
return;
@@ -459,10 +491,17 @@ static int handle_intercept(CPUState *env)
break;
}
+#ifndef KVM_UPSTREAM
+ r = 0;
+#endif
return r;
}
+#ifdef KVM_UPSTREAM
int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
+#else
+static int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
+#endif
{
int ret = 0;
@@ -476,8 +515,77 @@ int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
break;
default:
fprintf(stderr, "Unknown KVM exit: %d\n", run->exit_reason);
+ ret = -1;
break;
}
return ret;
}
+
+int kvm_arch_run(CPUState *env)
+{
+ return kvm_arch_handle_exit(env, env->kvm_run);
+}
+
+#ifndef KVM_UPSTREAM
+void kvm_arch_save_mpstate(CPUState *env)
+{
+}
+
+void kvm_arch_load_mpstate(CPUState *env)
+{
+}
+
+int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
+ void **vm_mem)
+{
+ return 0;
+}
+
+int kvm_arch_qemu_create_context(void)
+{
+ return 0;
+}
+
+void kvm_show_regs(CPUState *env)
+{
+ struct kvm_regs regs;
+ int i, r;
+
+ r = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
+ if (r < 0) {
+ perror("KVM_GET_REGS");
+ return;
+ }
+
+ for (i = 0; i < 16; i++) {
+ fprintf(stderr, "R%02d=%016lx", i, regs.gprs[i]);
+ if ((i % 4) == 3) {
+ fprintf(stderr, "\n");
+ } else {
+ fprintf(stderr, " ");
+ }
+ }
+
+ fprintf(stderr, "PSW=mask %016lx addr %016lx\n", env->kvm_run->psw_addr, env->kvm_run->psw_mask);
+ fprintf(stderr, "ENV PSW=mask %016lx addr %016lx\n", env->psw.addr, env->psw.mask);
+}
+
+int kvm_arch_halt(CPUState *env)
+{
+ return 1;
+}
+
+void kvm_show_code(CPUState *env)
+{
+}
+
+int kvm_arch_has_work(CPUState *env)
+{
+ return 1;
+}
+
+void kvm_arch_process_irqchip_events(CPUState *env)
+{
+}
+#endif
diff --git a/target-s390x/libkvm.h b/target-s390x/libkvm.h
new file mode 100644
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 <avi@qumranet.com>
+ * Yaniv Kamay <yaniv@qumranet.com>
+ *
+ * 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
--
1.6.0.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Make S390x work in qemu-kvm
2009-12-08 15:35 [PATCH] Make S390x work in qemu-kvm Alexander Graf
@ 2009-12-15 10:42 ` Avi Kivity
2009-12-15 11:13 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-12-15 10:42 UTC (permalink / raw)
To: Alexander Graf; +Cc: kvm
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<avi@qumranet.com>
> + * Yaniv Kamay<yaniv@qumranet.com>
> + *
> + * 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make S390x work in qemu-kvm
2009-12-15 10:42 ` Avi Kivity
@ 2009-12-15 11:13 ` Alexander Graf
2009-12-15 12:30 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2009-12-15 11:13 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
Avi Kivity wrote:
> 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.
That's what I did at first in a hacky way. To be honest, the new
makefile structure scares me a bit, so I'm not sure I'll easily figure
out how to do that properly :).
>
>> 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?
Because the KVM fd's are not available yet.
>
>>
>> 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.
Ok.
>
>> 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.
Yep. IMHO all the code duplication should die :).
>
>> @@ -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.
So it's useless for IA64 as well?
>
>>
>> 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.
Yes, there is. I figured this is just a temporary hack, so who cares :-).
>
>> 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.
Ditto what? This is code removal.
>
>>
>> +#ifdef __s390x__
>> +static
>> +#endif
>>
>
> Lovely. Why?
Because it collided with the init function provided by target-s390x/kvm.c.
>
>>
>> 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
>>
>
> :(
Yeah, feel like getting the naming a bit closer? :)
>
>> 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<avi@qumranet.com>
>> + * Yaniv Kamay<yaniv@qumranet.com>
>> + *
>> + * 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!
>
Some file failed to build without it. IIRC because PAGE_* was not defined.
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make S390x work in qemu-kvm
2009-12-15 11:13 ` Alexander Graf
@ 2009-12-15 12:30 ` Avi Kivity
2009-12-15 13:03 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-12-15 12:30 UTC (permalink / raw)
To: Alexander Graf; +Cc: kvm
On 12/15/2009 01:13 PM, Alexander Graf wrote:
>
>> This should be CONFIG_PCI or CONFIG_PCI_MSI. Also, better to hack it
>> at the Makefile level.
>>
> That's what I did at first in a hacky way. To be honest, the new
> makefile structure scares me a bit, so I'm not sure I'll easily figure
> out how to do that properly :).
>
I'm sure Juan will be glad to help.
>>> +
>>> +#ifndef KVM_UPSTREAM
>>> + kvm_arch_load_regs(env);
>>> +#endif
>>> }
>>>
>>>
>> Why isn't cpu_synchronize_state() suitable?
>>
> Because the KVM fd's are not available yet.
>
Then kvm_arch_load_regs() will fail as well, no?
>>> 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.
>>
> So it's useless for IA64 as well?
>
Yes.
>>> 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.
>>
> Yes, there is. I figured this is just a temporary hack, so who cares :-).
>
The whole of qemu-kvm.c is a temporary hack. No reason to make it
uglier than it needs to be.
>>> 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.
>>
> Ditto what? This is code removal.
>
Um, yes.
>>> +#ifdef __s390x__
>>> +static
>>> +#endif
>>>
>>>
>> Lovely. Why?
>>
> Because it collided with the init function provided by target-s390x/kvm.c.
>
I'd prefer a separate rename then.
>>> 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
>>>
>>>
>> :(
>>
> Yeah, feel like getting the naming a bit closer? :)
>
A renaming patch would be welcome.
>> I thought we no longer include libkvm.h!
>>
>>
> Some file failed to build without it. IIRC because PAGE_* was not defined.
>
There's a TARGET_PAGE_SIZE or something, we can use that instead.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make S390x work in qemu-kvm
2009-12-15 12:30 ` Avi Kivity
@ 2009-12-15 13:03 ` Alexander Graf
2009-12-15 13:07 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2009-12-15 13:03 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
Avi Kivity wrote:
> On 12/15/2009 01:13 PM, Alexander Graf wrote:
>>
>>> This should be CONFIG_PCI or CONFIG_PCI_MSI. Also, better to hack it
>>> at the Makefile level.
>>>
>> That's what I did at first in a hacky way. To be honest, the new
>> makefile structure scares me a bit, so I'm not sure I'll easily figure
>> out how to do that properly :).
>>
>
> I'm sure Juan will be glad to help.
>
>>>> +
>>>> +#ifndef KVM_UPSTREAM
>>>> + kvm_arch_load_regs(env);
>>>> +#endif
>>>> }
>>>>
>>>>
>>> Why isn't cpu_synchronize_state() suitable?
>>>
>> Because the KVM fd's are not available yet.
>>
>
> Then kvm_arch_load_regs() will fail as well, no?
Eeeh - there was a reason this didn't fail :-). I don't remember.
>
>>>> 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.
>>>
>> So it's useless for IA64 as well?
>>
>
> Yes.
>
>>>> 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.
>>>
>> Yes, there is. I figured this is just a temporary hack, so who cares
>> :-).
>>
>
> The whole of qemu-kvm.c is a temporary hack. No reason to make it
> uglier than it needs to be.
>
>>>> 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.
>>>
>> Ditto what? This is code removal.
>>
>
> Um, yes.
>
>>>> +#ifdef __s390x__
>>>> +static
>>>> +#endif
>>>>
>>>>
>>> Lovely. Why?
>>>
>> Because it collided with the init function provided by
>> target-s390x/kvm.c.
>>
>
> I'd prefer a separate rename then.
Rename in kvm.c?
>
>>>> 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
>>>>
>>>>
>>> :(
>>>
>> Yeah, feel like getting the naming a bit closer? :)
>>
>
> A renaming patch would be welcome.
>
>>> I thought we no longer include libkvm.h!
>>>
>>>
>> Some file failed to build without it. IIRC because PAGE_* was not
>> defined.
>>
>
> There's a TARGET_PAGE_SIZE or something, we can use that instead.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Make S390x work in qemu-kvm
2009-12-15 13:03 ` Alexander Graf
@ 2009-12-15 13:07 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2009-12-15 13:07 UTC (permalink / raw)
To: Alexander Graf; +Cc: kvm
On 12/15/2009 03:03 PM, Alexander Graf wrote:
>
>
>>>> Why isn't cpu_synchronize_state() suitable?
>>>>
>>>>
>>> Because the KVM fd's are not available yet.
>>>
>>>
>> Then kvm_arch_load_regs() will fail as well, no?
>>
> Eeeh - there was a reason this didn't fail :-). I don't remember.
>
Please find out.
>>> Because it collided with the init function provided by
>>> target-s390x/kvm.c.
>>>
>>>
>> I'd prefer a separate rename then.
>>
> Rename in kvm.c?
>
kvm.c is harder to coordinate (need to rename in upstream), so rename
the qemu-kvm specific function.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-12-15 13:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-08 15:35 [PATCH] Make S390x work in qemu-kvm Alexander Graf
2009-12-15 10:42 ` Avi Kivity
2009-12-15 11:13 ` Alexander Graf
2009-12-15 12:30 ` Avi Kivity
2009-12-15 13:03 ` Alexander Graf
2009-12-15 13:07 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).