2013-05-22 18:44:09

by David Daney

[permalink] [raw]
Subject: [PATCH v5 0/6] mips/kvm: Fix ABI for compatibility with 64-bit guests.

From: David Daney <[email protected]>

The initial patch set implementing MIPS KVM does not handle 64-bit
guests or use of the FPU. This patch set corrects these ABI issues,
and does some very minor clean up.

Changes from v4: No code change, just keep more of the code in
kvm_mips.c rather than kvm_trap_emul.c

Changes from v3: Use KVM_SET_ONE_REG instead of KVM_SET_MSRS. Added
ENOIOCTLCMD patch.

Changes from v2: Split into five parts, no code change.

David Daney (6):
mips/kvm: Fix ABI for use of FPU.
mips/kvm: Fix ABI for use of 64-bit registers.
mips/kvm: Fix name of gpr field in struct kvm_regs.
mips/kvm: Use ARRAY_SIZE() instead of hardcoded constants in
kvm_arch_vcpu_ioctl_{s,g}et_regs
mips/kvm: Fix ABI by moving manipulation of CP0 registers to
KVM_{G,S}ET_ONE_REG
mips/kvm: Use ENOIOCTLCMD to indicate unimplemented ioctls.

arch/mips/include/asm/kvm.h | 137 +++++++++++++++----
arch/mips/include/asm/kvm_host.h | 4 -
arch/mips/kvm/kvm_mips.c | 278 ++++++++++++++++++++++++++++++++++++---
arch/mips/kvm/kvm_trap_emul.c | 50 -------
4 files changed, 367 insertions(+), 102 deletions(-)

--
1.7.11.7


2013-05-22 18:44:13

by David Daney

[permalink] [raw]
Subject: [PATCH v5 1/6] mips/kvm: Fix ABI for use of FPU.

From: David Daney <[email protected]>

Define a non-empty struct kvm_fpu.

Signed-off-by: David Daney <[email protected]>
---
arch/mips/include/asm/kvm.h | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
index 85789ea..0e8f565 100644
--- a/arch/mips/include/asm/kvm.h
+++ b/arch/mips/include/asm/kvm.h
@@ -1,11 +1,12 @@
/*
-* 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.
-*
-* Copyright (C) 2012 MIPS Technologies, Inc. All rights reserved.
-* Authors: Sanjay Lal <[email protected]>
-*/
+ * 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.
+ *
+ * Copyright (C) 2012 MIPS Technologies, Inc. All rights reserved.
+ * Copyright (C) 2013 Cavium, Inc.
+ * Authors: Sanjay Lal <[email protected]>
+ */

#ifndef __LINUX_KVM_MIPS_H
#define __LINUX_KVM_MIPS_H
@@ -31,8 +32,20 @@ struct kvm_regs {
struct kvm_sregs {
};

-/* for KVM_GET_FPU and KVM_SET_FPU */
+/*
+ * for KVM_GET_FPU and KVM_SET_FPU
+ *
+ * If Status[FR] is zero (32-bit FPU), the upper 32-bits of the FPRs
+ * are zero filled.
+ */
struct kvm_fpu {
+ __u64 fpr[32];
+ __u32 fir;
+ __u32 fccr;
+ __u32 fexr;
+ __u32 fenr;
+ __u32 fcsr;
+ __u32 pad;
};

struct kvm_debug_exit_arch {
--
1.7.11.7

2013-05-22 18:44:11

by David Daney

[permalink] [raw]
Subject: [PATCH v5 2/6] mips/kvm: Fix ABI for use of 64-bit registers.

From: David Daney <[email protected]>

All registers are 64-bits wide, 32-bit guests use the least
significant portion of the register storage fields.

Signed-off-by: David Daney <[email protected]>
---
arch/mips/include/asm/kvm.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
index 0e8f565..86812fb 100644
--- a/arch/mips/include/asm/kvm.h
+++ b/arch/mips/include/asm/kvm.h
@@ -18,12 +18,18 @@
#define N_MIPS_COPROC_REGS 32
#define N_MIPS_COPROC_SEL 8

-/* for KVM_GET_REGS and KVM_SET_REGS */
+/*
+ * for KVM_GET_REGS and KVM_SET_REGS
+ *
+ * If Config[AT] is zero (32-bit CPU), the register contents are
+ * stored in the lower 32-bits of the struct kvm_regs fields and sign
+ * extended to 64-bits.
+ */
struct kvm_regs {
- __u32 gprs[32];
- __u32 hi;
- __u32 lo;
- __u32 pc;
+ __u64 gprs[32];
+ __u64 hi;
+ __u64 lo;
+ __u64 pc;

__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
};
--
1.7.11.7

2013-05-22 18:45:16

by David Daney

[permalink] [raw]
Subject: [PATCH v5 6/6] mips/kvm: Use ENOIOCTLCMD to indicate unimplemented ioctls.

From: David Daney <[email protected]>

The Linux Way is to return -ENOIOCTLCMD to the vfs when an
unimplemented ioctl is requested. Do this in kvm_mips instead of a
random mixture of -ENOTSUPP and -EINVAL.

Signed-off-by: David Daney <[email protected]>
---
arch/mips/kvm/kvm_mips.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 5aec4f4..13e9438 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -195,7 +195,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
long
kvm_arch_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
{
- return -EINVAL;
+ return -ENOIOCTLCMD;
}

void kvm_arch_free_memslot(struct kvm_memory_slot *free,
@@ -401,7 +401,7 @@ int
kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
- return -EINVAL;
+ return -ENOIOCTLCMD;
}

int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
@@ -475,14 +475,14 @@ int
kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
struct kvm_mp_state *mp_state)
{
- return -EINVAL;
+ return -ENOIOCTLCMD;
}

int
kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
struct kvm_mp_state *mp_state)
{
- return -EINVAL;
+ return -ENOIOCTLCMD;
}

#define KVM_REG_MIPS_CP0_INDEX (0x10000 + 8 * 0 + 0)
@@ -799,7 +799,7 @@ long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)

switch (ioctl) {
default:
- r = -EINVAL;
+ r = -ENOIOCTLCMD;
}

return r;
@@ -827,13 +827,13 @@ void kvm_arch_exit(void)
int
kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
{
- return -ENOTSUPP;
+ return -ENOIOCTLCMD;
}

int
kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
{
- return -ENOTSUPP;
+ return -ENOIOCTLCMD;
}

int kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
@@ -843,12 +843,12 @@ int kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)

int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
{
- return -ENOTSUPP;
+ return -ENOIOCTLCMD;
}

int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
{
- return -ENOTSUPP;
+ return -ENOIOCTLCMD;
}

int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
--
1.7.11.7

2013-05-22 18:45:48

by David Daney

[permalink] [raw]
Subject: [PATCH v5 4/6] mips/kvm: Use ARRAY_SIZE() instead of hardcoded constants in kvm_arch_vcpu_ioctl_{s,g}et_regs

From: David Daney <[email protected]>

Also we cannot set special zero register, so force it to zero.

Signed-off-by: David Daney <[email protected]>
---
arch/mips/kvm/kvm_mips.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 93da750..71a1fc1 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -677,9 +677,9 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
int i;

- for (i = 0; i < 32; i++)
+ for (i = 1; i < ARRAY_SIZE(vcpu->arch.gprs); i++)
vcpu->arch.gprs[i] = regs->gpr[i];
-
+ vcpu->arch.gprs[0] = 0; /* zero is special, and cannot be set. */
vcpu->arch.hi = regs->hi;
vcpu->arch.lo = regs->lo;
vcpu->arch.pc = regs->pc;
@@ -691,7 +691,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
int i;

