All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] KVM/MIPS32: Fixes for Linux 3.10
@ 2013-05-18 13:54 Sanjay Lal
  2013-05-18 13:54 ` [PATCH 1/4] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Sanjay Lal @ 2013-05-18 13:54 UTC (permalink / raw)
  To: linux-mips; +Cc: kvm, ralf, gleb, mtosatti, Sanjay Lal

The following patch set fixes a few issues with KVM/MIPS32 in Linux 3.10.

Changes from v2:
- Drop KVM-MIPS32-Fix-up-KVM-breakage-caused-by-d532f3d2671 as the offending
  commit has been reverted and will be submitted upstream via the linux-mips
  tree.
- Integrate with the new 64 bit compatible KVM/MIPS ABI 
  defined by David Daney @ Cavium.

--

Sanjay Lal (4):
  KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it
    is a user visible API.
  KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  KVM/MIPS32: Export min_low_pfn.
  KVM/MIPS32: Bring in patch from David Daney with new 64 bit
    compatible ABI.

 arch/mips/include/asm/kvm.h      |  55 -------
 arch/mips/include/asm/kvm_host.h |   9 +-
 arch/mips/include/uapi/asm/kvm.h | 113 ++++++++++++++
 arch/mips/kernel/mips_ksyms.c    |   6 +
 arch/mips/kvm/kvm_mips.c         | 102 +-----------
 arch/mips/kvm/kvm_mips_emul.c    |  22 +--
 arch/mips/kvm/kvm_tlb.c          |  55 ++++---
 arch/mips/kvm/kvm_trap_emul.c    | 330 ++++++++++++++++++++++++++++++++++-----
 8 files changed, 471 insertions(+), 221 deletions(-)
 delete mode 100644 arch/mips/include/asm/kvm.h
 create mode 100644 arch/mips/include/uapi/asm/kvm.h

-- 
1.7.11.3

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API.
  2013-05-18 13:54 [PATCH v3 0/4] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
@ 2013-05-18 13:54 ` Sanjay Lal
  2013-05-18 13:54 ` [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Sanjay Lal @ 2013-05-18 13:54 UTC (permalink / raw)
  To: linux-mips; +Cc: kvm, ralf, gleb, mtosatti, Sanjay Lal


Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm.h      | 55 ----------------------------------------
 arch/mips/include/uapi/asm/kvm.h | 55 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 55 deletions(-)
 delete mode 100644 arch/mips/include/asm/kvm.h
 create mode 100644 arch/mips/include/uapi/asm/kvm.h

diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
deleted file mode 100644
index 85789ea..0000000
--- a/arch/mips/include/asm/kvm.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
-* 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 <sanjayl@kymasys.com>
-*/
-
-#ifndef __LINUX_KVM_MIPS_H
-#define __LINUX_KVM_MIPS_H
-
-#include <linux/types.h>
-
-#define __KVM_MIPS
-
-#define N_MIPS_COPROC_REGS      32
-#define N_MIPS_COPROC_SEL   	8
-
-/* for KVM_GET_REGS and KVM_SET_REGS */
-struct kvm_regs {
-	__u32 gprs[32];
-	__u32 hi;
-	__u32 lo;
-	__u32 pc;
-
-	__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
-};
-
-/* for KVM_GET_SREGS and KVM_SET_SREGS */
-struct kvm_sregs {
-};
-
-/* for KVM_GET_FPU and KVM_SET_FPU */
-struct kvm_fpu {
-};
-
-struct kvm_debug_exit_arch {
-};
-
-/* for KVM_SET_GUEST_DEBUG */
-struct kvm_guest_debug_arch {
-};
-
-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/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
new file mode 100644
index 0000000..85789ea
--- /dev/null
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -0,0 +1,55 @@
+/*
+* 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 <sanjayl@kymasys.com>
+*/
+
+#ifndef __LINUX_KVM_MIPS_H
+#define __LINUX_KVM_MIPS_H
+
+#include <linux/types.h>
+
+#define __KVM_MIPS
+
+#define N_MIPS_COPROC_REGS      32
+#define N_MIPS_COPROC_SEL   	8
+
+/* for KVM_GET_REGS and KVM_SET_REGS */
+struct kvm_regs {
+	__u32 gprs[32];
+	__u32 hi;
+	__u32 lo;
+	__u32 pc;
+
+	__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
+};
+
+/* for KVM_GET_SREGS and KVM_SET_SREGS */
+struct kvm_sregs {
+};
+
+/* for KVM_GET_FPU and KVM_SET_FPU */
+struct kvm_fpu {
+};
+
+struct kvm_debug_exit_arch {
+};
+
+/* for KVM_SET_GUEST_DEBUG */
+struct kvm_guest_debug_arch {
+};
+
+struct kvm_mips_interrupt {
+	/* in */
+	__u32 cpu;
+	__u32 irq;
+};
+
+/* definition of registers in kvm_run */
+struct kvm_sync_regs {
+};
+
+#endif /* __LINUX_KVM_MIPS_H */
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-18 13:54 [PATCH v3 0/4] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
  2013-05-18 13:54 ` [PATCH 1/4] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
@ 2013-05-18 13:54 ` Sanjay Lal
  2013-05-19 12:52   ` Gleb Natapov
  2013-05-18 13:54 ` [PATCH 3/4] KVM/MIPS32: Export min_low_pfn Sanjay Lal
  2013-05-18 13:54 ` [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI Sanjay Lal
  3 siblings, 1 reply; 12+ messages in thread
From: Sanjay Lal @ 2013-05-18 13:54 UTC (permalink / raw)
  To: linux-mips; +Cc: kvm, ralf, gleb, mtosatti, Sanjay Lal

- As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
  Memory slots should be acccessed from a SRCU read section.
- kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
 if it cannot find a mapping for a particular gfn.

Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_tlb.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 89511a9..ab2e9b0 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -16,7 +16,10 @@
 #include <linux/mm.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/bootmem.h>
 #include <linux/kvm_host.h>
+#include <linux/srcu.h>
+
 
 #include <asm/cpu.h>
 #include <asm/bootinfo.h>
@@ -169,21 +172,27 @@ void kvm_mips_dump_shadow_tlbs(struct kvm_vcpu *vcpu)
 	}
 }
 
-static void kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
+static int kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
 {
+	int srcu_idx, err = 0;
 	pfn_t pfn;
 
 	if (kvm->arch.guest_pmap[gfn] != KVM_INVALID_PAGE)
-		return;
+		return 0;
 
+        srcu_idx = srcu_read_lock(&kvm->srcu);
 	pfn = kvm_mips_gfn_to_pfn(kvm, gfn);
 
 	if (kvm_mips_is_error_pfn(pfn)) {
-		panic("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
+		kvm_err("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
+		err = -EFAULT;
+		goto out;
 	}
 
 	kvm->arch.guest_pmap[gfn] = pfn;
-	return;
+out:
+	srcu_read_unlock(&kvm->srcu, srcu_idx);
+	return err;
 }
 
 /* Translate guest KSEG0 addresses to Host PA */
@@ -207,7 +216,10 @@ unsigned long kvm_mips_translate_guest_kseg0_to_hpa(struct kvm_vcpu *vcpu,
 			gva);
 		return KVM_INVALID_PAGE;
 	}
-	kvm_mips_map_page(vcpu->kvm, gfn);
+
+	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
+		return KVM_INVALID_ADDR;
+
 	return (kvm->arch.guest_pmap[gfn] << PAGE_SHIFT) + offset;
 }
 
