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

2013-05-22 Thread Sanjay Lal

On May 21, 2013, at 1:54 PM, David Daney wrote:

 From: David Daney david.da...@cavium.com
 
 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.
 
 Code related to implementing KVM_SET_ONE_REG/KVM_GET_ONE_REG is
 consolidated in to kvm_trap_emul.c, now unused code and definitions
 are removed.
 
 Signed-off-by: David Daney david.da...@cavium.com
 ---
 arch/mips/include/asm/kvm.h  |  91 +--
 arch/mips/include/asm/kvm_host.h |   4 -
 arch/mips/kvm/kvm_mips.c |  90 +--
 arch/mips/kvm/kvm_trap_emul.c| 338 ++-
 4 files changed, 383 insertions(+), 140 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_SEL8
 +/*
 + * 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..bc879bd 100644
 --- a/arch/mips/kvm/kvm_mips.c
 +++ b/arch/mips/kvm/kvm_mips.c
 @@ -51,16 +51,6 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
   {NULL}
 };
 
 -static int kvm_mips_reset_vcpu(struct kvm_vcpu *vcpu)
 -{
 - int i;
 - for_each_possible_cpu(i) {
 

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

2013-05-22 Thread David Daney

On 05/22/2013 10:44 AM, Sanjay Lal wrote:


On May 21, 2013, at 1:54 PM, David Daney wrote:


From: David Daney david.da...@cavium.com

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.

Code related to implementing KVM_SET_ONE_REG/KVM_GET_ONE_REG is
consolidated in to kvm_trap_emul.c, now unused code and definitions
are removed.

Signed-off-by: David Daney david.da...@cavium.com
---
arch/mips/include/asm/kvm.h  |  91 +--
arch/mips/include/asm/kvm_host.h |   4 -
arch/mips/kvm/kvm_mips.c |  90 +--
arch/mips/kvm/kvm_trap_emul.c| 338 ++-
4 files changed, 383 insertions(+), 140 deletions(-)

[...]




Most of the functions that have been relocated to kvm_trap_emul.c should stay 
in kvm_mips.c. They are/will shared between the trap and emulate and VZ modes.  
They include kvm_mips_reset_vcpu(), kvm_vcpu_ioctl_interrupt(), 
kvm_arch_vcpu_ioctl().

kvm_mips_get_reg() and kvm_mips_set_reg() should be in kvm_mips.c as they will 
be shared by the trap and emulate and VZ code.



OK, I will revise the patch set to rearrange things in a manner that 
leaves these in kvm_mips.c.  However, this is of secondary importance to 
the question of the suitability of the ABI.




If you plan on defining specific versions of these functions for Cavium's 
implementation of KVM, please make them callbacks.



There will soon be follow on patches that do exactly that.



--
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


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

2013-05-21 Thread David Daney
From: David Daney david.da...@cavium.com

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.

Code related to implementing KVM_SET_ONE_REG/KVM_GET_ONE_REG is
consolidated in to kvm_trap_emul.c, now unused code and definitions
are removed.

Signed-off-by: David Daney david.da...@cavium.com
---
 arch/mips/include/asm/kvm.h  |  91 +--
 arch/mips/include/asm/kvm_host.h |   4 -
 arch/mips/kvm/kvm_mips.c |  90 +--
 arch/mips/kvm/kvm_trap_emul.c| 338 ++-
 4 files changed, 383 insertions(+), 140 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..bc879bd 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -51,16 +51,6 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
{NULL}
 };
 
-static int kvm_mips_reset_vcpu(struct kvm_vcpu *vcpu)
-{
-   int i;
-   for_each_possible_cpu(i) {
-   vcpu-arch.guest_kernel_asid[i] = 0;
-   vcpu-arch.guest_user_asid[i] = 0;
-   }
-   return 0;
-}
-
 gfn_t