- for (i = 0; i < 32; i++)
+ for (i = 0; i < ARRAY_SIZE(vcpu->arch.gprs); i++)
regs->gpr[i] = vcpu->arch.gprs[i];

regs->hi = vcpu->arch.hi;
--
1.7.11.7

2013-05-22 18:45:47

by David Daney

[permalink] [raw]
Subject: [PATCH v5 5/6] mips/kvm: Fix ABI by moving manipulation of CP0 registers to KVM_{G,S}ET_ONE_REG

From: David Daney <[email protected]>

Because not all 256 CP0 registers are ever implemented, we need a
different method of manipulating them. Use the
KVM_SET_ONE_REG/KVM_GET_ONE_REG mechanism.

Now unused code and definitions are removed.

Signed-off-by: David Daney <[email protected]>
---
arch/mips/include/asm/kvm.h | 91 +++++++++++---
arch/mips/include/asm/kvm_host.h | 4 -
arch/mips/kvm/kvm_mips.c | 250 +++++++++++++++++++++++++++++++++++++--
arch/mips/kvm/kvm_trap_emul.c | 50 --------
4 files changed, 320 insertions(+), 75 deletions(-)

diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
index d145ead..3f424f5 100644
--- a/arch/mips/include/asm/kvm.h
+++ b/arch/mips/include/asm/kvm.h
@@ -13,10 +13,11 @@

#include <linux/types.h>

-#define __KVM_MIPS
-
-#define N_MIPS_COPROC_REGS 32
-#define N_MIPS_COPROC_SEL 8
+/*
+ * KVM MIPS specific structures and definitions.
+ *
+ * Some parts derived from the x86 version of this file.
+ */

/*
* for KVM_GET_REGS and KVM_SET_REGS
@@ -31,12 +32,6 @@ struct kvm_regs {
__u64 hi;
__u64 lo;
__u64 pc;
-
- __u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
-};
-
-/* for KVM_GET_SREGS and KVM_SET_SREGS */
-struct kvm_sregs {
};

/*
@@ -55,21 +50,89 @@ struct kvm_fpu {
__u32 pad;
};

+
+/*
+ * For MIPS, we use KVM_SET_ONE_REG and KVM_GET_ONE_REG to access CP0
+ * registers. The id field is broken down as follows:
+ *
+ * bits[2..0] - Register 'sel' index.
+ * bits[7..3] - Register 'rd' index.
+ * bits[15..8] - Must be zero.
+ * bits[63..16] - 1 -> CP0 registers.
+ *
+ * Other sets registers may be added in the future. Each set would
+ * have its own identifier in bits[63..16].
+ *
+ * The addr field of struct kvm_one_reg must point to an aligned
+ * 64-bit wide location. For registers that are narrower than
+ * 64-bits, the value is stored in the low order bits of the location,
+ * and sign extended to 64-bits.
+ *
+ * The registers defined in struct kvm_regs are also accessible, the
+ * id values for these are below.
+ */
+
+#define KVM_REG_MIPS_R0 0
+#define KVM_REG_MIPS_R1 1
+#define KVM_REG_MIPS_R2 2
+#define KVM_REG_MIPS_R3 3
+#define KVM_REG_MIPS_R4 4
+#define KVM_REG_MIPS_R5 5
+#define KVM_REG_MIPS_R6 6
+#define KVM_REG_MIPS_R7 7
+#define KVM_REG_MIPS_R8 8
+#define KVM_REG_MIPS_R9 9
+#define KVM_REG_MIPS_R10 10
+#define KVM_REG_MIPS_R11 11
+#define KVM_REG_MIPS_R12 12
+#define KVM_REG_MIPS_R13 13
+#define KVM_REG_MIPS_R14 14
+#define KVM_REG_MIPS_R15 15
+#define KVM_REG_MIPS_R16 16
+#define KVM_REG_MIPS_R17 17
+#define KVM_REG_MIPS_R18 18
+#define KVM_REG_MIPS_R19 19
+#define KVM_REG_MIPS_R20 20
+#define KVM_REG_MIPS_R21 21
+#define KVM_REG_MIPS_R22 22
+#define KVM_REG_MIPS_R23 23
+#define KVM_REG_MIPS_R24 24
+#define KVM_REG_MIPS_R25 25
+#define KVM_REG_MIPS_R26 26
+#define KVM_REG_MIPS_R27 27
+#define KVM_REG_MIPS_R28 28
+#define KVM_REG_MIPS_R29 29
+#define KVM_REG_MIPS_R30 30
+#define KVM_REG_MIPS_R31 31
+
+#define KVM_REG_MIPS_HI 32
+#define KVM_REG_MIPS_LO 33
+#define KVM_REG_MIPS_PC 34
+
+/*
+ * KVM MIPS specific structures and definitions
+ *
+ */
struct kvm_debug_exit_arch {
+ __u64 epc;
};

/* for KVM_SET_GUEST_DEBUG */
struct kvm_guest_debug_arch {
};

+/* definition of registers in kvm_run */
+struct kvm_sync_regs {
+};
+
+/* dummy definition */
+struct kvm_sregs {
+};
+
struct kvm_mips_interrupt {
/* in */
__u32 cpu;
__u32 irq;
};

-/* definition of registers in kvm_run */
-struct kvm_sync_regs {
-};
-
#endif /* __LINUX_KVM_MIPS_H */
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 143875c..4d6fa0b 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -496,10 +496,6 @@ struct kvm_mips_callbacks {
uint32_t cause);
int (*irq_clear) (struct kvm_vcpu *vcpu, unsigned int priority,
uint32_t cause);
- int (*vcpu_ioctl_get_regs) (struct kvm_vcpu *vcpu,
- struct kvm_regs *regs);
- int (*vcpu_ioctl_set_regs) (struct kvm_vcpu *vcpu,
- struct kvm_regs *regs);
};
extern struct kvm_mips_callbacks *kvm_mips_callbacks;
int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks);
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 71a1fc1..5aec4f4 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -485,15 +485,251 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
return -EINVAL;
}