@@ -310,8 +322,11 @@ int kvm_mips_handle_kseg0_tlb_fault(unsigned long badvaddr,
 	even = !(gfn & 0x1);
 	vaddr = badvaddr & (PAGE_MASK << 1);
 
-	kvm_mips_map_page(vcpu->kvm, gfn);
-	kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1);
+	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
+		return -1;
+
+	if (kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1) < 0)
+		return -1;
 
 	if (even) {
 		pfn0 = kvm->arch.guest_pmap[gfn];
@@ -389,8 +404,11 @@ kvm_mips_handle_mapped_seg_tlb_fault(struct kvm_vcpu *vcpu,
 		pfn0 = 0;
 		pfn1 = 0;
 	} else {
-		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT);
-		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT);
+		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT) < 0)
+			return -1;
+
+		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT) < 0)
+			return -1;
 
 		pfn0 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT];
 		pfn1 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT];
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] KVM/MIPS32: Export min_low_pfn.
  2013-05-18 13:54 [PATCH v3 0/4] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
  2013-05-18 13:54 ` [PATCH 1/4] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
  2013-05-18 13:54 ` [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
@ 2013-05-18 13:54 ` Sanjay Lal
  2013-05-18 13:54 ` [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI Sanjay Lal
  3 siblings, 0 replies; 12+ messages in thread
From: Sanjay Lal @ 2013-05-18 13:54 UTC (permalink / raw)
  To: linux-mips; +Cc: kvm, ralf, gleb, mtosatti, Sanjay Lal

The KVM module uses the standard MIPS cache management routines, which use min_low_pfn.
This creates and indirect dependency, requiring min_low_pfn to be exported.

Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kernel/mips_ksyms.c | 6 ++++++
 arch/mips/kvm/kvm_tlb.c       | 1 -
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/mips_ksyms.c b/arch/mips/kernel/mips_ksyms.c
index 6e58e97..0299472 100644
--- a/arch/mips/kernel/mips_ksyms.c
+++ b/arch/mips/kernel/mips_ksyms.c
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <asm/uaccess.h>
 #include <asm/ftrace.h>
+#include <linux/bootmem.h>
 
 extern void *__bzero(void *__s, size_t __count);
 extern long __strncpy_from_user_nocheck_asm(char *__to,
@@ -60,3 +61,8 @@ EXPORT_SYMBOL(invalid_pte_table);
 /* _mcount is defined in arch/mips/kernel/mcount.S */
 EXPORT_SYMBOL(_mcount);
 #endif
+
+/* The KVM module uses the standard MIPS cache functions which use
+ * min_low_pfn, requiring it to be exported.
+ */
+EXPORT_SYMBOL(min_low_pfn);
diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index ab2e9b0..87d845e 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -16,7 +16,6 @@
 #include <linux/mm.h>
 #include <linux/delay.h>
 #include <linux/module.h>
-#include <linux/bootmem.h>
 #include <linux/kvm_host.h>
 #include <linux/srcu.h>
 
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI.
  2013-05-18 13:54 [PATCH v3 0/4] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
                   ` (2 preceding siblings ...)
  2013-05-18 13:54 ` [PATCH 3/4] KVM/MIPS32: Export min_low_pfn Sanjay Lal
@ 2013-05-18 13:54 ` Sanjay Lal
  2013-05-19 14:17   ` Gleb Natapov
  3 siblings, 1 reply; 12+ messages in thread
From: Sanjay Lal @ 2013-05-18 13:54 UTC (permalink / raw)
  To: linux-mips; +Cc: kvm, ralf, gleb, mtosatti, Sanjay Lal, David Daney

From: David Daney <david.daney@cavium.com>

There are several parts to this:

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

o FPU register formats are defined.

o CP0 Registers are manipulated via the KVM_GET_MSRS/KVM_SET_MSRS
  mechanism.

The vcpu_ioctl_get_regs and vcpu_ioctl_set_regs function pointers
become unused so they were removed.

Some IOCTL functions were moved to kvm_trap_emul because the
implementations are only for that flavor of KVM host.  In the future, if
hardware based virtualization is added, they can be hidden behind
function pointers as appropriate.

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |   4 -
 arch/mips/include/uapi/asm/kvm.h | 106 ++++++++++---
 arch/mips/kvm/kvm_mips.c         | 102 +-----------
 arch/mips/kvm/kvm_trap_emul.c    | 330 ++++++++++++++++++++++++++++++++++-----
 4 files changed, 382 insertions(+), 160 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index e68781e..e3d49ec 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/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index 85789ea..83c44d8 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -1,55 +1,113 @@
 /*
-* 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 <sanjayl@kymasys.com>
-*/
+ * 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 <sanjayl@kymasys.com>
+ */
 
 #ifndef __LINUX_KVM_MIPS_H
 #define __LINUX_KVM_MIPS_H
 
 #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 */
+/*
+ * 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;
+	/* out (KVM_GET_REGS) / in (KVM_SET_REGS) */
+	__u64 gpr[32];
+	__u64 hi, lo;
+	__u64 pc;
+};
 
-	__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
+/* 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;
 };
 
-/* for KVM_GET_SREGS and KVM_SET_SREGS */
-struct kvm_sregs {
+
+/*
+ * For MIPS, we use the same APIs as x86, where 'msr' corresponds to a
+ * CP0 register.  The index 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[31..16] - 0 -> CP0 registers.
+ *
+ * Other sets registers may be added in the future.  Each set would
+ * have its own identifier in bits[31..16].
+ *
+ * For MSRs that are narrower than 64-bits, the value is stored in the
+ * low order bits of the data field, and sign extended to 64-bits.
+ */
+#define KVM_MIPS_MSR_CP0 0
+struct kvm_msr_entry {
+	__u32 index;
+	__u32 reserved;
+	__u64 data;
 };
 
-/* for KVM_GET_FPU and KVM_SET_FPU */
-struct kvm_fpu {
+/* for KVM_GET_MSRS and KVM_SET_MSRS */
+struct kvm_msrs {
+	__u32 nmsrs; /* number of msrs in entries */
+	__u32 pad;
+
+	struct kvm_msr_entry entries[0];
 };
 
+/* for KVM_GET_MSR_INDEX_LIST */
+struct kvm_msr_list {
+	__u32 nmsrs; /* number of msrs in entries */
+	__u32 indices[0];
+};
+
+/*
+ * 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/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index e0dad02..26a40e3 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 unalias_gfn(struct kvm *kvm, gfn_t gfn)
 {
 	return gfn;
@@ -192,12 +182,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	}
 }
 
-long
-kvm_arch_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
-{
-	return -EINVAL;
-}
-
 void kvm_arch_free_memslot(struct kvm_memory_slot *free,
 			   struct kvm_memory_slot *dont)
 {
@@ -435,42 +419,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 	return r;
 }
-
-int
-kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt *irq)
-{
-	int intr = (int)irq->irq;
-	struct kvm_vcpu *dvcpu = NULL;
-
-	if (intr == 3 || intr == -3 || intr == 4 || intr == -4)
-		kvm_debug("%s: CPU: %d, INTR: %d\n", __func__, irq->cpu,
-			  (int)intr);
-
-	if (irq->cpu == -1)
-		dvcpu = vcpu;
-	else
-		dvcpu = vcpu->kvm->vcpus[irq->cpu];
-
-	if (intr == 2 || intr == 3 || intr == 4) {
-		kvm_mips_callbacks->queue_io_int(dvcpu, irq);
-
-	} else if (intr == -2 || intr == -3 || intr == -4) {
-		kvm_mips_callbacks->dequeue_io_int(dvcpu, irq);
-	} else {
-		kvm_err("%s: invalid interrupt ioctl (%d:%d)\n", __func__,
-			irq->cpu, irq->irq);
-		return -EINVAL;
-	}
-
-	dvcpu->arch.wait = 0;
-
-	if (waitqueue_active(&dvcpu->wq)) {
-		wake_up_interruptible(&dvcpu->wq);
-	}
-
-	return 0;
-}
-
 int
 kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
 				struct kvm_mp_state *mp_state)
@@ -485,42 +433,6 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
-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_NMI:
-		/* Treat the NMI as a CPU reset */
-		r = kvm_mips_reset_vcpu(vcpu);
-		break;
-	case KVM_INTERRUPT:
-		{
-			struct kvm_mips_interrupt irq;
-			r = -EFAULT;
-			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);
-
-			r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
-			break;
-		}
-	default:
-		r = -EINVAL;
-	}
-
-out:
-	return r;
-}
-
 /*
  * Get (and clear) the dirty memory log for a memory slot.
  */
@@ -677,28 +589,28 @@ 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];
-
+	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;
 
