linux-mips
[Top] [All Lists]

Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.

To: Sanjay Lal <sanjayl@kymasys.com>
Subject: Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.
From: Avi Kivity <avi@redhat.com>
Date: Thu, 01 Nov 2012 17:04:39 +0200
Cc: kvm@vger.kernel.org, linux-mips@linux-mips.org
In-reply-to: <54507365-0EF7-480A-8A54-75E12B3677D9@kymasys.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
References: <54507365-0EF7-480A-8A54-75E12B3677D9@kymasys.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121016 Thunderbird/16.0.1
On 10/31/2012 05:18 PM, Sanjay Lal wrote:
> 
> Signed-off-by: Sanjay Lal <sanjayl@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 0000000..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 <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;
> +
> +    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

<Prev in Thread] Current Thread [Next in Thread>