Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.
On 11/02/2012 07:11 PM, Sanjay Lal wrote:
On Nov 1, 2012, at 11:04 AM, Avi Kivity wrote:
On 10/31/2012 05:18 PM, Sanjay Lal wrote:
+
+/* Special address that contains the comm page, used for reducing # of
traps */
+#define KVM_GUEST_COMMPAGE_ADDR 0x0
+
+struct kvm_arch
+{
+/* Guest GVA-HPA page table */
+ulong *guest_pmap;
+ulong guest_pmap_npages;
+
+/* Wired host TLB used for the commpage */
+int commpage_tlb;
+
+pfn_t (*gfn_to_pfn) (struct kvm *kvm, gfn_t gfn);
+void (*release_pfn_clean) (pfn_t pfn);
+bool (*is_error_pfn) (pfn_t pfn);
Why this indirection? Do those functions change at runtime?
On MIPS, kernel modules are executed from mapped space, which requires
TLBs. The TLB handling code is statically linked with the rest of the kernel
(kvm_tlb.c) to avoid the possibility of double faulting. The problem is that
the code references routines that are part of the the KVM module, which are
only available once the module is loaded, hence the indirection.
Ok. These should be global function pointers then, assigned when the
module is loaded, and cleared when it is unloaded, with a comment
explaining why.
+
+struct kvm_mips_callbacks {
+int (*handle_cop_unusable)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_mod)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_ld_miss)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_st_miss)(struct kvm_vcpu *vcpu);
+int (*handle_addr_err_st)(struct kvm_vcpu *vcpu);
+int (*handle_addr_err_ld)(struct kvm_vcpu *vcpu);
+int (*handle_syscall)(struct kvm_vcpu *vcpu);
+int (*handle_res_inst)(struct kvm_vcpu *vcpu);
+int (*handle_break)(struct kvm_vcpu *vcpu);
+gpa_t (*gva_to_gpa)(gva_t gva);
+void (*queue_timer_int)(struct kvm_vcpu *vcpu);
+void (*dequeue_timer_int)(struct kvm_vcpu *vcpu);
+void (*queue_io_int)(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt
*irq);
+void (*dequeue_io_int)(struct kvm_vcpu *vcpu, struct
kvm_mips_interrupt *irq);
+int (*irq_deliver)(struct kvm_vcpu *vcpu, unsigned int priority,
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);
+int (*vcpu_init)(struct kvm_vcpu *vcpu);
+};
We use callbacks on x86 because we have two separate implementations
(svm and vmx). Will that be the case on MIPS? If not, use direct calls.
We will eventually have separate implementations based on the features
supported by H/W.
It's probably better to add the indirection when the need arrives,
unless you already have hardware specs and know exactly the best points
for a split.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.
On Nov 1, 2012, at 11:04 AM, Avi Kivity wrote:
On 10/31/2012 05:18 PM, Sanjay Lal wrote:
+
+/* Special address that contains the comm page, used for reducing # of
traps */
+#define KVM_GUEST_COMMPAGE_ADDR 0x0
+
+struct kvm_arch
+{
+/* Guest GVA-HPA page table */
+ulong *guest_pmap;
+ulong guest_pmap_npages;
+
+/* Wired host TLB used for the commpage */
+int commpage_tlb;
+
+pfn_t (*gfn_to_pfn) (struct kvm *kvm, gfn_t gfn);
+void (*release_pfn_clean) (pfn_t pfn);
+bool (*is_error_pfn) (pfn_t pfn);
Why this indirection? Do those functions change at runtime?
On MIPS, kernel modules are executed from mapped space, which requires TLBs.
The TLB handling code is statically linked with the rest of the kernel
(kvm_tlb.c) to avoid the possibility of double faulting. The problem is that
the code references routines that are part of the the KVM module, which are
only available once the module is loaded, hence the indirection.
+
+struct kvm_mips_callbacks {
+int (*handle_cop_unusable)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_mod)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_ld_miss)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_st_miss)(struct kvm_vcpu *vcpu);
+int (*handle_addr_err_st)(struct kvm_vcpu *vcpu);
+int (*handle_addr_err_ld)(struct kvm_vcpu *vcpu);
+int (*handle_syscall)(struct kvm_vcpu *vcpu);
+int (*handle_res_inst)(struct kvm_vcpu *vcpu);
+int (*handle_break)(struct kvm_vcpu *vcpu);
+gpa_t (*gva_to_gpa)(gva_t gva);
+void (*queue_timer_int)(struct kvm_vcpu *vcpu);
+void (*dequeue_timer_int)(struct kvm_vcpu *vcpu);
+void (*queue_io_int)(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt
*irq);
+void (*dequeue_io_int)(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt
*irq);
+int (*irq_deliver)(struct kvm_vcpu *vcpu, unsigned int priority,
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);
+int (*vcpu_init)(struct kvm_vcpu *vcpu);
+};
We use callbacks on x86 because we have two separate implementations
(svm and vmx). Will that be the case on MIPS? If not, use direct calls.
We will eventually have separate implementations based on the features
supported by H/W.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.
On 10/31/2012 05:18 PM, Sanjay Lal wrote:
Signed-off-by: Sanjay Lal sanj...@kymasys.com
---
arch/mips/include/asm/kvm.h | 58
arch/mips/include/asm/kvm_host.h | 672
+++
2 files changed, 730 insertions(+)
create mode 100644 arch/mips/include/asm/kvm.h
create mode 100644 arch/mips/include/asm/kvm_host.h
diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
new file mode 100644
index 000..39bb715
--- /dev/null
+++ b/arch/mips/include/asm/kvm.h
@@ -0,0 +1,58 @@
+/*
+* 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 sanj...@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_SEL8
+
+/* for KVM_GET_REGS and KVM_SET_REGS */
+struct kvm_regs {
+__u32 gprs[32];
+__u32 hi;
+__u32 lo;
+__u32 pc;
+
+ulong cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
+};
ulong changes size in 64-bit archs, requiring compat translations when
issuing 32-bit syscalls on a 64-bit kernel. I don't know MIPS enough to
know whether that's a useful scenario.
+
+#define KVM_MAX_VCPUS 8
Set to 1 until smp is supported.
+#define KVM_MEMORY_SLOTS 32
+/* memory slots that does not exposed to userspace */
+#define KVM_PRIVATE_MEM_SLOTS 4
Do you really need those?
+
+#define ENTER_CRITICAL(flags) local_irq_save(flags)
+#define EXIT_CRITICAL(flags)local_irq_restore(flags)
Why wrap?
+
+
+#define KVM_GUEST_KERNEL_ASID
((vcpu-arch.guest_kernel_asid[smp_processor_id()]) ASID_MASK)
+
+#define KVM_GUEST_USER_ASID
((vcpu-arch.guest_user_asid[smp_processor_id()]) ASID_MASK)
+
+
+#define KVM_GUEST_WIRED_TLBS(current_cpu_data.tlbsize)
+#define KVM_GUEST_COMMPAGE_TLB (vcpu-kvm-arch.commpage_tlb)
+#define KVM_GUEST_TLBS KVM_GUEST_WIRED_TLBS
Don't use defines for variable expressions. Use inline functions for
those, or just open-code when it's more readable.
+
+/* Special address that contains the comm page, used for reducing # of traps
*/
+#define KVM_GUEST_COMMPAGE_ADDR 0x0
+
+struct kvm_arch
+{
+/* Guest GVA-HPA page table */
+ulong *guest_pmap;
+ulong guest_pmap_npages;
+
+/* Wired host TLB used for the commpage */
+int commpage_tlb;
+
+pfn_t (*gfn_to_pfn) (struct kvm *kvm, gfn_t gfn);
+void (*release_pfn_clean) (pfn_t pfn);
+bool (*is_error_pfn) (pfn_t pfn);
Why this indirection? Do those functions change at runtime?
+
+/* Stats for exit reasons */
+ulong exit_reason_stats[MAX_KVM_MIPS_EXIT_TYPES];
+};
Please use tracepoints for statistics instead of manual collection +
debugfs.
+
+struct kvm_mips_callbacks {
+int (*handle_cop_unusable)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_mod)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_ld_miss)(struct kvm_vcpu *vcpu);
+int (*handle_tlb_st_miss)(struct kvm_vcpu *vcpu);
+int (*handle_addr_err_st)(struct kvm_vcpu *vcpu);
+int (*handle_addr_err_ld)(struct kvm_vcpu *vcpu);
+int (*handle_syscall)(struct kvm_vcpu *vcpu);
+int (*handle_res_inst)(struct kvm_vcpu *vcpu);
+int (*handle_break)(struct kvm_vcpu *vcpu);
+gpa_t (*gva_to_gpa)(gva_t gva);
+void (*queue_timer_int)(struct kvm_vcpu *vcpu);
+void (*dequeue_timer_int)(struct kvm_vcpu *vcpu);
+void (*queue_io_int)(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt
*irq);
+void (*dequeue_io_int)(struct kvm_vcpu *vcpu, struct kvm_mips_interrupt
*irq);
+int (*irq_deliver)(struct kvm_vcpu *vcpu, unsigned int priority,
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);
+int (*vcpu_init)(struct kvm_vcpu *vcpu);
+};
We use callbacks on x86 because we have two separate implementations
(svm and vmx). Will that be the case on MIPS? If not, use direct calls.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.
On 11/01/2012 08:04 AM, Avi Kivity wrote:
On 10/31/2012 05:18 PM, Sanjay Lal wrote:
Signed-off-by: Sanjay Lal sanj...@kymasys.com
---
arch/mips/include/asm/kvm.h | 58
arch/mips/include/asm/kvm_host.h | 672 +++
2 files changed, 730 insertions(+)
create mode 100644 arch/mips/include/asm/kvm.h
create mode 100644 arch/mips/include/asm/kvm_host.h
diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
new file mode 100644
index 000..39bb715
--- /dev/null
+++ b/arch/mips/include/asm/kvm.h
@@ -0,0 +1,58 @@
+/*
+* 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 sanj...@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;
+
+ulong cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
+};
ulong changes size in 64-bit archs, requiring compat translations when
issuing 32-bit syscalls on a 64-bit kernel. I don't know MIPS enough to
know whether that's a useful scenario.
For a 32-bit machine all the registers are 32-bits wide.
For a 64-bit machine, they are ... yes 64-bits wide.
Since this entire structure seems to be useless for a 64-bit
implementation just use __u32 for everything.
This does beg the question though, how will you handle 64-bit machines?
Why not name the entire structure something like struct kvm_regs_mips32?
Then for the 64-bit implementation use struct kvm_regs_mips64.
I haven't studied it enough to know if you can do this. if it needs to
be called exactly 'struct kvm_regs', then you have to make everything
__u64 and only populate the lower halves of the fields for mips32.
Watch out for endian issues in this case.
David Daney
--
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 02/20] KVM/MIPS32: Arch specific KVM data structures.
Signed-off-by: Sanjay Lal sanj...@kymasys.com
---
arch/mips/include/asm/kvm.h | 58
arch/mips/include/asm/kvm_host.h | 672 +++
2 files changed, 730 insertions(+)
create mode 100644 arch/mips/include/asm/kvm.h
create mode 100644 arch/mips/include/asm/kvm_host.h
diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
new file mode 100644
index 000..39bb715
--- /dev/null
+++ b/arch/mips/include/asm/kvm.h
@@ -0,0 +1,58 @@
+/*
+* 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 sanj...@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;
+
+ulong 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/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
new file mode 100644
index 000..0352106
--- /dev/null
+++ b/arch/mips/include/asm/kvm_host.h
@@ -0,0 +1,672 @@
+/*
+* 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.
+*
+* PUT YOUR TITLE AND/OR INFORMATION FOR THE FILE HERE
+*
+* Copyright (C) 2012 MIPS Technologies, Inc. All rights reserved.
+* Authors: Sanjay Lal sanj...@kymasys.com
+*/
+
+#ifndef __MIPS_KVM_HOST_H__
+#define __MIPS_KVM_HOST_H__
+
+#include linux/mutex.h
+#include linux/hrtimer.h
+#include linux/interrupt.h
+#include linux/types.h
+#include linux/kvm_types.h
+#include linux/threads.h
+#include linux/spinlock.h
+
+#define KVM_MAX_VCPUS 8
+#define KVM_MEMORY_SLOTS 32
+/* memory slots that does not exposed to userspace */
+#define KVM_PRIVATE_MEM_SLOTS 4
+
+#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
+
+/* Don't support huge pages */
+#define KVM_HPAGE_GFN_SHIFT(x) 0
+
+/* We don't currently support large pages. */
+#define KVM_NR_PAGE_SIZES 1
+#define KVM_PAGES_PER_HPAGE(x) 1
+
+#define ENTER_CRITICAL(flags) local_irq_save(flags)
+#define EXIT_CRITICAL(flags)local_irq_restore(flags)
+
+
+#define KVM_GUEST_KERNEL_ASID
((vcpu-arch.guest_kernel_asid[smp_processor_id()]) ASID_MASK)
+
+#define KVM_GUEST_USER_ASID
((vcpu-arch.guest_user_asid[smp_processor_id()]) ASID_MASK)
+
+
+#define KVM_GUEST_WIRED_TLBS(current_cpu_data.tlbsize)
+#define KVM_GUEST_COMMPAGE_TLB (vcpu-kvm-arch.commpage_tlb)
+#define KVM_GUEST_TLBS KVM_GUEST_WIRED_TLBS
+
+/* Special address that contains the comm page, used for reducing # of traps */
+#define KVM_GUEST_COMMPAGE_ADDR 0x0
+
+#define KVM_GUEST_KERNEL_MODE(vcpu)
((kvm_read_c0_guest_status(vcpu-arch.cop0) (ST0_EXL | ST0_ERL)) || \
+
((kvm_read_c0_guest_status(vcpu-arch.cop0) KSU_USER) == 0))
+
+#define KVM_GUEST_KUSEG 0xUL
+#define KVM_GUEST_KSEG0 0x4000UL
+#define KVM_GUEST_KSEG230x6000UL
+#define KVM_GUEST_KSEGX(a) ((_ACAST32_(a)) 0x6000)
+#define KVM_GUEST_CPHYSADDR(a) ((_ACAST32_(a)) 0x1fff)
+
+#define KVM_GUEST_CKSEG0ADDR(a)(KVM_GUEST_CPHYSADDR(a) |
KVM_GUEST_KSEG0)
+#define KVM_GUEST_CKSEG1ADDR(a)(KVM_GUEST_CPHYSADDR(a) |
KVM_GUEST_KSEG1)
+#define KVM_GUEST_CKSEG23ADDR(a) (KVM_GUEST_CPHYSADDR(a) |
KVM_GUEST_KSEG23)
+
+/*
+ * Map an address to a certain kernel segment
+ */
+#define KVM_GUEST_KSEG0ADDR(a) (KVM_GUEST_CPHYSADDR(a) | KSEG0)
+#define KVM_GUEST_KSEG23ADDR(a)(KVM_GUEST_CPHYSADDR(a) |
KVM_GUEST_KSEG23)
+
+#define KVM_INVALID_PAGE0xdeadbeef
+#define KVM_INVALID_INST0xdeadbeef
+#define KVM_INVALID_ADDR0xdeadbeef
+
+#define KVM_MALTA_GUEST_RTC_ADDR0xb870UL
+
+#ifndef __unused
+#define __unused __attribute__((unused))
+#endif
+
+#define GUEST_TICKS_PER_JIFFY (4000/HZ)
+#define MS_TO_NS(x) (x * 1E6L)
+
+#define CAUSEB_DC 27
+#define CAUSEF_DC (_ULCAST_(1)27)
+
+struct kvm;
+struct kvm_run;
+struct kvm_vcpu;
+struct kvm_interrupt;
+
+extern atomic_t kvm_mips_instance;
+
+struct kvm_vm_stat
+{
+u32 remote_tlb_flush;
+};
+
+struct kvm_vcpu_stat
+{
+
