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 <address@hidden>
---
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 <address@hidden>
+ *
+ * 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 |