-	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)
 {
 	int i;
 
-	for (i = 0; i < 32; i++)
-		regs->gprs[i] = vcpu->arch.gprs[i];
+	for (i = 0; i < ARRAY_SIZE(vcpu->arch.gprs); i++)
+		regs->gpr[i] = vcpu->arch.gprs[i];
 
 	regs->hi = vcpu->arch.hi;
 	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..df98dcb 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -13,7 +13,7 @@
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/vmalloc.h>
-
+#include <linux/fs.h>
 #include <linux/kvm_host.h>
 
 #include "kvm_mips_opcode.h"
@@ -345,54 +345,312 @@ 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)
+#define MSR_MIPS_CP0_INDEX (8 * 0 + 0)
+#define MSR_MIPS_CP0_ENTRYLO0 (8 * 2 + 0)
+#define MSR_MIPS_CP0_ENTRYLO1 (8 * 3 + 0)
+#define MSR_MIPS_CP0_CONTEXT (8 * 4 + 0)
+#define MSR_MIPS_CP0_USERLOCAL (8 * 4 + 2)
+#define MSR_MIPS_CP0_PAGEMASK (8 * 5 + 0)
+#define MSR_MIPS_CP0_PAGEGRAIN (8 * 5 + 1)
+#define MSR_MIPS_CP0_WIRED (8 * 6 + 0)
+#define MSR_MIPS_CP0_HWRENA (8 * 7 + 0)
+#define MSR_MIPS_CP0_BADVADDR (8 * 8 + 0)
+#define MSR_MIPS_CP0_COUNT (8 * 9 + 0)
+#define MSR_MIPS_CP0_ENTRYHI (8 * 10 + 0)
+#define MSR_MIPS_CP0_COMPARE (8 * 11 + 0)
+#define MSR_MIPS_CP0_STATUS (8 * 12 + 0)
+#define MSR_MIPS_CP0_CAUSE (8 * 13 + 0)
+#define MSR_MIPS_CP0_EBASE (8 * 15 + 1)
+#define MSR_MIPS_CP0_CONFIG (8 * 16 + 0)
+#define MSR_MIPS_CP0_CONFIG1 (8 * 16 + 1)
+#define MSR_MIPS_CP0_CONFIG2 (8 * 16 + 2)
+#define MSR_MIPS_CP0_CONFIG3 (8 * 16 + 3)
+#define MSR_MIPS_CP0_CONFIG7 (8 * 16 + 7)
+#define MSR_MIPS_CP0_XCONTEXT (8 * 20 + 0)
+#define MSR_MIPS_CP0_ERROREPC (8 * 30 + 0)
+
+static u32 msrs_to_save[] = {
+	MSR_MIPS_CP0_INDEX,
+	MSR_MIPS_CP0_CONTEXT,
+	MSR_MIPS_CP0_PAGEMASK,
+	MSR_MIPS_CP0_WIRED,
+	MSR_MIPS_CP0_BADVADDR,
+	MSR_MIPS_CP0_ENTRYHI,
+	MSR_MIPS_CP0_STATUS,
+	MSR_MIPS_CP0_CAUSE,
+	/* EPC set via kvm_regs, et al. */
+	MSR_MIPS_CP0_CONFIG,
+	MSR_MIPS_CP0_CONFIG1,
+	MSR_MIPS_CP0_CONFIG2,
+	MSR_MIPS_CP0_CONFIG3,
+	MSR_MIPS_CP0_CONFIG7,
+	MSR_MIPS_CP0_XCONTEXT,
+	MSR_MIPS_CP0_ERROREPC
+};
+
+#define MAX_IO_MSRS 128
+
+static int mipsvz_get_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	s64 v;
 
-	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]);
+	switch (index) {
+	case MSR_MIPS_CP0_INDEX:
+		v = (long)kvm_read_c0_guest_index(cop0);
+		break;
+	case MSR_MIPS_CP0_CONTEXT:
+		v = (long)kvm_read_c0_guest_context(cop0);
+		break;
+	case MSR_MIPS_CP0_PAGEMASK:
+		v = (long)kvm_read_c0_guest_pagemask(cop0);
+		break;
+	case MSR_MIPS_CP0_WIRED:
+		v = (long)kvm_read_c0_guest_wired(cop0);
+		break;
+	case MSR_MIPS_CP0_BADVADDR:
+		v = (long)kvm_read_c0_guest_badvaddr(cop0);
+		break;
+	case MSR_MIPS_CP0_ENTRYHI:
+		v = (long)kvm_read_c0_guest_entryhi(cop0);
+		break;
+	case MSR_MIPS_CP0_STATUS:
+		v = (long)kvm_read_c0_guest_status(cop0);
+		break;
+	case MSR_MIPS_CP0_CAUSE:
+		v = (long)kvm_read_c0_guest_cause(cop0);
+		break;
+	case MSR_MIPS_CP0_ERROREPC:
+		v = (long)kvm_read_c0_guest_errorepc(cop0);
+		break;
+	case MSR_MIPS_CP0_CONFIG:
+		v = (long)kvm_read_c0_guest_config(cop0);
+		break;
+	case MSR_MIPS_CP0_CONFIG1:
+		v = (long)kvm_read_c0_guest_config1(cop0);
+		break;
+	case MSR_MIPS_CP0_CONFIG2:
+		v = (long)kvm_read_c0_guest_config2(cop0);
+		break;
+	case MSR_MIPS_CP0_CONFIG3:
+		v = (long)kvm_read_c0_guest_config3(cop0);
+		break;
+	case MSR_MIPS_CP0_CONFIG7:
+		v = (long)kvm_read_c0_guest_config7(cop0);
+		break;
+	default:
+		return -EINVAL;
+	}
+	*data = v;
+	return 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]);
+static int mipsvz_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	u64 v = *data;
 
+	switch (index) {
+	case MSR_MIPS_CP0_INDEX:
+		kvm_write_c0_guest_index(cop0, v);
+		break;
+	case MSR_MIPS_CP0_CONTEXT:
+		kvm_write_c0_guest_context(cop0, v);
+		break;
+	case MSR_MIPS_CP0_PAGEMASK:
+		kvm_write_c0_guest_pagemask(cop0, v);
+		break;
+	case MSR_MIPS_CP0_WIRED:
+		kvm_write_c0_guest_wired(cop0, v);
+		break;
+	case MSR_MIPS_CP0_BADVADDR:
+		kvm_write_c0_guest_badvaddr(cop0, v);
+		break;
+	case MSR_MIPS_CP0_ENTRYHI:
+		kvm_write_c0_guest_entryhi(cop0, v);
+		break;
+	case MSR_MIPS_CP0_STATUS:
+		kvm_write_c0_guest_status(cop0, v);
+		break;
+	case MSR_MIPS_CP0_CAUSE:
+		kvm_write_c0_guest_cause(cop0, v);
+		break;
+	case MSR_MIPS_CP0_ERROREPC:
+		kvm_write_c0_guest_errorepc(cop0, v);
+		break;
+	default:
+		return -EINVAL;
+	}
 	return 0;
 }
 