+#define KVM_REG_MIPS_CP0_INDEX (0x10000 + 8 * 0 + 0)
+#define KVM_REG_MIPS_CP0_ENTRYLO0 (0x10000 + 8 * 2 + 0)
+#define KVM_REG_MIPS_CP0_ENTRYLO1 (0x10000 + 8 * 3 + 0)
+#define KVM_REG_MIPS_CP0_CONTEXT (0x10000 + 8 * 4 + 0)
+#define KVM_REG_MIPS_CP0_USERLOCAL (0x10000 + 8 * 4 + 2)
+#define KVM_REG_MIPS_CP0_PAGEMASK (0x10000 + 8 * 5 + 0)
+#define KVM_REG_MIPS_CP0_PAGEGRAIN (0x10000 + 8 * 5 + 1)
+#define KVM_REG_MIPS_CP0_WIRED (0x10000 + 8 * 6 + 0)
+#define KVM_REG_MIPS_CP0_HWRENA (0x10000 + 8 * 7 + 0)
+#define KVM_REG_MIPS_CP0_BADVADDR (0x10000 + 8 * 8 + 0)
+#define KVM_REG_MIPS_CP0_COUNT (0x10000 + 8 * 9 + 0)
+#define KVM_REG_MIPS_CP0_ENTRYHI (0x10000 + 8 * 10 + 0)
+#define KVM_REG_MIPS_CP0_COMPARE (0x10000 + 8 * 11 + 0)
+#define KVM_REG_MIPS_CP0_STATUS (0x10000 + 8 * 12 + 0)
+#define KVM_REG_MIPS_CP0_CAUSE (0x10000 + 8 * 13 + 0)
+#define KVM_REG_MIPS_CP0_EBASE (0x10000 + 8 * 15 + 1)
+#define KVM_REG_MIPS_CP0_CONFIG (0x10000 + 8 * 16 + 0)
+#define KVM_REG_MIPS_CP0_CONFIG1 (0x10000 + 8 * 16 + 1)
+#define KVM_REG_MIPS_CP0_CONFIG2 (0x10000 + 8 * 16 + 2)
+#define KVM_REG_MIPS_CP0_CONFIG3 (0x10000 + 8 * 16 + 3)
+#define KVM_REG_MIPS_CP0_CONFIG7 (0x10000 + 8 * 16 + 7)
+#define KVM_REG_MIPS_CP0_XCONTEXT (0x10000 + 8 * 20 + 0)
+#define KVM_REG_MIPS_CP0_ERROREPC (0x10000 + 8 * 30 + 0)
+
+static u64 kvm_mips_get_one_regs[] = {
+ KVM_REG_MIPS_R0,
+ KVM_REG_MIPS_R1,
+ KVM_REG_MIPS_R2,
+ KVM_REG_MIPS_R3,
+ KVM_REG_MIPS_R4,
+ KVM_REG_MIPS_R5,
+ KVM_REG_MIPS_R6,
+ KVM_REG_MIPS_R7,
+ KVM_REG_MIPS_R8,
+ KVM_REG_MIPS_R9,
+ KVM_REG_MIPS_R10,
+ KVM_REG_MIPS_R11,
+ KVM_REG_MIPS_R12,
+ KVM_REG_MIPS_R13,
+ KVM_REG_MIPS_R14,
+ KVM_REG_MIPS_R15,
+ KVM_REG_MIPS_R16,
+ KVM_REG_MIPS_R17,
+ KVM_REG_MIPS_R18,
+ KVM_REG_MIPS_R19,
+ KVM_REG_MIPS_R20,
+ KVM_REG_MIPS_R21,
+ KVM_REG_MIPS_R22,
+ KVM_REG_MIPS_R23,
+ KVM_REG_MIPS_R24,
+ KVM_REG_MIPS_R25,
+ KVM_REG_MIPS_R26,
+ KVM_REG_MIPS_R27,
+ KVM_REG_MIPS_R28,
+ KVM_REG_MIPS_R29,
+ KVM_REG_MIPS_R30,
+ KVM_REG_MIPS_R31,
+
+ KVM_REG_MIPS_HI,
+ KVM_REG_MIPS_LO,
+ KVM_REG_MIPS_PC,
+
+ KVM_REG_MIPS_CP0_INDEX,
+ KVM_REG_MIPS_CP0_CONTEXT,
+ KVM_REG_MIPS_CP0_PAGEMASK,
+ KVM_REG_MIPS_CP0_WIRED,
+ KVM_REG_MIPS_CP0_BADVADDR,
+ KVM_REG_MIPS_CP0_ENTRYHI,
+ KVM_REG_MIPS_CP0_STATUS,
+ KVM_REG_MIPS_CP0_CAUSE,
+ /* EPC set via kvm_regs, et al. */
+ KVM_REG_MIPS_CP0_CONFIG,
+ KVM_REG_MIPS_CP0_CONFIG1,
+ KVM_REG_MIPS_CP0_CONFIG2,
+ KVM_REG_MIPS_CP0_CONFIG3,
+ KVM_REG_MIPS_CP0_CONFIG7,
+ KVM_REG_MIPS_CP0_ERROREPC
+};
+
+static int kvm_mips_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
+{
+ u64 __user *uaddr = (u64 __user *)(long)reg->addr;
+
+ struct mips_coproc *cop0 = vcpu->arch.cop0;
+ s64 v;
+
+ switch (reg->id) {
+ case KVM_REG_MIPS_R0 ... KVM_REG_MIPS_R31:
+ v = (long)vcpu->arch.gprs[reg->id - KVM_REG_MIPS_R0];
+ break;
+ case KVM_REG_MIPS_HI:
+ v = (long)vcpu->arch.hi;
+ break;
+ case KVM_REG_MIPS_LO:
+ v = (long)vcpu->arch.lo;
+ break;
+ case KVM_REG_MIPS_PC:
+ v = (long)vcpu->arch.pc;
+ break;
+
+ case KVM_REG_MIPS_CP0_INDEX:
+ v = (long)kvm_read_c0_guest_index(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CONTEXT:
+ v = (long)kvm_read_c0_guest_context(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_PAGEMASK:
+ v = (long)kvm_read_c0_guest_pagemask(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_WIRED:
+ v = (long)kvm_read_c0_guest_wired(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_BADVADDR:
+ v = (long)kvm_read_c0_guest_badvaddr(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_ENTRYHI:
+ v = (long)kvm_read_c0_guest_entryhi(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_STATUS:
+ v = (long)kvm_read_c0_guest_status(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CAUSE:
+ v = (long)kvm_read_c0_guest_cause(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_ERROREPC:
+ v = (long)kvm_read_c0_guest_errorepc(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CONFIG:
+ v = (long)kvm_read_c0_guest_config(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CONFIG1:
+ v = (long)kvm_read_c0_guest_config1(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CONFIG2:
+ v = (long)kvm_read_c0_guest_config2(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CONFIG3:
+ v = (long)kvm_read_c0_guest_config3(cop0);
+ break;
+ case KVM_REG_MIPS_CP0_CONFIG7:
+ v = (long)kvm_read_c0_guest_config7(cop0);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return put_user(v, uaddr);
+}
+
+static int kvm_mips_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
+{
+ u64 __user *uaddr = (u64 __user *)(long)reg->addr;
+ struct mips_coproc *cop0 = vcpu->arch.cop0;
+ u64 v;
+
+ if (get_user(v, uaddr) != 0)
+ return -EFAULT;
+
+ switch (reg->id) {
+ case KVM_REG_MIPS_R0:
+ /* Silently ignore requests to set $0 */
+ break;
+ case KVM_REG_MIPS_R1 ... KVM_REG_MIPS_R31:
+ vcpu->arch.gprs[reg->id - KVM_REG_MIPS_R0] = v;
+ break;
+ case KVM_REG_MIPS_HI:
+ vcpu->arch.hi = v;
+ break;
+ case KVM_REG_MIPS_LO:
+ vcpu->arch.lo = v;
+ break;
+ case KVM_REG_MIPS_PC:
+ vcpu->arch.pc = v;
+ break;
+
+ case KVM_REG_MIPS_CP0_INDEX:
+ kvm_write_c0_guest_index(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_CONTEXT:
+ kvm_write_c0_guest_context(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_PAGEMASK:
+ kvm_write_c0_guest_pagemask(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_WIRED:
+ kvm_write_c0_guest_wired(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_BADVADDR:
+ kvm_write_c0_guest_badvaddr(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_ENTRYHI:
+ kvm_write_c0_guest_entryhi(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_STATUS:
+ kvm_write_c0_guest_status(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_CAUSE:
+ kvm_write_c0_guest_cause(cop0, v);
+ break;
+ case KVM_REG_MIPS_CP0_ERROREPC:
+ kvm_write_c0_guest_errorepc(cop0, v);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
long
kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
{
struct kvm_vcpu *vcpu = filp->private_data;
void __user *argp = (void __user *)arg;
long r;
- int intr;

switch (ioctl) {
+ case KVM_SET_ONE_REG:
+ case KVM_GET_ONE_REG: {
+ struct kvm_one_reg reg;
+ if (copy_from_user(&reg, argp, sizeof(reg)))
+ return -EFAULT;
+ if (ioctl == KVM_SET_ONE_REG)
+ return kvm_mips_set_reg(vcpu, &reg);
+ else
+ return kvm_mips_get_reg(vcpu, &reg);
+ }
+ case KVM_GET_REG_LIST: {
+ struct kvm_reg_list __user *user_list = argp;
+ u64 __user *reg_dest;
+ struct kvm_reg_list reg_list;
+ unsigned n;
+
+ if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
+ return -EFAULT;
+ n = reg_list.n;
+ reg_list.n = ARRAY_SIZE(kvm_mips_get_one_regs);
+ if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
+ return -EFAULT;
+ if (n < reg_list.n)
+ return -E2BIG;
+ reg_dest = user_list->reg;
+ if (copy_to_user(reg_dest, kvm_mips_get_one_regs,
+ sizeof(kvm_mips_get_one_regs)))
+ return -EFAULT;
+ return 0;
+ }
case KVM_NMI:
/* Treat the NMI as a CPU reset */
r = kvm_mips_reset_vcpu(vcpu);
@@ -505,8 +741,6 @@ kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
if (copy_from_user(&irq, argp, sizeof(irq)))
goto out;

- intr = (int)irq.irq;
-
kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
irq.irq);

@@ -514,7 +748,7 @@ kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
break;
}
default:
- r = -EINVAL;
+ r = -ENOIOCTLCMD;
}

out:
@@ -627,6 +861,9 @@ int kvm_dev_ioctl_check_extension(long ext)
int r;

switch (ext) {
+ case KVM_CAP_ONE_REG:
+ r = 1;
+ break;
case KVM_CAP_COALESCED_MMIO:
r = KVM_COALESCED_MMIO_PAGE_OFFSET;
break;
@@ -635,7 +872,6 @@ int kvm_dev_ioctl_check_extension(long ext)
break;
}
return r;
-
}

int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
@@ -684,7 +920,7 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
vcpu->arch.lo = regs->lo;
vcpu->arch.pc = regs->pc;

- return kvm_mips_callbacks->vcpu_ioctl_set_regs(vcpu, regs);
+ return 0;
}

int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
@@ -698,7 +934,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
regs->lo = vcpu->arch.lo;
regs->pc = vcpu->arch.pc;

- return kvm_mips_callbacks->vcpu_ioctl_get_regs(vcpu, regs);
+ return 0;
}

void kvm_mips_comparecount_func(unsigned long data)
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index 466aeef..30d7253 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -345,54 +345,6 @@ static int kvm_trap_emul_handle_break(struct kvm_vcpu *vcpu)
return ret;
}

-static int
-kvm_trap_emul_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
-{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
-
- kvm_write_c0_guest_index(cop0, regs->cp0reg[MIPS_CP0_TLB_INDEX][0]);
- kvm_write_c0_guest_context(cop0, regs->cp0reg[MIPS_CP0_TLB_CONTEXT][0]);
- kvm_write_c0_guest_badvaddr(cop0, regs->cp0reg[MIPS_CP0_BAD_VADDR][0]);
- kvm_write_c0_guest_entryhi(cop0, regs->cp0reg[MIPS_CP0_TLB_HI][0]);
- kvm_write_c0_guest_epc(cop0, regs->cp0reg[MIPS_CP0_EXC_PC][0]);
-
- kvm_write_c0_guest_status(cop0, regs->cp0reg[MIPS_CP0_STATUS][0]);
- kvm_write_c0_guest_cause(cop0, regs->cp0reg[MIPS_CP0_CAUSE][0]);
- kvm_write_c0_guest_pagemask(cop0,
- regs->cp0reg[MIPS_CP0_TLB_PG_MASK][0]);
- kvm_write_c0_guest_wired(cop0, regs->cp0reg[MIPS_CP0_TLB_WIRED][0]);
- kvm_write_c0_guest_errorepc(cop0, regs->cp0reg[MIPS_CP0_ERROR_PC][0]);
-
- return 0;
-}
-
-static int
-kvm_trap_emul_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
-{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
-
- regs->cp0reg[MIPS_CP0_TLB_INDEX][0] = kvm_read_c0_guest_index(cop0);
- regs->cp0reg[MIPS_CP0_TLB_CONTEXT][0] = kvm_read_c0_guest_context(cop0);
- regs->cp0reg[MIPS_CP0_BAD_VADDR][0] = kvm_read_c0_guest_badvaddr(cop0);
- regs->cp0reg[MIPS_CP0_TLB_HI][0] = kvm_read_c0_guest_entryhi(cop0);
- regs->cp0reg[MIPS_CP0_EXC_PC][0] = kvm_read_c0_guest_epc(cop0);
-
- regs->cp0reg[MIPS_CP0_STATUS][0] = kvm_read_c0_guest_status(cop0);
- regs->cp0reg[MIPS_CP0_CAUSE][0] = kvm_read_c0_guest_cause(cop0);
- regs->cp0reg[MIPS_CP0_TLB_PG_MASK][0] =
- kvm_read_c0_guest_pagemask(cop0);
- regs->cp0reg[MIPS_CP0_TLB_WIRED][0] = kvm_read_c0_guest_wired(cop0);
- regs->cp0reg[MIPS_CP0_ERROR_PC][0] = kvm_read_c0_guest_errorepc(cop0);
-
- regs->cp0reg[MIPS_CP0_CONFIG][0] = kvm_read_c0_guest_config(cop0);
- regs->cp0reg[MIPS_CP0_CONFIG][1] = kvm_read_c0_guest_config1(cop0);
- regs->cp0reg[MIPS_CP0_CONFIG][2] = kvm_read_c0_guest_config2(cop0);
- regs->cp0reg[MIPS_CP0_CONFIG][3] = kvm_read_c0_guest_config3(cop0);
- regs->cp0reg[MIPS_CP0_CONFIG][7] = kvm_read_c0_guest_config7(cop0);
-
- return 0;
-}
-
static int kvm_trap_emul_vm_init(struct kvm *kvm)
{
return 0;
@@ -471,8 +423,6 @@ static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
.dequeue_io_int = kvm_mips_dequeue_io_int_cb,
.irq_deliver = kvm_mips_irq_deliver_cb,
.irq_clear = kvm_mips_irq_clear_cb,
- .vcpu_ioctl_get_regs = kvm_trap_emul_ioctl_get_regs,
- .vcpu_ioctl_set_regs = kvm_trap_emul_ioctl_set_regs,
};

int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks)
--
1.7.11.7

2013-05-22 18:46:51

by David Daney

[permalink] [raw]
Subject: [PATCH v5 3/6] mips/kvm: Fix name of gpr field in struct kvm_regs.

From: David Daney <[email protected]>

Signed-off-by: David Daney <[email protected]>
---
arch/mips/include/asm/kvm.h | 3 ++-
arch/mips/kvm/kvm_mips.c | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
index 86812fb..d145ead 100644
--- a/arch/mips/include/asm/kvm.h
+++ b/arch/mips/include/asm/kvm.h
@@ -26,7 +26,8 @@
* extended to 64-bits.
*/
struct kvm_regs {
- __u64 gprs[32];
+ /* out (KVM_GET_REGS) / in (KVM_SET_REGS) */
+ __u64 gpr[32];
__u64 hi;
__u64 lo;
__u64 pc;
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index e0dad02..93da750 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -678,7 +678,7 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
int i;

for (i = 0; i < 32; i++)
- vcpu->arch.gprs[i] = regs->gprs[i];
+ vcpu->arch.gprs[i] = regs->gpr[i];

vcpu->arch.hi = regs->hi;
vcpu->arch.lo = regs->lo;
@@ -692,7 +692,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
int i;

for (i = 0; i < 32; i++)
- regs->gprs[i] = vcpu->arch.gprs[i];
+ regs->gpr[i] = vcpu->arch.gprs[i];

regs->hi = vcpu->arch.hi;
regs->lo = vcpu->arch.lo;
--
1.7.11.7

2013-05-22 19:05:57

by Sanjay Lal

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] mips/kvm: Fix ABI for compatibility with 64-bit guests.


On May 22, 2013, at 11:43 AM, David Daney wrote:

> From: David Daney <[email protected]>
>
> The initial patch set implementing MIPS KVM does not handle 64-bit
> guests or use of the FPU. This patch set corrects these ABI issues,
> and does some very minor clean up.
>
> Changes from v4: No code change, just keep more of the code in
> kvm_mips.c rather than kvm_trap_emul.c
>
> Changes from v3: Use KVM_SET_ONE_REG instead of KVM_SET_MSRS. Added
> ENOIOCTLCMD patch.
>
> Changes from v2: Split into five parts, no code change.
>
> David Daney (6):
> mips/kvm: Fix ABI for use of FPU.
> mips/kvm: Fix ABI for use of 64-bit registers.
> mips/kvm: Fix name of gpr field in struct kvm_regs.
> mips/kvm: Use ARRAY_SIZE() instead of hardcoded constants in
> kvm_arch_vcpu_ioctl_{s,g}et_regs
> mips/kvm: Fix ABI by moving manipulation of CP0 registers to
> KVM_{G,S}ET_ONE_REG
> mips/kvm: Use ENOIOCTLCMD to indicate unimplemented ioctls.
>
> arch/mips/include/asm/kvm.h | 137 +++++++++++++++----
> arch/mips/include/asm/kvm_host.h | 4 -
> arch/mips/kvm/kvm_mips.c | 278 ++++++++++++++++++++++++++++++++++++---
> arch/mips/kvm/kvm_trap_emul.c | 50 -------
> 4 files changed, 367 insertions(+), 102 deletions(-)
>
> --
> 1.7.11.7

Acked-by: Sanjay Lal <[email protected]>

Regards
Sanjay

2013-05-23 10:28:50

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] mips/kvm: Fix ABI for compatibility with 64-bit guests.

On Wed, May 22, 2013 at 11:43:50AM -0700, David Daney wrote:
> From: David Daney <[email protected]>
>
Please regenerate against master. arch/mips/include/asm/kvm.h does not
exists any more.

> The initial patch set implementing MIPS KVM does not handle 64-bit
> guests or use of the FPU. This patch set corrects these ABI issues,
> and does some very minor clean up.
>
> Changes from v4: No code change, just keep more of the code in
> kvm_mips.c rather than kvm_trap_emul.c
>
> Changes from v3: Use KVM_SET_ONE_REG instead of KVM_SET_MSRS. Added
> ENOIOCTLCMD patch.
>
> Changes from v2: Split into five parts, no code change.
>
> David Daney (6):
> mips/kvm: Fix ABI for use of FPU.
> mips/kvm: Fix ABI for use of 64-bit registers.
> mips/kvm: Fix name of gpr field in struct kvm_regs.
> mips/kvm: Use ARRAY_SIZE() instead of hardcoded constants in
> kvm_arch_vcpu_ioctl_{s,g}et_regs
> mips/kvm: Fix ABI by moving manipulation of CP0 registers to
> KVM_{G,S}ET_ONE_REG
> mips/kvm: Use ENOIOCTLCMD to indicate unimplemented ioctls.
>
> arch/mips/include/asm/kvm.h | 137 +++++++++++++++----
> arch/mips/include/asm/kvm_host.h | 4 -
> arch/mips/kvm/kvm_mips.c | 278 ++++++++++++++++++++++++++++++++++++---
> arch/mips/kvm/kvm_trap_emul.c | 50 -------
> 4 files changed, 367 insertions(+), 102 deletions(-)
>
> --
> 1.7.11.7

--
Gleb.

2013-05-23 16:58:14

by David Daney

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] mips/kvm: Fix ABI for compatibility with 64-bit guests.

On 05/23/2013 03:28 AM, Gleb Natapov wrote:
> On Wed, May 22, 2013 at 11:43:50AM -0700, David Daney wrote:
>> From: David Daney <[email protected]>
>>
> Please regenerate against master. arch/mips/include/asm/kvm.h does not
> exists any more.

New patch sent. I gather from this message, that you want to merge this
particular set via your tree instead of Ralf's MIPS tree. That's fine
with me. However, we were hoping to have these ABI fixing patches
pushed to Linus before 3.10 releases, so that there don't exist kernel
versions with the defective ABI.

Future patch sets may affect core MIPS architecture files, and therefore
may not be so amenable to merging via the KVM tree. We will have to
figure out how to handle that.

David Daney


2013-05-23 17:09:39

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] mips/kvm: Fix ABI for compatibility with 64-bit guests.

On Thu, May 23, 2013 at 09:58:00AM -0700, David Daney wrote:
> On 05/23/2013 03:28 AM, Gleb Natapov wrote:
> >On Wed, May 22, 2013 at 11:43:50AM -0700, David Daney wrote:
> >>From: David Daney <[email protected]>
> >>
> >Please regenerate against master. arch/mips/include/asm/kvm.h does not
> >exists any more.
>
> New patch sent. I gather from this message, that you want to merge
> this particular set via your tree instead of Ralf's MIPS tree.
> That's fine with me. However, we were hoping to have these ABI
> fixing patches pushed to Linus before 3.10 releases, so that there
> don't exist kernel versions with the defective ABI.
>
I do plan to push them to 3.10 that is why I asked to generate them
against master ("next" branch is for next merge window)

> Future patch sets may affect core MIPS architecture files, and
> therefore may not be so amenable to merging via the KVM tree. We
> will have to figure out how to handle that.
>
Are you talking about initial MIPS HW vitalization support? Lets see
the patch series. If core changes are to big for for Ralf to ack them
and push through kvm.git then we can push them via MPIS tree.

--
Gleb.

2013-06-08 11:05:27

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5 5/6] mips/kvm: Fix ABI by moving manipulation of CP0 registers to KVM_{G,S}ET_ONE_REG

On Wed, May 22, 2013 at 11:43:55AM -0700, David Daney wrote:
> From: David Daney <[email protected]>
>
> Because not all 256 CP0 registers are ever implemented, we need a
> different method of manipulating them. Use the
> KVM_SET_ONE_REG/KVM_GET_ONE_REG mechanism.
>
> Now unused code and definitions are removed.
>
Just noticed that KVM_REG_MIPS_ definitions are wrong. You need to
define KVM_REG_MIPS in include/uapi/linux/kvm.h (please use
0x7000000000000000ULL as 0x6000000000000000ULL is reserved for ARM64)
and define all KVM_REG_MIPS_ values as "KVM_REG_MIPS | value". Can you
send a patch to do that ASAP?

> Signed-off-by: David Daney <[email protected]>
> ---
> arch/mips/include/asm/kvm.h | 91 +++++++++++---
> arch/mips/include/asm/kvm_host.h | 4 -
> arch/mips/kvm/kvm_mips.c | 250 +++++++++++++++++++++++++++++++++++++--
> arch/mips/kvm/kvm_trap_emul.c | 50 --------
> 4 files changed, 320 insertions(+), 75 deletions(-)
>
> diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
> index d145ead..3f424f5 100644
> --- a/arch/mips/include/asm/kvm.h
> +++ b/arch/mips/include/asm/kvm.h
> @@ -13,10 +13,11 @@
>
> #include <linux/types.h>
>
> -#define __KVM_MIPS
> -
> -#define N_MIPS_COPROC_REGS 32
> -#define N_MIPS_COPROC_SEL 8
> +/*
> + * KVM MIPS specific structures and definitions.
> + *
> + * Some parts derived from the x86 version of this file.
> + */
>
> /*
> * for KVM_GET_REGS and KVM_SET_REGS
> @@ -31,12 +32,6 @@ struct kvm_regs {
> __u64 hi;
> __u64 lo;
> __u64 pc;
> -
> - __u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
> -};
> -
> -/* for KVM_GET_SREGS and KVM_SET_SREGS */
> -struct kvm_sregs {
> };
>
> /*
> @@ -55,21 +50,89 @@ struct kvm_fpu {
> __u32 pad;
> };
>
> +
> +/*
> + * For MIPS, we use KVM_SET_ONE_REG and KVM_GET_ONE_REG to access CP0
> + * registers. The id field is broken down as follows:
> + *
> + * bits[2..0] - Register 'sel' index.
> + * bits[7..3] - Register 'rd' index.
> + * bits[15..8] - Must be zero.
> + * bits[63..16] - 1 -> CP0 registers.
> + *
> + * Other sets registers may be added in the future. Each set would
> + * have its own identifier in bits[63..16].
> + *
> + * The addr field of struct kvm_one_reg must point to an aligned
> + * 64-bit wide location. For registers that are narrower than
> + * 64-bits, the value is stored in the low order bits of the location,
> + * and sign extended to 64-bits.
> + *
> + * The registers defined in struct kvm_regs are also accessible, the
> + * id values for these are below.
> + */
> +
> +#define KVM_REG_MIPS_R0 0
> +#define KVM_REG_MIPS_R1 1
> +#define KVM_REG_MIPS_R2 2
> +#define KVM_REG_MIPS_R3 3
> +#define KVM_REG_MIPS_R4 4
> +#define KVM_REG_MIPS_R5 5
> +#define KVM_REG_MIPS_R6 6
> +#define KVM_REG_MIPS_R7 7
> +#define KVM_REG_MIPS_R8 8
> +#define KVM_REG_MIPS_R9 9
> +#define KVM_REG_MIPS_R10 10
> +#define KVM_REG_MIPS_R11 11
> +#define KVM_REG_MIPS_R12 12
> +#define KVM_REG_MIPS_R13 13
> +#define KVM_REG_MIPS_R14 14
> +#define KVM_REG_MIPS_R15 15
> +#define KVM_REG_MIPS_R16 16
> +#define KVM_REG_MIPS_R17 17
> +#define KVM_REG_MIPS_R18 18
> +#define KVM_REG_MIPS_R19 19
> +#define KVM_REG_MIPS_R20 20
> +#define KVM_REG_MIPS_R21 21
> +#define KVM_REG_MIPS_R22 22
> +#define KVM_REG_MIPS_R23 23
> +#define KVM_REG_MIPS_R24 24
> +#define KVM_REG_MIPS_R25 25
> +#define KVM_REG_MIPS_R26 26
> +#define KVM_REG_MIPS_R27 27
> +#define KVM_REG_MIPS_R28 28
> +#define KVM_REG_MIPS_R29 29
> +#define KVM_REG_MIPS_R30 30
> +#define KVM_REG_MIPS_R31 31
> +
> +#define KVM_REG_MIPS_HI 32
> +#define KVM_REG_MIPS_LO 33
> +#define KVM_REG_MIPS_PC 34
> +
> +/*
> + * KVM MIPS specific structures and definitions
> + *
> + */
> struct kvm_debug_exit_arch {
> + __u64 epc;
> };
>
> /* for KVM_SET_GUEST_DEBUG */
> struct kvm_guest_debug_arch {
> };
>
> +/* definition of registers in kvm_run */
> +struct kvm_sync_regs {
> +};
> +
> +/* dummy definition */
> +struct kvm_sregs {
> +};
> +
> struct kvm_mips_interrupt {
> /* in */
> __u32 cpu;
> __u32 irq;
> };
>
> -/* definition of registers in kvm_run */
> -struct kvm_sync_regs {
> -};
> -
> #endif /* __LINUX_KVM_MIPS_H */
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index 143875c..4d6fa0b 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -496,10 +496,6 @@ struct kvm_mips_callbacks {
> uint32_t cause);
> int (*irq_clear) (struct kvm_vcpu *vcpu, unsigned int priority,
> uint32_t cause);
> - int (*vcpu_ioctl_get_regs) (struct kvm_vcpu *vcpu,
> - struct kvm_regs *regs);
> - int (*vcpu_ioctl_set_regs) (struct kvm_vcpu *vcpu,
> - struct kvm_regs *regs);
> };
> extern struct kvm_mips_callbacks *kvm_mips_callbacks;
> int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks);
> diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
> index 71a1fc1..5aec4f4 100644
> --- a/arch/mips/kvm/kvm_mips.c
> +++ b/arch/mips/kvm/kvm_mips.c
> @@ -485,15 +485,251 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> +#define KVM_REG_MIPS_CP0_INDEX (0x10000 + 8 * 0 + 0)
> +#define KVM_REG_MIPS_CP0_ENTRYLO0 (0x10000 + 8 * 2 + 0)
> +#define KVM_REG_MIPS_CP0_ENTRYLO1 (0x10000 + 8 * 3 + 0)
> +#define KVM_REG_MIPS_CP0_CONTEXT (0x10000 + 8 * 4 + 0)
> +#define KVM_REG_MIPS_CP0_USERLOCAL (0x10000 + 8 * 4 + 2)
> +#define KVM_REG_MIPS_CP0_PAGEMASK (0x10000 + 8 * 5 + 0)
> +#define KVM_REG_MIPS_CP0_PAGEGRAIN (0x10000 + 8 * 5 + 1)
> +#define KVM_REG_MIPS_CP0_WIRED (0x10000 + 8 * 6 + 0)
> +#define KVM_REG_MIPS_CP0_HWRENA (0x10000 + 8 * 7 + 0)
> +#define KVM_REG_MIPS_CP0_BADVADDR (0x10000 + 8 * 8 + 0)
> +#define KVM_REG_MIPS_CP0_COUNT (0x10000 + 8 * 9 + 0)
> +#define KVM_REG_MIPS_CP0_ENTRYHI (0x10000 + 8 * 10 + 0)
> +#define KVM_REG_MIPS_CP0_COMPARE (0x10000 + 8 * 11 + 0)
> +#define KVM_REG_MIPS_CP0_STATUS (0x10000 + 8 * 12 + 0)
> +#define KVM_REG_MIPS_CP0_CAUSE (0x10000 + 8 * 13 + 0)
> +#define KVM_REG_MIPS_CP0_EBASE (0x10000 + 8 * 15 + 1)
> +#define KVM_REG_MIPS_CP0_CONFIG (0x10000 + 8 * 16 + 0)
> +#define KVM_REG_MIPS_CP0_CONFIG1 (0x10000 + 8 * 16 + 1)
> +#define KVM_REG_MIPS_CP0_CONFIG2 (0x10000 + 8 * 16 + 2)
> +#define KVM_REG_MIPS_CP0_CONFIG3 (0x10000 + 8 * 16 + 3)
> +#define KVM_REG_MIPS_CP0_CONFIG7 (0x10000 + 8 * 16 + 7)
> +#define KVM_REG_MIPS_CP0_XCONTEXT (0x10000 + 8 * 20 + 0)
> +#define KVM_REG_MIPS_CP0_ERROREPC (0x10000 + 8 * 30 + 0)
> +
> +static u64 kvm_mips_get_one_regs[] = {
> + KVM_REG_MIPS_R0,
> + KVM_REG_MIPS_R1,
> + KVM_REG_MIPS_R2,
> + KVM_REG_MIPS_R3,
> + KVM_REG_MIPS_R4,
> + KVM_REG_MIPS_R5,
> + KVM_REG_MIPS_R6,
> + KVM_REG_MIPS_R7,
> + KVM_REG_MIPS_R8,
> + KVM_REG_MIPS_R9,
> + KVM_REG_MIPS_R10,
> + KVM_REG_MIPS_R11,
> + KVM_REG_MIPS_R12,
> + KVM_REG_MIPS_R13,
> + KVM_REG_MIPS_R14,
> + KVM_REG_MIPS_R15,
> + KVM_REG_MIPS_R16,
> + KVM_REG_MIPS_R17,
> + KVM_REG_MIPS_R18,
> + KVM_REG_MIPS_R19,
> + KVM_REG_MIPS_R20,
> + KVM_REG_MIPS_R21,
> + KVM_REG_MIPS_R22,
> + KVM_REG_MIPS_R23,
> + KVM_REG_MIPS_R24,
> + KVM_REG_MIPS_R25,
> + KVM_REG_MIPS_R26,
> + KVM_REG_MIPS_R27,
> + KVM_REG_MIPS_R28,
> + KVM_REG_MIPS_R29,
> + KVM_REG_MIPS_R30,
> + KVM_REG_MIPS_R31,
> +
> + KVM_REG_MIPS_HI,
> + KVM_REG_MIPS_LO,
> + KVM_REG_MIPS_PC,
> +
> + KVM_REG_MIPS_CP0_INDEX,
> + KVM_REG_MIPS_CP0_CONTEXT,
> + KVM_REG_MIPS_CP0_PAGEMASK,
> + KVM_REG_MIPS_CP0_WIRED,
> + KVM_REG_MIPS_CP0_BADVADDR,
> + KVM_REG_MIPS_CP0_ENTRYHI,
> + KVM_REG_MIPS_CP0_STATUS,
> + KVM_REG_MIPS_CP0_CAUSE,
> + /* EPC set via kvm_regs, et al. */
> + KVM_REG_MIPS_CP0_CONFIG,
> + KVM_REG_MIPS_CP0_CONFIG1,
> + KVM_REG_MIPS_CP0_CONFIG2,
> + KVM_REG_MIPS_CP0_CONFIG3,
> + KVM_REG_MIPS_CP0_CONFIG7,
> + KVM_REG_MIPS_CP0_ERROREPC
> +};
> +
> +static int kvm_mips_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> +{
> + u64 __user *uaddr = (u64 __user *)(long)reg->addr;
> +
> + struct mips_coproc *cop0 = vcpu->arch.cop0;
> + s64 v;
> +
> + switch (reg->id) {
> + case KVM_REG_MIPS_R0 ... KVM_REG_MIPS_R31:
> + v = (long)vcpu->arch.gprs[reg->id - KVM_REG_MIPS_R0];
> + break;
> + case KVM_REG_MIPS_HI:
> + v = (long)vcpu->arch.hi;
> + break;
> + case KVM_REG_MIPS_LO:
> + v = (long)vcpu->arch.lo;
> + break;
> + case KVM_REG_MIPS_PC:
> + v = (long)vcpu->arch.pc;
> + break;
> +
> + case KVM_REG_MIPS_CP0_INDEX:
> + v = (long)kvm_read_c0_guest_index(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CONTEXT:
> + v = (long)kvm_read_c0_guest_context(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_PAGEMASK:
> + v = (long)kvm_read_c0_guest_pagemask(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_WIRED:
> + v = (long)kvm_read_c0_guest_wired(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_BADVADDR:
> + v = (long)kvm_read_c0_guest_badvaddr(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_ENTRYHI:
> + v = (long)kvm_read_c0_guest_entryhi(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_STATUS:
> + v = (long)kvm_read_c0_guest_status(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CAUSE:
> + v = (long)kvm_read_c0_guest_cause(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_ERROREPC:
> + v = (long)kvm_read_c0_guest_errorepc(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CONFIG:
> + v = (long)kvm_read_c0_guest_config(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CONFIG1:
> + v = (long)kvm_read_c0_guest_config1(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CONFIG2:
> + v = (long)kvm_read_c0_guest_config2(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CONFIG3:
> + v = (long)kvm_read_c0_guest_config3(cop0);
> + break;
> + case KVM_REG_MIPS_CP0_CONFIG7:
> + v = (long)kvm_read_c0_guest_config7(cop0);
> + break;
> + default:
> + return -EINVAL;
> + }
> + return put_user(v, uaddr);
> +}
> +
> +static int kvm_mips_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> +{
> + u64 __user *uaddr = (u64 __user *)(long)reg->addr;
> + struct mips_coproc *cop0 = vcpu->arch.cop0;
> + u64 v;
> +
> + if (get_user(v, uaddr) != 0)
> + return -EFAULT;
> +
> + switch (reg->id) {
> + case KVM_REG_MIPS_R0:
> + /* Silently ignore requests to set $0 */
> + break;
> + case KVM_REG_MIPS_R1 ... KVM_REG_MIPS_R31:
> + vcpu->arch.gprs[reg->id - KVM_REG_MIPS_R0] = v;
> + break;
> + case KVM_REG_MIPS_HI:
> + vcpu->arch.hi = v;
> + break;
> + case KVM_REG_MIPS_LO:
> + vcpu->arch.lo = v;
> + break;
> + case KVM_REG_MIPS_PC:
> + vcpu->arch.pc = v;
> + break;
> +
> + case KVM_REG_MIPS_CP0_INDEX:
> + kvm_write_c0_guest_index(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_CONTEXT:
> + kvm_write_c0_guest_context(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_PAGEMASK:
> + kvm_write_c0_guest_pagemask(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_WIRED:
> + kvm_write_c0_guest_wired(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_BADVADDR:
> + kvm_write_c0_guest_badvaddr(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_ENTRYHI:
> + kvm_write_c0_guest_entryhi(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_STATUS:
> + kvm_write_c0_guest_status(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_CAUSE:
> + kvm_write_c0_guest_cause(cop0, v);
> + break;
> + case KVM_REG_MIPS_CP0_ERROREPC:
> + kvm_write_c0_guest_errorepc(cop0, v);
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> long
> kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> {
> struct kvm_vcpu *vcpu = filp->private_data;
> void __user *argp = (void __user *)arg;
> long r;
> - int intr;
>
> switch (ioctl) {
> + case KVM_SET_ONE_REG:
> + case KVM_GET_ONE_REG: {
> + struct kvm_one_reg reg;
> + if (copy_from_user(&reg, argp, sizeof(reg)))
> + return -EFAULT;
> + if (ioctl == KVM_SET_ONE_REG)
> + return kvm_mips_set_reg(vcpu, &reg);
> + else
> + return kvm_mips_get_reg(vcpu, &reg);
> + }
> + case KVM_GET_REG_LIST: {
> + struct kvm_reg_list __user *user_list = argp;
> + u64 __user *reg_dest;
> + struct kvm_reg_list reg_list;
> + unsigned n;
> +
> + if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
> + return -EFAULT;
> + n = reg_list.n;
> + reg_list.n = ARRAY_SIZE(kvm_mips_get_one_regs);
> + if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
> + return -EFAULT;
> + if (n < reg_list.n)
> + return -E2BIG;
> + reg_dest = user_list->reg;
> + if (copy_to_user(reg_dest, kvm_mips_get_one_regs,
> + sizeof(kvm_mips_get_one_regs)))
> + return -EFAULT;
> + return 0;
> + }
> case KVM_NMI:
> /* Treat the NMI as a CPU reset */
> r = kvm_mips_reset_vcpu(vcpu);
> @@ -505,8 +741,6 @@ kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> if (copy_from_user(&irq, argp, sizeof(irq)))
> goto out;
>
> - intr = (int)irq.irq;
> -
> kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
> irq.irq);
>
> @@ -514,7 +748,7 @@ kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> break;
> }
> default:
> - r = -EINVAL;
> + r = -ENOIOCTLCMD;
> }
>
> out:
> @@ -627,6 +861,9 @@ int kvm_dev_ioctl_check_extension(long ext)
> int r;
>
> switch (ext) {
> + case KVM_CAP_ONE_REG:
> + r = 1;
> + break;
> case KVM_CAP_COALESCED_MMIO:
> r = KVM_COALESCED_MMIO_PAGE_OFFSET;
> break;
> @@ -635,7 +872,6 @@ int kvm_dev_ioctl_check_extension(long ext)
> break;
> }
> return r;
> -
> }
>
> int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
> @@ -684,7 +920,7 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> vcpu->arch.lo = regs->lo;
> vcpu->arch.pc = regs->pc;
>
> - return kvm_mips_callbacks->vcpu_ioctl_set_regs(vcpu, regs);
> + return 0;
> }
>
> int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> @@ -698,7 +934,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> regs->lo = vcpu->arch.lo;
> regs->pc = vcpu->arch.pc;
>
> - return kvm_mips_callbacks->vcpu_ioctl_get_regs(vcpu, regs);
> + return 0;
> }
>
> void kvm_mips_comparecount_func(unsigned long data)
> diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
> index 466aeef..30d7253 100644
> --- a/arch/mips/kvm/kvm_trap_emul.c
> +++ b/arch/mips/kvm/kvm_trap_emul.c
> @@ -345,54 +345,6 @@ static int kvm_trap_emul_handle_break(struct kvm_vcpu *vcpu)
> return ret;
> }
>
> -static int
> -kvm_trap_emul_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> -{
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> -
> - kvm_write_c0_guest_index(cop0, regs->cp0reg[MIPS_CP0_TLB_INDEX][0]);
> - kvm_write_c0_guest_context(cop0, regs->cp0reg[MIPS_CP0_TLB_CONTEXT][0]);
> - kvm_write_c0_guest_badvaddr(cop0, regs->cp0reg[MIPS_CP0_BAD_VADDR][0]);
> - kvm_write_c0_guest_entryhi(cop0, regs->cp0reg[MIPS_CP0_TLB_HI][0]);
> - kvm_write_c0_guest_epc(cop0, regs->cp0reg[MIPS_CP0_EXC_PC][0]);
> -
> - kvm_write_c0_guest_status(cop0, regs->cp0reg[MIPS_CP0_STATUS][0]);
> - kvm_write_c0_guest_cause(cop0, regs->cp0reg[MIPS_CP0_CAUSE][0]);
> - kvm_write_c0_guest_pagemask(cop0,
> - regs->cp0reg[MIPS_CP0_TLB_PG_MASK][0]);
> - kvm_write_c0_guest_wired(cop0, regs->cp0reg[MIPS_CP0_TLB_WIRED][0]);
> - kvm_write_c0_guest_errorepc(cop0, regs->cp0reg[MIPS_CP0_ERROR_PC][0]);
> -
> - return 0;
> -}
> -
> -static int
> -kvm_trap_emul_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> -{
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> -
> - regs->cp0reg[MIPS_CP0_TLB_INDEX][0] = kvm_read_c0_guest_index(cop0);
> - regs->cp0reg[MIPS_CP0_TLB_CONTEXT][0] = kvm_read_c0_guest_context(cop0);
> - regs->cp0reg[MIPS_CP0_BAD_VADDR][0] = kvm_read_c0_guest_badvaddr(cop0);
> - regs->cp0reg[MIPS_CP0_TLB_HI][0] = kvm_read_c0_guest_entryhi(cop0);
> - regs->cp0reg[MIPS_CP0_EXC_PC][0] = kvm_read_c0_guest_epc(cop0);
> -
> - regs->cp0reg[MIPS_CP0_STATUS][0] = kvm_read_c0_guest_status(cop0);
> - regs->cp0reg[MIPS_CP0_CAUSE][0] = kvm_read_c0_guest_cause(cop0);
> - regs->cp0reg[MIPS_CP0_TLB_PG_MASK][0] =
> - kvm_read_c0_guest_pagemask(cop0);
> - regs->cp0reg[MIPS_CP0_TLB_WIRED][0] = kvm_read_c0_guest_wired(cop0);
> - regs->cp0reg[MIPS_CP0_ERROR_PC][0] = kvm_read_c0_guest_errorepc(cop0);
> -
> - regs->cp0reg[MIPS_CP0_CONFIG][0] = kvm_read_c0_guest_config(cop0);
> - regs->cp0reg[MIPS_CP0_CONFIG][1] = kvm_read_c0_guest_config1(cop0);
> - regs->cp0reg[MIPS_CP0_CONFIG][2] = kvm_read_c0_guest_config2(cop0);
> - regs->cp0reg[MIPS_CP0_CONFIG][3] = kvm_read_c0_guest_config3(cop0);
> - regs->cp0reg[MIPS_CP0_CONFIG][7] = kvm_read_c0_guest_config7(cop0);
> -
> - return 0;
> -}
> -
> static int kvm_trap_emul_vm_init(struct kvm *kvm)
> {
> return 0;
> @@ -471,8 +423,6 @@ static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
> .dequeue_io_int = kvm_mips_dequeue_io_int_cb,
> .irq_deliver = kvm_mips_irq_deliver_cb,
> .irq_clear = kvm_mips_irq_clear_cb,
> - .vcpu_ioctl_get_regs = kvm_trap_emul_ioctl_get_regs,
> - .vcpu_ioctl_set_regs = kvm_trap_emul_ioctl_set_regs,
> };
>
> int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks)
> --
> 1.7.11.7

--
Gleb.

2013-06-09 22:58:03

by David Daney

[permalink] [raw]
Subject: Re: [PATCH v5 5/6] mips/kvm: Fix ABI by moving manipulation of CP0 registers to KVM_{G,S}ET_ONE_REG

On 06/08/2013 04:05 AM, Gleb Natapov wrote:
> On Wed, May 22, 2013 at 11:43:55AM -0700, David Daney wrote:
>> From: David Daney <[email protected]>
>>
>> Because not all 256 CP0 registers are ever implemented, we need a
>> different method of manipulating them. Use the
>> KVM_SET_ONE_REG/KVM_GET_ONE_REG mechanism.
>>
>> Now unused code and definitions are removed.
>>
> Just noticed that KVM_REG_MIPS_ definitions are wrong. You need to
> define KVM_REG_MIPS in include/uapi/linux/kvm.h (please use
> 0x7000000000000000ULL as 0x6000000000000000ULL is reserved for ARM64)
> and define all KVM_REG_MIPS_ values as "KVM_REG_MIPS | value". Can you
> send a patch to do that ASAP?

It will have to be tomorrow, I cannot test anything today.

Thanks,
David Daney