Re: [Qemu-devel] [PATCH v2 07/10] target-mips: kvm: Add main KVM support for MIPS
Hi Andreas,
On 10/02/14 14:07, Andreas Färber wrote:
+#define dprintf(fmt, ...) \
dprintf is the name of a stdio.h function, so DPRINTF may be a better name.
Okay.
+int kvm_arch_init_vcpu(CPUState *env)
Please use env only for CPUMIPSState, use cpu or cs here. The
usual convention is cs for CPUState in target-*/ so that cpu can be
used for MIPSCPU.
Okay.
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+static inline int cpu_mips_io_interrupts_pending(CPUArchState *env)
Please don't use CPUArchState in MIPS-specific code, use CPUMIPSState.
Although in this trivial case MIPSCPU would be more future-proof.
True.
+{
+dprintf(%s: %#x\n, __func__, env-CP0_Cause (1 (2 + CP0Ca_IP)));
+return env-CP0_Cause (0x1 (2 + CP0Ca_IP));
+}
+
+
+void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int r;
+struct kvm_mips_interrupt intr;
+
+if ((cs-interrupt_request CPU_INTERRUPT_HARD)
+(cpu_mips_io_interrupts_pending(env))) {
Parentheses around cpu_mips_io_interrupts_pending() seem unnecessary
here FWIW.
Good spot
+intr.cpu = -1;
+intr.irq = 2;
+r = kvm_vcpu_ioctl(cs, KVM_INTERRUPT, intr);
+if (r 0) {
+printf(cpu %d fail inject %x\n, cs-cpu_index, intr.irq);
Should this really be a printf() rather than error_report() or trace point?
It looks like error_report() would indeed be better, thanks
+int kvm_mips_set_interrupt(CPUMIPSState *env, int irq, int level)
+{
+CPUState *cs = ENV_GET_CPU(env);
CPU(mips_env_get_cpu(env)) please - ENV_GET_CPU() is for generic code
only and supposed to go away.
Any chance a MIPSCPU *cpu (or CPUState *cs) argument can be used instead?
Yep, MIPSCPU can happily be used here (I thought the same thing after
fixing cpu_mips_io_interrupts_pending above).
Thanks for taking the time to review!
Cheers
James
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v2 07/10] target-mips: kvm: Add main KVM support for MIPS
Am 16.12.2013 15:12, schrieb James Hogan:
From: Sanjay Lal sanj...@kymasys.com
Implement the main KVM arch API for MIPS.
Signed-off-by: Sanjay Lal sanj...@kymasys.com
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Aurelien Jarno aurel...@aurel32.net
Cc: Gleb Natapov g...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
---
Changes in v2:
- Expand commit message
- Checkpatch cleanups.
- Some interrupt bug fixes from Yann Le Du l...@kymasys.com
- Add get/set register functionality from Yann Le Du l...@kymasys.com
- Use new 64 bit compatible ABI from Cavium from Sanjay Lal
sanj...@kymasys.com
- Add dummy kvm_arch_init_irq_routing()
The common KVM code insists on calling kvm_arch_init_irq_routing() as
soon as it sees kernel header support for it (regardless of whether
QEMU supports it). Provide a dummy function to satisfy this.
- Remove request_interrupt_window code (Peter Maydell)
---
target-mips/kvm.c | 463
+
target-mips/kvm_mips.h | 28 +++
2 files changed, 491 insertions(+)
create mode 100644 target-mips/kvm.c
create mode 100644 target-mips/kvm_mips.h
diff --git a/target-mips/kvm.c b/target-mips/kvm.c
new file mode 100644
index 000..951959b
--- /dev/null
+++ b/target-mips/kvm.c
@@ -0,0 +1,463 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * KVM/MIPS: MIPS specific KVM APIs
+ *
+ * Copyright (C) 2012-2013 Imagination Technologies Ltd.
+ * Authors: Sanjay Lal sanj...@kymasys.com
+*/
+
+#include sys/types.h
+#include sys/ioctl.h
+#include sys/mman.h
+
+#include linux/kvm.h
+
+#include qemu-common.h
+#include qemu/timer.h
+#include sysemu/sysemu.h
+#include sysemu/kvm.h
+#include cpu.h
+#include sysemu/cpus.h
+#include kvm_mips.h
+
+#define DEBUG_KVM 0
+
+#define dprintf(fmt, ...) \
dprintf is the name of a stdio.h function, so DPRINTF may be a better name.
+do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
This looks really modern, thanks. :)
+
+const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+KVM_CAP_LAST_INFO
+};
+
+unsigned long kvm_arch_vcpu_id(CPUState *cpu)
+{
+return cpu-cpu_index;
+}
+
+int kvm_arch_init(KVMState *s)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+int kvm_arch_init_vcpu(CPUState *env)
Please use env only for CPUMIPSState, use cpu or cs here. The
usual convention is cs for CPUState in target-*/ so that cpu can be
used for MIPSCPU.
+{
+int ret = 0;
+dprintf(%s\n, __func__);
+return ret;
+}
+
+void kvm_arch_reset_vcpu(CPUState *env)
Dito.
+{
+dprintf(%s\n, __func__);
+}
+
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+struct kvm_regs regs;
+int ret;
+int i;
+
+/* Set the registers based on QEMU's view of things */
+for (i = 0; i 32; i++) {
+regs.gpr[i] = env-active_tc.gpr[i];
+}
+
+regs.hi = env-active_tc.HI[0];
+regs.lo = env-active_tc.LO[0];
+regs.pc = env-active_tc.PC;
+
+ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, regs);
+
+if (ret 0) {
+return ret;
+}
+
+ret = kvm_mips_te_put_cp0_registers(cs, KVM_PUT_FULL_STATE);
+if (ret 0) {
+return ret;
+}
+
+return ret;
+}
+
+int kvm_arch_get_registers(CPUState *cs)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int ret = 0;
+struct kvm_regs regs;
+int i;
+
+/* Get the current register set as KVM seems it */
+ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, regs);
+
+if (ret 0) {
+return ret;
+}
+
+for (i = 0; i 32; i++) {
+env-active_tc.gpr[i] = regs.gpr[i];
+}
+
+env-active_tc.HI[0] = regs.hi;
+env-active_tc.LO[0] = regs.lo;
+env-active_tc.PC = regs.pc;
+
+kvm_mips_te_get_cp0_registers(cs);
+
+return ret;
+}
+
+int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint
*bp)
Dito.
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint
*bp)
Dito.
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+static inline int cpu_mips_io_interrupts_pending(CPUArchState *env)
Please don't use CPUArchState in MIPS-specific code, use CPUMIPSState.
Although in this trivial case MIPSCPU would be more future-proof.
+{
+dprintf(%s: %#x\n, __func__, env-CP0_Cause (1 (2 + CP0Ca_IP)));
+return env-CP0_Cause (0x1 (2 + CP0Ca_IP));
+}
+
+
+void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int r;
+struct kvm_mips_interrupt intr;
+
+if
Re: [PATCH v2 07/10] target-mips: kvm: Add main KVM support for MIPS
On Mon, Dec 16, 2013 at 02:12:42PM +, James Hogan wrote:
From: Sanjay Lal sanj...@kymasys.com
Implement the main KVM arch API for MIPS.
Signed-off-by: Sanjay Lal sanj...@kymasys.com
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Aurelien Jarno aurel...@aurel32.net
Cc: Gleb Natapov g...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
---
Changes in v2:
- Expand commit message
- Checkpatch cleanups.
- Some interrupt bug fixes from Yann Le Du l...@kymasys.com
- Add get/set register functionality from Yann Le Du l...@kymasys.com
- Use new 64 bit compatible ABI from Cavium from Sanjay Lal
sanj...@kymasys.com
- Add dummy kvm_arch_init_irq_routing()
The common KVM code insists on calling kvm_arch_init_irq_routing() as
soon as it sees kernel header support for it (regardless of whether
QEMU supports it). Provide a dummy function to satisfy this.
- Remove request_interrupt_window code (Peter Maydell)
---
target-mips/kvm.c | 463
+
target-mips/kvm_mips.h | 28 +++
2 files changed, 491 insertions(+)
create mode 100644 target-mips/kvm.c
create mode 100644 target-mips/kvm_mips.h
diff --git a/target-mips/kvm.c b/target-mips/kvm.c
new file mode 100644
index 000..951959b
--- /dev/null
+++ b/target-mips/kvm.c
@@ -0,0 +1,463 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * KVM/MIPS: MIPS specific KVM APIs
+ *
+ * Copyright (C) 2012-2013 Imagination Technologies Ltd.
+ * Authors: Sanjay Lal sanj...@kymasys.com
+*/
+
+#include sys/types.h
+#include sys/ioctl.h
+#include sys/mman.h
+
+#include linux/kvm.h
+
+#include qemu-common.h
+#include qemu/timer.h
+#include sysemu/sysemu.h
+#include sysemu/kvm.h
+#include cpu.h
+#include sysemu/cpus.h
+#include kvm_mips.h
+
+#define DEBUG_KVM 0
+
+#define dprintf(fmt, ...) \
+do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+
+const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+KVM_CAP_LAST_INFO
+};
+
+unsigned long kvm_arch_vcpu_id(CPUState *cpu)
+{
+return cpu-cpu_index;
+}
+
+int kvm_arch_init(KVMState *s)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+int kvm_arch_init_vcpu(CPUState *env)
+{
+int ret = 0;
+dprintf(%s\n, __func__);
+return ret;
+}
+
+void kvm_arch_reset_vcpu(CPUState *env)
+{
+dprintf(%s\n, __func__);
+}
+
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+struct kvm_regs regs;
+int ret;
+int i;
+
+/* Set the registers based on QEMU's view of things */
+for (i = 0; i 32; i++) {
+regs.gpr[i] = env-active_tc.gpr[i];
+}
+
+regs.hi = env-active_tc.HI[0];
+regs.lo = env-active_tc.LO[0];
+regs.pc = env-active_tc.PC;
+
+ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, regs);
+
+if (ret 0) {
+return ret;
+}
+
+ret = kvm_mips_te_put_cp0_registers(cs, KVM_PUT_FULL_STATE);
+if (ret 0) {
+return ret;
+}
+
+return ret;
+}
+
+int kvm_arch_get_registers(CPUState *cs)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int ret = 0;
+struct kvm_regs regs;
+int i;
+
+/* Get the current register set as KVM seems it */
+ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, regs);
+
+if (ret 0) {
+return ret;
+}
+
+for (i = 0; i 32; i++) {
+env-active_tc.gpr[i] = regs.gpr[i];
+}
+
+env-active_tc.HI[0] = regs.hi;
+env-active_tc.LO[0] = regs.lo;
+env-active_tc.PC = regs.pc;
+
+kvm_mips_te_get_cp0_registers(cs);
+
+return ret;
+}
+
+int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint
*bp)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint
*bp)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+static inline int cpu_mips_io_interrupts_pending(CPUArchState *env)
+{
+dprintf(%s: %#x\n, __func__, env-CP0_Cause (1 (2 + CP0Ca_IP)));
+return env-CP0_Cause (0x1 (2 + CP0Ca_IP));
+}
+
+
+void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int r;
+struct kvm_mips_interrupt intr;
+
+if ((cs-interrupt_request CPU_INTERRUPT_HARD)
+(cpu_mips_io_interrupts_pending(env))) {
+intr.cpu = -1;
+intr.irq = 2;
+r = kvm_vcpu_ioctl(cs, KVM_INTERRUPT, intr);
+if (r 0) {
+printf(cpu %d fail inject %x\n, cs-cpu_index, intr.irq);
+}
+}
+}
+
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+{
+dprintf(%s\n,
[PATCH v2 07/10] target-mips: kvm: Add main KVM support for MIPS
From: Sanjay Lal sanj...@kymasys.com
Implement the main KVM arch API for MIPS.
Signed-off-by: Sanjay Lal sanj...@kymasys.com
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Aurelien Jarno aurel...@aurel32.net
Cc: Gleb Natapov g...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
---
Changes in v2:
- Expand commit message
- Checkpatch cleanups.
- Some interrupt bug fixes from Yann Le Du l...@kymasys.com
- Add get/set register functionality from Yann Le Du l...@kymasys.com
- Use new 64 bit compatible ABI from Cavium from Sanjay Lal
sanj...@kymasys.com
- Add dummy kvm_arch_init_irq_routing()
The common KVM code insists on calling kvm_arch_init_irq_routing() as
soon as it sees kernel header support for it (regardless of whether
QEMU supports it). Provide a dummy function to satisfy this.
- Remove request_interrupt_window code (Peter Maydell)
---
target-mips/kvm.c | 463 +
target-mips/kvm_mips.h | 28 +++
2 files changed, 491 insertions(+)
create mode 100644 target-mips/kvm.c
create mode 100644 target-mips/kvm_mips.h
diff --git a/target-mips/kvm.c b/target-mips/kvm.c
new file mode 100644
index 000..951959b
--- /dev/null
+++ b/target-mips/kvm.c
@@ -0,0 +1,463 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * KVM/MIPS: MIPS specific KVM APIs
+ *
+ * Copyright (C) 2012-2013 Imagination Technologies Ltd.
+ * Authors: Sanjay Lal sanj...@kymasys.com
+*/
+
+#include sys/types.h
+#include sys/ioctl.h
+#include sys/mman.h
+
+#include linux/kvm.h
+
+#include qemu-common.h
+#include qemu/timer.h
+#include sysemu/sysemu.h
+#include sysemu/kvm.h
+#include cpu.h
+#include sysemu/cpus.h
+#include kvm_mips.h
+
+#define DEBUG_KVM 0
+
+#define dprintf(fmt, ...) \
+do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+
+const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+KVM_CAP_LAST_INFO
+};
+
+unsigned long kvm_arch_vcpu_id(CPUState *cpu)
+{
+return cpu-cpu_index;
+}
+
+int kvm_arch_init(KVMState *s)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+int kvm_arch_init_vcpu(CPUState *env)
+{
+int ret = 0;
+dprintf(%s\n, __func__);
+return ret;
+}
+
+void kvm_arch_reset_vcpu(CPUState *env)
+{
+dprintf(%s\n, __func__);
+}
+
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+struct kvm_regs regs;
+int ret;
+int i;
+
+/* Set the registers based on QEMU's view of things */
+for (i = 0; i 32; i++) {
+regs.gpr[i] = env-active_tc.gpr[i];
+}
+
+regs.hi = env-active_tc.HI[0];
+regs.lo = env-active_tc.LO[0];
+regs.pc = env-active_tc.PC;
+
+ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, regs);
+
+if (ret 0) {
+return ret;
+}
+
+ret = kvm_mips_te_put_cp0_registers(cs, KVM_PUT_FULL_STATE);
+if (ret 0) {
+return ret;
+}
+
+return ret;
+}
+
+int kvm_arch_get_registers(CPUState *cs)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int ret = 0;
+struct kvm_regs regs;
+int i;
+
+/* Get the current register set as KVM seems it */
+ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, regs);
+
+if (ret 0) {
+return ret;
+}
+
+for (i = 0; i 32; i++) {
+env-active_tc.gpr[i] = regs.gpr[i];
+}
+
+env-active_tc.HI[0] = regs.hi;
+env-active_tc.LO[0] = regs.lo;
+env-active_tc.PC = regs.pc;
+
+kvm_mips_te_get_cp0_registers(cs);
+
+return ret;
+}
+
+int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
+{
+dprintf(%s\n, __func__);
+return 0;
+}
+
+static inline int cpu_mips_io_interrupts_pending(CPUArchState *env)
+{
+dprintf(%s: %#x\n, __func__, env-CP0_Cause (1 (2 + CP0Ca_IP)));
+return env-CP0_Cause (0x1 (2 + CP0Ca_IP));
+}
+
+
+void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = cpu-env;
+int r;
+struct kvm_mips_interrupt intr;
+
+if ((cs-interrupt_request CPU_INTERRUPT_HARD)
+(cpu_mips_io_interrupts_pending(env))) {
+intr.cpu = -1;
+intr.irq = 2;
+r = kvm_vcpu_ioctl(cs, KVM_INTERRUPT, intr);
+if (r 0) {
+printf(cpu %d fail inject %x\n, cs-cpu_index, intr.irq);
+}
+}
+}
+
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+{
+dprintf(%s\n, __func__);
+}
+
+int kvm_arch_process_async_events(CPUState *cs)
+{
+return cs-halted;
+}
+
+int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
+{
+int ret;
+
+dprintf(%s\n, __func__);
+switch (run-exit_reason) {
+default:
+