-static int
-kvm_trap_emul_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
+/*
+ * Read or write a bunch of msrs. Parameters are user addresses.
+ *
+ * @return number of msrs set successfully.
+ */
+static int msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs __user *user_msrs,
+		  int (*do_msr)(struct kvm_vcpu *vcpu,
+				unsigned index, u64 *data),
+		  bool writeback)
 {
-	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	struct kvm_msrs msrs;
+	struct kvm_msr_entry *entries;
+	int r, n;
+	unsigned size;
+
+	r = -EFAULT;
+	if (copy_from_user(&msrs, user_msrs, sizeof(msrs)))
+		goto out;
+
+	r = -E2BIG;
+	if (msrs.nmsrs >= MAX_IO_MSRS)
+		goto out;
+
+	size = sizeof(struct kvm_msr_entry) * msrs.nmsrs;
+	entries = memdup_user(user_msrs->entries, size);
+	if (IS_ERR(entries)) {
+		r = PTR_ERR(entries);
+		goto out;
+	}
 
-	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);
+	for (n = 0; n < msrs.nmsrs; ++n)
+		if (do_msr(vcpu, entries[n].index, &entries[n].data))
+			break;
+
+	r = -EFAULT;
+	if (writeback && copy_to_user(user_msrs->entries, entries, size))
+		goto out_free;
+
+	r = n;
+
+out_free:
+	kfree(entries);
+out:
+	return r;
+}
+
+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;
+}
+
+int
+kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt *irq)
+{
+	int intr = (int)irq->irq;
+	struct kvm_vcpu *dvcpu = NULL;
+
+	if (intr == 3 || intr == -3 || intr == 4 || intr == -4)
+		kvm_debug("%s: CPU: %d, INTR: %d\n", __func__, irq->cpu,
+			  (int)intr);
+
+	if (irq->cpu == -1)
+		dvcpu = vcpu;
+	else
+		dvcpu = vcpu->kvm->vcpus[irq->cpu];
+
+	if (intr == 2 || intr == 3 || intr == 4) {
+		kvm_mips_callbacks->queue_io_int(dvcpu, irq);
+
+	} else if (intr == -2 || intr == -3 || intr == -4) {
+		kvm_mips_callbacks->dequeue_io_int(dvcpu, irq);
+	} else {
+		kvm_err("%s: invalid interrupt ioctl (%d:%d)\n", __func__,
+			irq->cpu, irq->irq);
+		return -EINVAL;
+	}
+
+	dvcpu->arch.wait = 0;
+
+	if (waitqueue_active(&dvcpu->wq))
+		wake_up_interruptible(&dvcpu->wq);
 
 	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;
+
+	switch (ioctl) {
+	case KVM_GET_MSRS:
+		r = msr_io(vcpu, argp, mipsvz_get_msr, true);
+		break;
+	case KVM_SET_MSRS:
+		r = msr_io(vcpu, argp, mipsvz_set_msr, false);
+		break;
+	case KVM_NMI:
+		/* Treat the NMI as a CPU reset */
+		r = kvm_mips_reset_vcpu(vcpu);
+		break;
+	case KVM_INTERRUPT:
+		{
+			struct kvm_mips_interrupt irq;
+			r = -EFAULT;
+			if (copy_from_user(&irq, argp, sizeof(irq)))
+				goto out;
+
+			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
+				  irq.irq);
+
+			r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
+			break;
+		}
+	default:
+		r = -ENOIOCTLCMD;
+	}
+
+out:
+	return r;
+}
+
+long kvm_arch_dev_ioctl(struct file *filp,
+			unsigned int ioctl, unsigned long arg)
+{
+	long r;
+	void __user *argp = (void __user *)arg;
+
+	switch (ioctl) {
+	case KVM_GET_MSR_INDEX_LIST: {
+		struct kvm_msr_list __user *user_msr_list = argp;
+		struct kvm_msr_list msr_list;
+		unsigned n;
+		unsigned num_msrs_to_save;
+
+		r = -EFAULT;
+		if (copy_from_user(&msr_list, user_msr_list, sizeof(msr_list)))
+			goto out;
+		n = msr_list.nmsrs;
+		msr_list.nmsrs = ARRAY_SIZE(msrs_to_save);
+		num_msrs_to_save = min(n, msr_list.nmsrs);
+		if (copy_to_user(user_msr_list, &msr_list, sizeof(msr_list)))
+			goto out;
+		r = -E2BIG;
+		if (n < msr_list.nmsrs)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
+				 num_msrs_to_save * sizeof(u32)))
+			goto out;
+		r = 0;
+		break;
+	}
+	default:
+		r = -ENOIOCTLCMD;
+	}
+out:
+	return r;
+}
+
 static int kvm_trap_emul_vm_init(struct kvm *kvm)
 {
 	return 0;
@@ -471,8 +729,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.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-18 13:54 ` [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
@ 2013-05-19 12:52   ` Gleb Natapov
  2013-05-19 14:36     ` Sanjay Lal
  0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2013-05-19 12:52 UTC (permalink / raw)
  To: Sanjay Lal; +Cc: linux-mips, kvm, ralf, mtosatti

On Sat, May 18, 2013 at 06:54:24AM -0700, Sanjay Lal wrote:
> - As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
>   Memory slots should be acccessed from a SRCU read section.
> - kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
>  if it cannot find a mapping for a particular gfn.
> 
> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
> ---
>  arch/mips/kvm/kvm_tlb.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
> index 89511a9..ab2e9b0 100644
> --- a/arch/mips/kvm/kvm_tlb.c
> +++ b/arch/mips/kvm/kvm_tlb.c
> @@ -16,7 +16,10 @@
>  #include <linux/mm.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
> +#include <linux/bootmem.h>
You haven't answered it when I asked it on v2:
Is this include still needed now when export of min_low_pfn is not
longer here?

>  #include <linux/kvm_host.h>
> +#include <linux/srcu.h>
> +
>  
>  #include <asm/cpu.h>
>  #include <asm/bootinfo.h>
> @@ -169,21 +172,27 @@ void kvm_mips_dump_shadow_tlbs(struct kvm_vcpu *vcpu)
>  	}
>  }
>  
> -static void kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
> +static int kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
>  {
> +	int srcu_idx, err = 0;
>  	pfn_t pfn;
>  
>  	if (kvm->arch.guest_pmap[gfn] != KVM_INVALID_PAGE)
> -		return;
> +		return 0;
>  
> +        srcu_idx = srcu_read_lock(&kvm->srcu);
>  	pfn = kvm_mips_gfn_to_pfn(kvm, gfn);
>  
>  	if (kvm_mips_is_error_pfn(pfn)) {
> -		panic("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
> +		kvm_err("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
> +		err = -EFAULT;
> +		goto out;
>  	}
>  
>  	kvm->arch.guest_pmap[gfn] = pfn;
> -	return;
> +out:
> +	srcu_read_unlock(&kvm->srcu, srcu_idx);
> +	return err;
>  }
>  
>  /* Translate guest KSEG0 addresses to Host PA */
> @@ -207,7 +216,10 @@ unsigned long kvm_mips_translate_guest_kseg0_to_hpa(struct kvm_vcpu *vcpu,
>  			gva);
>  		return KVM_INVALID_PAGE;
>  	}
> -	kvm_mips_map_page(vcpu->kvm, gfn);
> +
> +	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
> +		return KVM_INVALID_ADDR;
> +
>  	return (kvm->arch.guest_pmap[gfn] << PAGE_SHIFT) + offset;
>  }
>  
> @@ -310,8 +322,11 @@ int kvm_mips_handle_kseg0_tlb_fault(unsigned long badvaddr,
>  	even = !(gfn & 0x1);
>  	vaddr = badvaddr & (PAGE_MASK << 1);
>  
> -	kvm_mips_map_page(vcpu->kvm, gfn);
> -	kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1);
> +	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
> +		return -1;
> +
> +	if (kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1) < 0)
> +		return -1;
>  
>  	if (even) {
>  		pfn0 = kvm->arch.guest_pmap[gfn];
> @@ -389,8 +404,11 @@ kvm_mips_handle_mapped_seg_tlb_fault(struct kvm_vcpu *vcpu,
>  		pfn0 = 0;
>  		pfn1 = 0;
>  	} else {
> -		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT);
> -		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT);
> +		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT) < 0)
> +			return -1;
> +
> +		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT) < 0)
> +			return -1;
>  
>  		pfn0 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT];
>  		pfn1 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT];
> -- 
> 1.7.11.3

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI.
  2013-05-18 13:54 ` [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI Sanjay Lal
@ 2013-05-19 14:17   ` Gleb Natapov
  2013-05-19 21:17     ` David Daney
  0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2013-05-19 14:17 UTC (permalink / raw)
  To: Sanjay Lal; +Cc: linux-mips, kvm, ralf, mtosatti, David Daney

On Sat, May 18, 2013 at 06:54:26AM -0700, Sanjay Lal wrote:
> From: David Daney <david.daney@cavium.com>
> 
> There are several parts to this:
> 
> o All registers are 64-bits wide, 32-bit guests use the least
>   significant portion of the register storage fields.
> 
> o FPU register formats are defined.
> 
> o CP0 Registers are manipulated via the KVM_GET_MSRS/KVM_SET_MSRS
>   mechanism.
> 
> The vcpu_ioctl_get_regs and vcpu_ioctl_set_regs function pointers
> become unused so they were removed.
> 
> Some IOCTL functions were moved to kvm_trap_emul because the
> implementations are only for that flavor of KVM host.  In the future, if
> hardware based virtualization is added, they can be hidden behind
> function pointers as appropriate.
> 
David, can you please divide this one big patch to smaller patches
with each one having only one of the changes listed above?

> Signed-off-by: David Daney <david.daney@cavium.com>
> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
> ---
>  arch/mips/include/asm/kvm_host.h |   4 -
>  arch/mips/include/uapi/asm/kvm.h | 106 ++++++++++---
>  arch/mips/kvm/kvm_mips.c         | 102 +-----------
>  arch/mips/kvm/kvm_trap_emul.c    | 330 ++++++++++++++++++++++++++++++++++-----
>  4 files changed, 382 insertions(+), 160 deletions(-)
> 
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index e68781e..e3d49ec 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/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
> index 85789ea..83c44d8 100644
> --- a/arch/mips/include/uapi/asm/kvm.h
> +++ b/arch/mips/include/uapi/asm/kvm.h
> @@ -1,55 +1,113 @@
>  /*
> -* 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 <sanjayl@kymasys.com>
> -*/
> + * 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 <sanjayl@kymasys.com>
> + */
>  
>  #ifndef __LINUX_KVM_MIPS_H
>  #define __LINUX_KVM_MIPS_H
>  
>  #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 */
> +/*
> + * 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;
> +	/* out (KVM_GET_REGS) / in (KVM_SET_REGS) */
> +	__u64 gpr[32];
> +	__u64 hi, lo;
> +	__u64 pc;
> +};
>  
> -	__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
> +/* 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;
>  };
>  
> -/* for KVM_GET_SREGS and KVM_SET_SREGS */
> -struct kvm_sregs {
> +
> +/*
> + * For MIPS, we use the same APIs as x86, where 'msr' corresponds to a
> + * CP0 register.  The index 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[31..16] - 0 -> CP0 registers.
> + *
> + * Other sets registers may be added in the future.  Each set would
> + * have its own identifier in bits[31..16].
> + *
> + * For MSRs that are narrower than 64-bits, the value is stored in the
> + * low order bits of the data field, and sign extended to 64-bits.
> + */
> +#define KVM_MIPS_MSR_CP0 0
> +struct kvm_msr_entry {
> +	__u32 index;
> +	__u32 reserved;
> +	__u64 data;
>  };
>  
> -/* for KVM_GET_FPU and KVM_SET_FPU */
> -struct kvm_fpu {
> +/* for KVM_GET_MSRS and KVM_SET_MSRS */
> +struct kvm_msrs {
> +	__u32 nmsrs; /* number of msrs in entries */
> +	__u32 pad;
> +
> +	struct kvm_msr_entry entries[0];
>  };
>  
> +/* for KVM_GET_MSR_INDEX_LIST */
> +struct kvm_msr_list {
> +	__u32 nmsrs; /* number of msrs in entries */
> +	__u32 indices[0];
> +};
> +
> +/*
> + * 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/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
> index e0dad02..26a40e3 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 unalias_gfn(struct kvm *kvm, gfn_t gfn)
>  {
>  	return gfn;
> @@ -192,12 +182,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
>  	}
>  }
>  
> -long
> -kvm_arch_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> -{
> -	return -EINVAL;
> -}
> -
>  void kvm_arch_free_memslot(struct kvm_memory_slot *free,
>  			   struct kvm_memory_slot *dont)
>  {
> @@ -435,42 +419,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  	return r;
>  }
> -
> -int
> -kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt *irq)
> -{
> -	int intr = (int)irq->irq;
> -	struct kvm_vcpu *dvcpu = NULL;
> -
> -	if (intr == 3 || intr == -3 || intr == 4 || intr == -4)
> -		kvm_debug("%s: CPU: %d, INTR: %d\n", __func__, irq->cpu,
> -			  (int)intr);
> -
> -	if (irq->cpu == -1)
> -		dvcpu = vcpu;
> -	else
> -		dvcpu = vcpu->kvm->vcpus[irq->cpu];
> -
> -	if (intr == 2 || intr == 3 || intr == 4) {
> -		kvm_mips_callbacks->queue_io_int(dvcpu, irq);
> -
> -	} else if (intr == -2 || intr == -3 || intr == -4) {
> -		kvm_mips_callbacks->dequeue_io_int(dvcpu, irq);
> -	} else {
> -		kvm_err("%s: invalid interrupt ioctl (%d:%d)\n", __func__,
> -			irq->cpu, irq->irq);
> -		return -EINVAL;
> -	}
> -
> -	dvcpu->arch.wait = 0;
> -
> -	if (waitqueue_active(&dvcpu->wq)) {
> -		wake_up_interruptible(&dvcpu->wq);
> -	}
> -
> -	return 0;
> -}
> -
>  int
>  kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
>  				struct kvm_mp_state *mp_state)
> @@ -485,42 +433,6 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> -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_NMI:
> -		/* Treat the NMI as a CPU reset */
> -		r = kvm_mips_reset_vcpu(vcpu);
> -		break;
> -	case KVM_INTERRUPT:
> -		{
> -			struct kvm_mips_interrupt irq;
> -			r = -EFAULT;
> -			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);
> -
> -			r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
> -			break;
> -		}
> -	default:
> -		r = -EINVAL;
> -	}
> -
> -out:
> -	return r;
> -}
> -
>  /*
>   * Get (and clear) the dirty memory log for a memory slot.
>   */
> @@ -677,28 +589,28 @@ 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];
> -
> +	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;
>  
> -	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)
>  {
>  	int i;
>  
> -	for (i = 0; i < 32; i++)
> -		regs->gprs[i] = vcpu->arch.gprs[i];
> +	for (i = 0; i < ARRAY_SIZE(vcpu->arch.gprs); i++)
> +		regs->gpr[i] = vcpu->arch.gprs[i];
>  
>  	regs->hi = vcpu->arch.hi;
>  	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..df98dcb 100644
> --- a/arch/mips/kvm/kvm_trap_emul.c
> +++ b/arch/mips/kvm/kvm_trap_emul.c
> @@ -13,7 +13,7 @@
>  #include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/vmalloc.h>
> -
> +#include <linux/fs.h>
>  #include <linux/kvm_host.h>
>  
>  #include "kvm_mips_opcode.h"
> @@ -345,54 +345,312 @@ 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)
> +#define MSR_MIPS_CP0_INDEX (8 * 0 + 0)
> +#define MSR_MIPS_CP0_ENTRYLO0 (8 * 2 + 0)
> +#define MSR_MIPS_CP0_ENTRYLO1 (8 * 3 + 0)
> +#define MSR_MIPS_CP0_CONTEXT (8 * 4 + 0)
> +#define MSR_MIPS_CP0_USERLOCAL (8 * 4 + 2)
> +#define MSR_MIPS_CP0_PAGEMASK (8 * 5 + 0)
> +#define MSR_MIPS_CP0_PAGEGRAIN (8 * 5 + 1)
> +#define MSR_MIPS_CP0_WIRED (8 * 6 + 0)
> +#define MSR_MIPS_CP0_HWRENA (8 * 7 + 0)
> +#define MSR_MIPS_CP0_BADVADDR (8 * 8 + 0)
> +#define MSR_MIPS_CP0_COUNT (8 * 9 + 0)
> +#define MSR_MIPS_CP0_ENTRYHI (8 * 10 + 0)
> +#define MSR_MIPS_CP0_COMPARE (8 * 11 + 0)
> +#define MSR_MIPS_CP0_STATUS (8 * 12 + 0)
> +#define MSR_MIPS_CP0_CAUSE (8 * 13 + 0)
> +#define MSR_MIPS_CP0_EBASE (8 * 15 + 1)
> +#define MSR_MIPS_CP0_CONFIG (8 * 16 + 0)
> +#define MSR_MIPS_CP0_CONFIG1 (8 * 16 + 1)
> +#define MSR_MIPS_CP0_CONFIG2 (8 * 16 + 2)
> +#define MSR_MIPS_CP0_CONFIG3 (8 * 16 + 3)
> +#define MSR_MIPS_CP0_CONFIG7 (8 * 16 + 7)
> +#define MSR_MIPS_CP0_XCONTEXT (8 * 20 + 0)
> +#define MSR_MIPS_CP0_ERROREPC (8 * 30 + 0)
> +
> +static u32 msrs_to_save[] = {
> +	MSR_MIPS_CP0_INDEX,
> +	MSR_MIPS_CP0_CONTEXT,
> +	MSR_MIPS_CP0_PAGEMASK,
> +	MSR_MIPS_CP0_WIRED,
> +	MSR_MIPS_CP0_BADVADDR,
> +	MSR_MIPS_CP0_ENTRYHI,
> +	MSR_MIPS_CP0_STATUS,
> +	MSR_MIPS_CP0_CAUSE,
> +	/* EPC set via kvm_regs, et al. */
> +	MSR_MIPS_CP0_CONFIG,
> +	MSR_MIPS_CP0_CONFIG1,
> +	MSR_MIPS_CP0_CONFIG2,
> +	MSR_MIPS_CP0_CONFIG3,
> +	MSR_MIPS_CP0_CONFIG7,
> +	MSR_MIPS_CP0_XCONTEXT,
> +	MSR_MIPS_CP0_ERROREPC
> +};
> +
> +#define MAX_IO_MSRS 128
> +
> +static int mipsvz_get_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
>  {
>  	struct mips_coproc *cop0 = vcpu->arch.cop0;
> +	s64 v;
>  
> -	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]);
> +	switch (index) {
> +	case MSR_MIPS_CP0_INDEX:
> +		v = (long)kvm_read_c0_guest_index(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CONTEXT:
> +		v = (long)kvm_read_c0_guest_context(cop0);
> +		break;
> +	case MSR_MIPS_CP0_PAGEMASK:
> +		v = (long)kvm_read_c0_guest_pagemask(cop0);
> +		break;
> +	case MSR_MIPS_CP0_WIRED:
> +		v = (long)kvm_read_c0_guest_wired(cop0);
> +		break;
> +	case MSR_MIPS_CP0_BADVADDR:
> +		v = (long)kvm_read_c0_guest_badvaddr(cop0);
> +		break;
> +	case MSR_MIPS_CP0_ENTRYHI:
> +		v = (long)kvm_read_c0_guest_entryhi(cop0);
> +		break;
> +	case MSR_MIPS_CP0_STATUS:
> +		v = (long)kvm_read_c0_guest_status(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CAUSE:
> +		v = (long)kvm_read_c0_guest_cause(cop0);
> +		break;
> +	case MSR_MIPS_CP0_ERROREPC:
> +		v = (long)kvm_read_c0_guest_errorepc(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CONFIG:
> +		v = (long)kvm_read_c0_guest_config(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CONFIG1:
> +		v = (long)kvm_read_c0_guest_config1(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CONFIG2:
> +		v = (long)kvm_read_c0_guest_config2(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CONFIG3:
> +		v = (long)kvm_read_c0_guest_config3(cop0);
> +		break;
> +	case MSR_MIPS_CP0_CONFIG7:
> +		v = (long)kvm_read_c0_guest_config7(cop0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	*data = v;
> +	return 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]);
> +static int mipsvz_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
> +{
> +	struct mips_coproc *cop0 = vcpu->arch.cop0;
> +	u64 v = *data;
>  
> +	switch (index) {
> +	case MSR_MIPS_CP0_INDEX:
> +		kvm_write_c0_guest_index(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_CONTEXT:
> +		kvm_write_c0_guest_context(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_PAGEMASK:
> +		kvm_write_c0_guest_pagemask(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_WIRED:
> +		kvm_write_c0_guest_wired(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_BADVADDR:
> +		kvm_write_c0_guest_badvaddr(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_ENTRYHI:
> +		kvm_write_c0_guest_entryhi(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_STATUS:
> +		kvm_write_c0_guest_status(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_CAUSE:
> +		kvm_write_c0_guest_cause(cop0, v);
> +		break;
> +	case MSR_MIPS_CP0_ERROREPC:
> +		kvm_write_c0_guest_errorepc(cop0, v);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>  	return 0;
>  }
>  
> -static int
> -kvm_trap_emul_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> +/*
> + * Read or write a bunch of msrs. Parameters are user addresses.
> + *
> + * @return number of msrs set successfully.
> + */
> +static int msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs __user *user_msrs,
> +		  int (*do_msr)(struct kvm_vcpu *vcpu,
> +				unsigned index, u64 *data),
> +		  bool writeback)
>  {
> -	struct mips_coproc *cop0 = vcpu->arch.cop0;
> +	struct kvm_msrs msrs;
> +	struct kvm_msr_entry *entries;
> +	int r, n;
> +	unsigned size;
> +
> +	r = -EFAULT;
> +	if (copy_from_user(&msrs, user_msrs, sizeof(msrs)))
> +		goto out;
> +
> +	r = -E2BIG;
> +	if (msrs.nmsrs >= MAX_IO_MSRS)
> +		goto out;
> +
> +	size = sizeof(struct kvm_msr_entry) * msrs.nmsrs;
> +	entries = memdup_user(user_msrs->entries, size);
> +	if (IS_ERR(entries)) {
> +		r = PTR_ERR(entries);
> +		goto out;
> +	}
>  
> -	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);
> +	for (n = 0; n < msrs.nmsrs; ++n)
> +		if (do_msr(vcpu, entries[n].index, &entries[n].data))
> +			break;
> +
> +	r = -EFAULT;
> +	if (writeback && copy_to_user(user_msrs->entries, entries, size))
> +		goto out_free;
> +
> +	r = n;
> +
> +out_free:
> +	kfree(entries);
> +out:
> +	return r;
> +}
> +
> +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;
> +}
> +
> +int
> +kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt *irq)
> +{
> +	int intr = (int)irq->irq;
> +	struct kvm_vcpu *dvcpu = NULL;
> +
> +	if (intr == 3 || intr == -3 || intr == 4 || intr == -4)
> +		kvm_debug("%s: CPU: %d, INTR: %d\n", __func__, irq->cpu,
> +			  (int)intr);
> +
> +	if (irq->cpu == -1)
> +		dvcpu = vcpu;
> +	else
> +		dvcpu = vcpu->kvm->vcpus[irq->cpu];
> +
> +	if (intr == 2 || intr == 3 || intr == 4) {
> +		kvm_mips_callbacks->queue_io_int(dvcpu, irq);
> +
> +	} else if (intr == -2 || intr == -3 || intr == -4) {
> +		kvm_mips_callbacks->dequeue_io_int(dvcpu, irq);
> +	} else {
> +		kvm_err("%s: invalid interrupt ioctl (%d:%d)\n", __func__,
> +			irq->cpu, irq->irq);
> +		return -EINVAL;
> +	}
> +
> +	dvcpu->arch.wait = 0;
> +
> +	if (waitqueue_active(&dvcpu->wq))
> +		wake_up_interruptible(&dvcpu->wq);
>  
>  	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;
> +
> +	switch (ioctl) {
> +	case KVM_GET_MSRS:
> +		r = msr_io(vcpu, argp, mipsvz_get_msr, true);
> +		break;
> +	case KVM_SET_MSRS:
> +		r = msr_io(vcpu, argp, mipsvz_set_msr, false);
> +		break;
> +	case KVM_NMI:
> +		/* Treat the NMI as a CPU reset */
> +		r = kvm_mips_reset_vcpu(vcpu);
> +		break;
> +	case KVM_INTERRUPT:
> +		{
> +			struct kvm_mips_interrupt irq;
> +			r = -EFAULT;
> +			if (copy_from_user(&irq, argp, sizeof(irq)))
> +				goto out;
> +
> +			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
> +				  irq.irq);
> +
> +			r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
> +			break;
> +		}
> +	default:
> +		r = -ENOIOCTLCMD;
> +	}
> +
> +out:
> +	return r;
> +}
> +
> +long kvm_arch_dev_ioctl(struct file *filp,
> +			unsigned int ioctl, unsigned long arg)
> +{
> +	long r;
> +	void __user *argp = (void __user *)arg;
> +
> +	switch (ioctl) {
> +	case KVM_GET_MSR_INDEX_LIST: {
> +		struct kvm_msr_list __user *user_msr_list = argp;
> +		struct kvm_msr_list msr_list;
> +		unsigned n;
> +		unsigned num_msrs_to_save;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&msr_list, user_msr_list, sizeof(msr_list)))
> +			goto out;
> +		n = msr_list.nmsrs;
> +		msr_list.nmsrs = ARRAY_SIZE(msrs_to_save);
> +		num_msrs_to_save = min(n, msr_list.nmsrs);
> +		if (copy_to_user(user_msr_list, &msr_list, sizeof(msr_list)))
> +			goto out;
> +		r = -E2BIG;
> +		if (n < msr_list.nmsrs)
> +			goto out;
> +		r = -EFAULT;
> +		if (copy_to_user(user_msr_list->indices, &msrs_to_save,
> +				 num_msrs_to_save * sizeof(u32)))
> +			goto out;
> +		r = 0;
> +		break;
> +	}
> +	default:
> +		r = -ENOIOCTLCMD;
> +	}
> +out:
> +	return r;
> +}
> +
>  static int kvm_trap_emul_vm_init(struct kvm *kvm)
>  {
>  	return 0;
> @@ -471,8 +729,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.3

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-19 12:52   ` Gleb Natapov
@ 2013-05-19 14:36     ` Sanjay Lal
  2013-05-21  8:00       ` Gleb Natapov
  0 siblings, 1 reply; 12+ messages in thread
From: Sanjay Lal @ 2013-05-19 14:36 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: linux-mips, kvm, ralf, mtosatti


On May 19, 2013, at 8:52 AM, Gleb Natapov wrote:

> On Sat, May 18, 2013 at 06:54:24AM -0700, Sanjay Lal wrote:
>> - As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
>>  Memory slots should be acccessed from a SRCU read section.
>> - kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
>> if it cannot find a mapping for a particular gfn.
>> 
>> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
>> ---
>> arch/mips/kvm/kvm_tlb.c | 36 +++++++++++++++++++++++++++---------
>> 1 file changed, 27 insertions(+), 9 deletions(-)
>> 
>> diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
>> index 89511a9..ab2e9b0 100644
>> --- a/arch/mips/kvm/kvm_tlb.c
>> +++ b/arch/mips/kvm/kvm_tlb.c
>> @@ -16,7 +16,10 @@
>> #include <linux/mm.h>
>> #include <linux/delay.h>
>> #include <linux/module.h>
>> +#include <linux/bootmem.h>
> You haven't answered it when I asked it on v2:
> Is this include still needed now when export of min_low_pfn is not
> longer here?
> 

Sorry about that, juggling too many patches, bootmem.h is no longer needed in kvm_tlb.c.  Actually, I thought I had removed it before posting v3.

Regards
Sanjay

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI.
  2013-05-19 14:17   ` Gleb Natapov
@ 2013-05-19 21:17     ` David Daney
  2013-05-20  6:02       ` Gleb Natapov
  0 siblings, 1 reply; 12+ messages in thread
From: David Daney @ 2013-05-19 21:17 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Sanjay Lal, linux-mips, kvm, ralf, mtosatti, David Daney

On 05/19/2013 07:17 AM, Gleb Natapov wrote:
> On Sat, May 18, 2013 at 06:54:26AM -0700, Sanjay Lal wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> There are several parts to this:
>>
>> o All registers are 64-bits wide, 32-bit guests use the least
>>    significant portion of the register storage fields.
>>
>> o FPU register formats are defined.
>>
>> o CP0 Registers are manipulated via the KVM_GET_MSRS/KVM_SET_MSRS
>>    mechanism.
>>
>> The vcpu_ioctl_get_regs and vcpu_ioctl_set_regs function pointers
>> become unused so they were removed.
>>
>> Some IOCTL functions were moved to kvm_trap_emul because the
>> implementations are only for that flavor of KVM host.  In the future, if
>> hardware based virtualization is added, they can be hidden behind
>> function pointers as appropriate.
>>
> David, can you please divide this one big patch to smaller patches
> with each one having only one of the changes listed above?

Expanding the registers to 64 bits changes only four lines. Defining the 
FPU registers is an additional seven lines.  The rest really has to be 
an atomic change.

The point here is that we change the ABI.  Any userspace tools have to 
change too.  So is it better to have a multi-part patch set where the 
interface is unusable in the intermediate patches?  Or is it preferable 
to do an 'atomic' switch?

It wasn't out of laziness that I chose to do it this way, it was because 
I thought it was cleaner.

So to directly answer your question:  I prefer not to split this up, and 
would want to have a better reason than an orthodox interpretation of 
SubmittingPatches sec. 3.

David Daney

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI.
  2013-05-19 21:17     ` David Daney
@ 2013-05-20  6:02       ` Gleb Natapov
  0 siblings, 0 replies; 12+ messages in thread
From: Gleb Natapov @ 2013-05-20  6:02 UTC (permalink / raw)
  To: David Daney; +Cc: Sanjay Lal, linux-mips, kvm, ralf, mtosatti, David Daney

On Sun, May 19, 2013 at 02:17:33PM -0700, David Daney wrote:
> On 05/19/2013 07:17 AM, Gleb Natapov wrote:
> >On Sat, May 18, 2013 at 06:54:26AM -0700, Sanjay Lal wrote:
> >>From: David Daney <david.daney@cavium.com>
> >>
> >>There are several parts to this:
> >>
> >>o All registers are 64-bits wide, 32-bit guests use the least
> >>   significant portion of the register storage fields.
> >>
> >>o FPU register formats are defined.
> >>
> >>o CP0 Registers are manipulated via the KVM_GET_MSRS/KVM_SET_MSRS
> >>   mechanism.
> >>
> >>The vcpu_ioctl_get_regs and vcpu_ioctl_set_regs function pointers
> >>become unused so they were removed.
> >>
> >>Some IOCTL functions were moved to kvm_trap_emul because the
> >>implementations are only for that flavor of KVM host.  In the future, if
> >>hardware based virtualization is added, they can be hidden behind
> >>function pointers as appropriate.
> >>
> >David, can you please divide this one big patch to smaller patches
> >with each one having only one of the changes listed above?
> 
> Expanding the registers to 64 bits changes only four lines. Defining
> the FPU registers is an additional seven lines.  The rest really has
> to be an atomic change.
> 
It does not matter. If you have 10 logically unrelated one-liners (even
if they are all part of one big goal) I expect to get 10 patches.

> The point here is that we change the ABI.  Any userspace tools have
> to change too.  So is it better to have a multi-part patch set where
> the interface is unusable in the intermediate patches?  Or is it
> preferable to do an 'atomic' switch?
Are "The vcpu_ioctl_get_regs and vcpu_ioctl_set_regs function pointers
become unused so they were removed." and "Some IOCTL functions were
moved to kvm_trap_emul..." also changes ABI? Unlikely, and then I expect
to have two series: first one only have patches that change ABI and
another rearrange the code. First one will go into 3.10 second in 3.11.

> 
> It wasn't out of laziness that I chose to do it this way, it was
> because I thought it was cleaner.
> 
> So to directly answer your question:  I prefer not to split this up,
> and would want to have a better reason than an orthodox
> interpretation of SubmittingPatches sec. 3.
> 
It may seams orthodox interpretation if you are on a sender side, from
a reviewer point of view it is the interpretation that saves a lot of
time. I did looked into the patch before asking for split, not just
asked for it based on the description. And, in addition, in this case,
I want to have minimal set of changes that will go into 3.10.

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-19 14:36     ` Sanjay Lal
@ 2013-05-21  8:00       ` Gleb Natapov
  2013-05-21 14:22         ` Sanjay Lal
  0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2013-05-21  8:00 UTC (permalink / raw)
  To: Sanjay Lal; +Cc: linux-mips, kvm, ralf, mtosatti

On Sun, May 19, 2013 at 10:36:32AM -0400, Sanjay Lal wrote:
> 
> On May 19, 2013, at 8:52 AM, Gleb Natapov wrote:
> 
> > On Sat, May 18, 2013 at 06:54:24AM -0700, Sanjay Lal wrote:
> >> - As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
> >>  Memory slots should be acccessed from a SRCU read section.
> >> - kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
> >> if it cannot find a mapping for a particular gfn.
> >> 
> >> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
> >> ---
> >> arch/mips/kvm/kvm_tlb.c | 36 +++++++++++++++++++++++++++---------
> >> 1 file changed, 27 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
> >> index 89511a9..ab2e9b0 100644
> >> --- a/arch/mips/kvm/kvm_tlb.c
> >> +++ b/arch/mips/kvm/kvm_tlb.c
> >> @@ -16,7 +16,10 @@
> >> #include <linux/mm.h>
> >> #include <linux/delay.h>
> >> #include <linux/module.h>
> >> +#include <linux/bootmem.h>
> > You haven't answered it when I asked it on v2:
> > Is this include still needed now when export of min_low_pfn is not
> > longer here?
> > 
> 
> Sorry about that, juggling too many patches, bootmem.h is no longer needed in kvm_tlb.c.  Actually, I thought I had removed it before posting v3.
> 
Should I expect new version, or can I just drop this include from the
patch and apply?

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-21  8:00       ` Gleb Natapov
@ 2013-05-21 14:22         ` Sanjay Lal
  0 siblings, 0 replies; 12+ messages in thread
From: Sanjay Lal @ 2013-05-21 14:22 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: linux-mips, kvm, ralf, mtosatti


On May 21, 2013, at 1:00 AM, Gleb Natapov wrote:

> On Sun, May 19, 2013 at 10:36:32AM -0400, Sanjay Lal wrote:
>> 
>> On May 19, 2013, at 8:52 AM, Gleb Natapov wrote:
>> 
>>> On Sat, May 18, 2013 at 06:54:24AM -0700, Sanjay Lal wrote:
>>>> - As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
>>>> Memory slots should be acccessed from a SRCU read section.
>>>> - kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
>>>> if it cannot find a mapping for a particular gfn.
>>>> 
>>>> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
>>>> ---
>>>> arch/mips/kvm/kvm_tlb.c | 36 +++++++++++++++++++++++++++---------
>>>> 1 file changed, 27 insertions(+), 9 deletions(-)
>>>> 
>>>> diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
>>>> index 89511a9..ab2e9b0 100644
>>>> --- a/arch/mips/kvm/kvm_tlb.c
>>>> +++ b/arch/mips/kvm/kvm_tlb.c
>>>> @@ -16,7 +16,10 @@
>>>> #include <linux/mm.h>
>>>> #include <linux/delay.h>
>>>> #include <linux/module.h>
>>>> +#include <linux/bootmem.h>
>>> You haven't answered it when I asked it on v2:
>>> Is this include still needed now when export of min_low_pfn is not
>>> longer here?
>>> 
>> 
>> Sorry about that, juggling too many patches, bootmem.h is no longer needed in kvm_tlb.c.  Actually, I thought I had removed it before posting v3.
>> 
> Should I expect new version, or can I just drop this include from the
> patch and apply?
> 
Please drop the include.

Regards
Sanjay

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-05-21 14:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-18 13:54 [PATCH v3 0/4] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
2013-05-18 13:54 ` [PATCH 1/4] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
2013-05-18 13:54 ` [PATCH 2/4] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
2013-05-19 12:52   ` Gleb Natapov
2013-05-19 14:36     ` Sanjay Lal
2013-05-21  8:00       ` Gleb Natapov
2013-05-21 14:22         ` Sanjay Lal
2013-05-18 13:54 ` [PATCH 3/4] KVM/MIPS32: Export min_low_pfn Sanjay Lal
2013-05-18 13:54 ` [PATCH 4/4] KVM/MIPS32: Bring in patch from David Daney with new 64 bit compatible ABI Sanjay Lal
2013-05-19 14:17   ` Gleb Natapov
2013-05-19 21:17     ` David Daney
2013-05-20  6:02       ` Gleb Natapov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.