From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH 06/15] KVM: ARM: Initial skeleton to compile KVM support Date: Tue, 25 Sep 2012 21:43:14 -0400 Message-ID: <50625DB2.4000700@virtualopensystems.com> References: <20120915153359.21241.86002.stgit@ubuntu> <20120915153508.21241.47712.stgit@ubuntu> <20120925152055.GC28728@mudshark.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "kvm@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "kvmarm@lists.cs.columbia.edu" , rusty.russell@linaro.org, avi@redhat.com To: Will Deacon Return-path: Received: from mail-pa0-f46.google.com ([209.85.220.46]:51010 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753531Ab2IZPrc (ORCPT ); Wed, 26 Sep 2012 11:47:32 -0400 Received: by padhz1 with SMTP id hz1so551430pad.19 for ; Wed, 26 Sep 2012 08:47:32 -0700 (PDT) In-Reply-To: <20120925152055.GC28728@mudshark.cambridge.arm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 09/25/2012 11:20 AM, Will Deacon wrote: > On Sat, Sep 15, 2012 at 04:35:08PM +0100, Christoffer Dall wrote: >> Targets KVM support for Cortex A-15 processors. >> >> Contains all the framework components, make files, header files, som= e >> tracing functionality, and basic user space API. >> >> Only supported core is Cortex-A15 for now. >> >> Most functionality is in arch/arm/kvm/* or arch/arm/include/asm/kvm_= *.h. >> >> =E2=80=9CNothing to see here. Move along, move along..." > > [...] > >> diff --git a/arch/arm/include/asm/kvm.h b/arch/arm/include/asm/kvm.h >> new file mode 100644 >> index 0000000..a13b582 >> --- /dev/null >> +++ b/arch/arm/include/asm/kvm.h >> @@ -0,0 +1,88 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#ifndef __ARM_KVM_H__ >> +#define __ARM_KVM_H__ >> + >> +#include >> + >> +#define __KVM_HAVE_GUEST_DEBUG >> + >> +#define KVM_REG_SIZE(id) = \ >> + (1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT)) >> + >> +struct kvm_regs { >> + __u32 usr_regs[15]; /* R0_usr - R14_usr */ >> + __u32 svc_regs[3]; /* SP_svc, LR_svc, SPSR_svc */ >> + __u32 abt_regs[3]; /* SP_abt, LR_abt, SPSR_abt */ >> + __u32 und_regs[3]; /* SP_und, LR_und, SPSR_und */ >> + __u32 irq_regs[3]; /* SP_irq, LR_irq, SPSR_irq */ >> + __u32 fiq_regs[8]; /* R8_fiq - R14_fiq, SPSR_fiq */ >> + __u32 pc; /* The program counter (r15) */ >> + __u32 cpsr; /* The guest CPSR */ >> +}; >> + >> +/* Supported Processor Types */ >> +#define KVM_ARM_TARGET_CORTEX_A15 (0xC0F) > > So there's this define... > >> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/k= vm_arm.h >> new file mode 100644 >> index 0000000..2f9d28e >> --- /dev/null >> +++ b/arch/arm/include/asm/kvm_arm.h >> @@ -0,0 +1,28 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#ifndef __ARM_KVM_ARM_H__ >> +#define __ARM_KVM_ARM_H__ >> + >> +/* Supported Processor Types */ >> +#define CORTEX_A15 (0xC0F) > > ... and then this one. Do we really need both? > no, we don't >> +/* Multiprocessor Affinity Register */ >> +#define MPIDR_CPUID (0x3 << 0) > > I'm fairly sure we already have code under arch/arm/ for dealing with= the > mpidr. Let's re-use that rather than reinventing it here. > I see some defines in topology.c - do you want some of these factored=20 out into a header file that we can then also use from kvm? If so, where= ? >> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/k= vm_asm.h >> new file mode 100644 >> index 0000000..44591f9 >> --- /dev/null >> +++ b/arch/arm/include/asm/kvm_asm.h >> @@ -0,0 +1,30 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#ifndef __ARM_KVM_ASM_H__ >> +#define __ARM_KVM_ASM_H__ >> + >> +#define ARM_EXCEPTION_RESET 0 >> +#define ARM_EXCEPTION_UNDEFINED 1 >> +#define ARM_EXCEPTION_SOFTWARE 2 >> +#define ARM_EXCEPTION_PREF_ABORT 3 >> +#define ARM_EXCEPTION_DATA_ABORT 4 >> +#define ARM_EXCEPTION_IRQ 5 >> +#define ARM_EXCEPTION_FIQ 6 > > Again, you have these defines (which look more suited to an enum type= ), but > later (in kvm_host.h) you have: well, unless I miss some known trick, assembly code doesn't like enums? > >> +#define EXCEPTION_NONE 0 >> +#define EXCEPTION_RESET 0x80 >> +#define EXCEPTION_UNDEFINED 0x40 >> +#define EXCEPTION_SOFTWARE 0x20 >> +#define EXCEPTION_PREFETCH 0x10 >> +#define EXCEPTION_DATA 0x08 >> +#define EXCEPTION_IMPRECISE 0x04 >> +#define EXCEPTION_IRQ 0x02 >> +#define EXCEPTION_FIQ 0x01 > > Why the noise? > these are simply cruft from a previous life of KVM/ARM. >> diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/a= sm/kvm_emulate.h >> new file mode 100644 >> index 0000000..9e29335 >> --- /dev/null >> +++ b/arch/arm/include/asm/kvm_emulate.h >> @@ -0,0 +1,108 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#ifndef __ARM_KVM_EMULATE_H__ >> +#define __ARM_KVM_EMULATE_H__ >> + >> +#include >> +#include >> + >> +u32 *vcpu_reg_mode(struct kvm_vcpu *vcpu, u8 reg_num, enum vcpu_mod= e mode); >> + >> +static inline u8 __vcpu_mode(u32 cpsr) >> +{ >> + u8 modes_table[32] =3D { >> + 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, >> + 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, >> + MODE_USR, /* 0x0 */ >> + MODE_FIQ, /* 0x1 */ >> + MODE_IRQ, /* 0x2 */ >> + MODE_SVC, /* 0x3 */ >> + 0xf, 0xf, 0xf, >> + MODE_ABT, /* 0x7 */ >> + 0xf, 0xf, 0xf, >> + MODE_UND, /* 0xb */ >> + 0xf, 0xf, 0xf, >> + MODE_SYS /* 0xf */ >> + }; > > These MODE_* definitions sound like our *_MODE definitions... except = they're > not. It would probably be far more readable as a switch, but at least= change > the name of those things! fair enough, they're renamed to VCPU_XXX_MODE >> +static inline enum vcpu_mode vcpu_mode(struct kvm_vcpu *vcpu) >> +{ >> + u8 mode =3D __vcpu_mode(vcpu->arch.regs.cpsr); >> + BUG_ON(mode =3D=3D 0xf); >> + return mode; >> +} > > I noticed that you have a fair few BUG_ONs throughout the series. Fai= r > enough, but for hyp code is that really the right thing to do? Killin= g the > guest could make more sense, perhaps? the idea is to have BUG_ONs that are indeed BUG_ONs that we want to=20 catch explicitly on the host. We have had a pass over the code to chang= e=20 all the BUG_ONs that can be provoked by the guest and inject the proper= =20 exceptions into the guest in this case. If you find places where this i= s=20 not the case, it should be changed, and do let me know. > >> +static inline u32 *vcpu_pc(struct kvm_vcpu *vcpu) >> +{ >> + return vcpu_reg(vcpu, 15); >> +} > > If you stick a struct pt_regs into struct kvm_regs, you could reuse A= RM_pc > here etc. > I prefer not to, because we'd have those registers presumably for usr=20 mode and then we only define the others explicit. I think it's much=20 clearer to look at kvm_regs today. >> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/= kvm_host.h >> new file mode 100644 >> index 0000000..24959f4 >> --- /dev/null >> +++ b/arch/arm/include/asm/kvm_host.h >> @@ -0,0 +1,172 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#ifndef __ARM_KVM_HOST_H__ >> +#define __ARM_KVM_HOST_H__ >> + >> +#include >> + >> +#define KVM_MAX_VCPUS 4 > > NR_CPUS? > well this is defined by KVM generic code, and is common for other=20 architecture. >> +#define KVM_MEMORY_SLOTS 32 >> +#define KVM_PRIVATE_MEM_SLOTS 4 >> +#define KVM_COALESCED_MMIO_PAGE_OFFSET 1 >> + >> +#define NUM_FEATURES 0 > > Ha! No idea what that means, but hopefully there's less code to revie= w > because of it :) > that's actually true. will rename to KVM_VCPU_MAX_FEATURES (or do you want NR in this case? :-\) >> + >> +/* We don't currently support large pages. */ >> +#define KVM_HPAGE_GFN_SHIFT(x) 0 >> +#define KVM_NR_PAGE_SIZES 1 >> +#define KVM_PAGES_PER_HPAGE(x) (1UL<<31) >> + >> +struct kvm_vcpu; >> +u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode); >> +int kvm_target_cpu(void); >> +int kvm_reset_vcpu(struct kvm_vcpu *vcpu); >> +void kvm_reset_coprocs(struct kvm_vcpu *vcpu); >> + >> +struct kvm_arch { >> + /* The VMID generation used for the virt. memory system */ >> + u64 vmid_gen; >> + u32 vmid; >> + >> + /* 1-level 2nd stage table and lock */ >> + spinlock_t pgd_lock; >> + pgd_t *pgd; >> + >> + /* VTTBR value associated with above pgd and vmid */ >> + u64 vttbr; >> +}; >> + >> +#define EXCEPTION_NONE 0 >> +#define EXCEPTION_RESET 0x80 >> +#define EXCEPTION_UNDEFINED 0x40 >> +#define EXCEPTION_SOFTWARE 0x20 >> +#define EXCEPTION_PREFETCH 0x10 >> +#define EXCEPTION_DATA 0x08 >> +#define EXCEPTION_IMPRECISE 0x04 >> +#define EXCEPTION_IRQ 0x02 >> +#define EXCEPTION_FIQ 0x01 >> + >> +#define KVM_NR_MEM_OBJS 40 >> + >> +/* >> + * We don't want allocation failures within the mmu code, so we pre= allocate >> + * enough memory for a single page fault in a cache. >> + */ >> +struct kvm_mmu_memory_cache { >> + int nobjs; >> + void *objects[KVM_NR_MEM_OBJS]; >> +}; >> + >> +/* >> + * Modes used for short-hand mode determinition in the world-switch= code and >> + * in emulation code. >> + * >> + * Note: These indices do NOT correspond to the value of the CPSR m= ode bits! >> + */ >> +enum vcpu_mode { >> + MODE_FIQ =3D 0, >> + MODE_IRQ, >> + MODE_SVC, >> + MODE_ABT, >> + MODE_UND, >> + MODE_USR, >> + MODE_SYS >> +}; > > So the need for this enum is for indexing the array of modes, right? = But > accesses to that array are already hidden behind an accessor function= from > what I can tell, so I'd rather the arithmetic from cpsr -> index was > restricted to that function and the rest of the code just passed eith= er the > raw mode or the full cpsr around. > good point, this was really useful in that prior life of kvm/arm where=20 we did a bunch of emulation and decoding all over the place. I'll send=20 out a v2 with this reworked. >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c >> new file mode 100644 >> index 0000000..fd6fa9b >> --- /dev/null >> +++ b/arch/arm/kvm/arm.c >> @@ -0,0 +1,345 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define CREATE_TRACE_POINTS >> +#include "trace.h" >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#ifdef REQUIRES_VIRT >> +__asm__(".arch_extension virt"); >> +#endif >> + >> +int kvm_arch_hardware_enable(void *garbage) >> +{ >> + return 0; >> +} >> + >> +int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu) >> +{ >> + return kvm_vcpu_exiting_guest_mode(vcpu) =3D=3D IN_GUEST_MOD= E; >> +} >> + >> +void kvm_arch_hardware_disable(void *garbage) >> +{ >> +} >> + >> +int kvm_arch_hardware_setup(void) >> +{ >> + return 0; >> +} >> + >> +void kvm_arch_hardware_unsetup(void) >> +{ >> +} >> + >> +void kvm_arch_check_processor_compat(void *rtn) >> +{ >> + *(int *)rtn =3D 0; >> +} >> + >> +void kvm_arch_sync_events(struct kvm *kvm) >> +{ >> +} >> + >> +int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) >> +{ >> + if (type) >> + return -EINVAL; >> + >> + return 0; >> +} >> + >> +int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf= ) >> +{ >> + return VM_FAULT_SIGBUS; >> +} >> + >> +void kvm_arch_free_memslot(struct kvm_memory_slot *free, >> + struct kvm_memory_slot *dont) >> +{ >> +} >> + >> +int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned = long npages) >> +{ >> + return 0; >> +} >> + >> +void kvm_arch_destroy_vm(struct kvm *kvm) >> +{ >> + int i; >> + >> + for (i =3D 0; i < KVM_MAX_VCPUS; ++i) { >> + if (kvm->vcpus[i]) { >> + kvm_arch_vcpu_free(kvm->vcpus[i]); >> + kvm->vcpus[i] =3D NULL; >> + } >> + } >> +} >> + >> +int kvm_dev_ioctl_check_extension(long ext) >> +{ >> + int r; >> + switch (ext) { >> + case KVM_CAP_USER_MEMORY: >> + case KVM_CAP_DESTROY_MEMORY_REGION_WORKS: >> + case KVM_CAP_ONE_REG: >> + r =3D 1; >> + break; >> + case KVM_CAP_COALESCED_MMIO: >> + r =3D KVM_COALESCED_MMIO_PAGE_OFFSET; >> + break; >> + default: >> + r =3D 0; >> + break; >> + } >> + return r; >> +} >> + >> +long kvm_arch_dev_ioctl(struct file *filp, >> + unsigned int ioctl, unsigned long arg) >> +{ >> + return -EINVAL; >> +} >> + >> +int kvm_arch_set_memory_region(struct kvm *kvm, >> + struct kvm_userspace_memory_region *m= em, >> + struct kvm_memory_slot old, >> + int user_alloc) >> +{ >> + return 0; >> +} >> + >> +int kvm_arch_prepare_memory_region(struct kvm *kvm, >> + struct kvm_memory_slot *memslot, >> + struct kvm_memory_slot old, >> + struct kvm_userspace_memory_regio= n *mem, >> + int user_alloc) >> +{ >> + return 0; >> +} >> + >> +void kvm_arch_commit_memory_region(struct kvm *kvm, >> + struct kvm_userspace_memory_regio= n *mem, >> + struct kvm_memory_slot old, >> + int user_alloc) >> +{ >> +} >> + >> +void kvm_arch_flush_shadow_all(struct kvm *kvm) >> +{ >> +} >> + >> +void kvm_arch_flush_shadow_memslot(struct kvm *kvm, >> + struct kvm_memory_slot *slot) >> +{ >> +} >> + >> +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int= id) >> +{ >> + int err; >> + struct kvm_vcpu *vcpu; >> + >> + vcpu =3D kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); >> + if (!vcpu) { >> + err =3D -ENOMEM; >> + goto out; >> + } >> + >> + err =3D kvm_vcpu_init(vcpu, kvm, id); >> + if (err) >> + goto free_vcpu; >> + >> + return vcpu; >> +free_vcpu: >> + kmem_cache_free(kvm_vcpu_cache, vcpu); >> +out: >> + return ERR_PTR(err); >> +} >> + >> +void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) >> +{ >> +} >> + >> +void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) >> +{ >> + kvm_arch_vcpu_free(vcpu); >> +} >> + >> +int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu) >> +{ >> + return 0; >> +} >> + >> +int __attribute_const__ kvm_target_cpu(void) >> +{ >> + unsigned int midr; >> + >> + midr =3D read_cpuid_id(); >> + switch ((midr >> 4) & 0xfff) { >> + case KVM_ARM_TARGET_CORTEX_A15: >> + return KVM_ARM_TARGET_CORTEX_A15; > > I have this code already in perf_event.c. Can we move it somewhere co= mmon > and share it? You should also check that the implementor field is 0x4= 1. > by all means, you can probably suggest a good place better than I can..= =2E >> + default: >> + return -EINVAL; >> + } >> +} >> + >> +int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) >> +{ >> + return 0; >> +} >> + >> +void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) >> +{ >> +} >> + >> +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) >> +{ >> +} >> + >> +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) >> +{ >> +} >> + >> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, >> + struct kvm_guest_debug *dbg) >> +{ >> + return -EINVAL; >> +} >> + >> + >> +int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu, >> + struct kvm_mp_state *mp_state) >> +{ >> + return -EINVAL; >> +} >> + >> +int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, >> + struct kvm_mp_state *mp_state) >> +{ >> + return -EINVAL; >> +} >> + >> +int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) >> +{ >> + return 0; >> +} >> + >> +int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *v) >> +{ >> + return 0; >> +} >> + >> +int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *= run) >> +{ >> + return -EINVAL; >> +} >> + >> +long kvm_arch_vcpu_ioctl(struct file *filp, >> + unsigned int ioctl, unsigned long arg) >> +{ >> + struct kvm_vcpu *vcpu =3D filp->private_data; >> + void __user *argp =3D (void __user *)arg; >> + >> + switch (ioctl) { >> + case KVM_ARM_VCPU_INIT: { >> + struct kvm_vcpu_init init; >> + >> + if (copy_from_user(&init, argp, sizeof init)) >> + return -EFAULT; >> + >> + return kvm_vcpu_set_target(vcpu, &init); >> + >> + } >> + case KVM_SET_ONE_REG: >> + case KVM_GET_ONE_REG: { >> + struct kvm_one_reg reg; >> + if (copy_from_user(®, argp, sizeof(reg))) >> + return -EFAULT; >> + if (ioctl =3D=3D KVM_SET_ONE_REG) >> + return kvm_arm_set_reg(vcpu, ®); >> + else >> + return kvm_arm_get_reg(vcpu, ®); >> + } >> + case KVM_GET_REG_LIST: { >> + struct kvm_reg_list __user *user_list =3D argp; >> + struct kvm_reg_list reg_list; >> + unsigned n; >> + >> + if (copy_from_user(®_list, user_list, sizeof reg_= list)) >> + return -EFAULT; >> + n =3D reg_list.n; >> + reg_list.n =3D kvm_arm_num_regs(vcpu); >> + if (copy_to_user(user_list, ®_list, sizeof reg_li= st)) >> + return -EFAULT; >> + if (n < reg_list.n) >> + return -E2BIG; >> + return kvm_arm_copy_reg_indices(vcpu, user_list->reg= ); > > kvm_reg_list sounds like it could be done using a regset instead. > >> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c >> new file mode 100644 >> index 0000000..690bbb3 >> --- /dev/null >> +++ b/arch/arm/kvm/emulate.c >> @@ -0,0 +1,127 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#include >> + >> +#define REG_OFFSET(_reg) \ >> + (offsetof(struct kvm_regs, _reg) / sizeof(u32)) >> + >> +#define USR_REG_OFFSET(_num) REG_OFFSET(usr_regs[_num]) >> + >> +static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] =3D { >> + /* FIQ Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), >> + REG_OFFSET(fiq_regs[1]), /* r8 */ >> + REG_OFFSET(fiq_regs[1]), /* r9 */ >> + REG_OFFSET(fiq_regs[2]), /* r10 */ >> + REG_OFFSET(fiq_regs[3]), /* r11 */ >> + REG_OFFSET(fiq_regs[4]), /* r12 */ >> + REG_OFFSET(fiq_regs[5]), /* r13 */ >> + REG_OFFSET(fiq_regs[6]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> + >> + /* IRQ Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET= (8), >> + USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSE= T(11), >> + USR_REG_OFFSET(12), >> + REG_OFFSET(irq_regs[0]), /* r13 */ >> + REG_OFFSET(irq_regs[1]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> + >> + /* SVC Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET= (8), >> + USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSE= T(11), >> + USR_REG_OFFSET(12), >> + REG_OFFSET(svc_regs[0]), /* r13 */ >> + REG_OFFSET(svc_regs[1]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> + >> + /* ABT Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET= (8), >> + USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSE= T(11), >> + USR_REG_OFFSET(12), >> + REG_OFFSET(abt_regs[0]), /* r13 */ >> + REG_OFFSET(abt_regs[1]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> + >> + /* UND Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET= (8), >> + USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSE= T(11), >> + USR_REG_OFFSET(12), >> + REG_OFFSET(und_regs[0]), /* r13 */ >> + REG_OFFSET(und_regs[1]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> + >> + /* USR Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET= (8), >> + USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSE= T(11), >> + USR_REG_OFFSET(12), >> + REG_OFFSET(usr_regs[13]), /* r13 */ >> + REG_OFFSET(usr_regs[14]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> + >> + /* SYS Registers */ >> + { >> + USR_REG_OFFSET(0), USR_REG_OFFSET(1), USR_REG_OFFSET= (2), >> + USR_REG_OFFSET(3), USR_REG_OFFSET(4), USR_REG_OFFSET= (5), >> + USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET= (8), >> + USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSE= T(11), >> + USR_REG_OFFSET(12), >> + REG_OFFSET(usr_regs[13]), /* r13 */ >> + REG_OFFSET(usr_regs[14]), /* r14 */ >> + REG_OFFSET(pc) /* r15 */ >> + }, >> +}; >> + >> +/* >> + * Return a pointer to the register number valid in the specified m= ode of >> + * the virtual CPU. >> + */ >> +u32 *vcpu_reg_mode(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode) >> +{ >> + u32 *reg_array =3D (u32 *)&vcpu->arch.regs; >> + >> + BUG_ON(reg_num > 15); >> + BUG_ON(mode > MODE_SYS); > > Again, BUG_ON seems a bit OTT here. Also, this is where the mode =3D>= idx > calculation should happen. > >> + return reg_array + vcpu_reg_offsets[mode][reg_num]; >> +} >> diff --git a/arch/arm/kvm/exports.c b/arch/arm/kvm/exports.c >> new file mode 100644 >> index 0000000..3e38c95 >> --- /dev/null >> +++ b/arch/arm/kvm/exports.c >> @@ -0,0 +1,21 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#include >> + >> +EXPORT_SYMBOL_GPL(smp_send_reschedule); > > Erm... > > We already have arch/arm/kernel/armksyms.c for exports -- please use = that. > However, exporting such low-level operations sounds like a bad idea. = How > realistic is kvm-as-a-module on ARM anyway? > at this point it's broken, so I'll just remove this and leave this for = a=20 fun project for some poor soul at some point if anyone ever needs half=20 the code outside the kernel as a module (the other half needs to be=20 compiled in anyway) >> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c >> new file mode 100644 >> index 0000000..19a5389 >> --- /dev/null >> +++ b/arch/arm/kvm/guest.c >> @@ -0,0 +1,211 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define VM_STAT(x) { #x, offsetof(struct kvm, stat.x), KVM_STAT_VM = } >> +#define VCPU_STAT(x) { #x, offsetof(struct kvm_vcpu, stat.x), KVM_S= TAT_VCPU } >> + >> +struct kvm_stats_debugfs_item debugfs_entries[] =3D { >> + { NULL } >> +}; >> + >> +int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) >> +{ >> + return 0; >> +} >> + >> +static u64 core_reg_offset_from_id(u64 id) >> +{ >> + return id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_RE= G_ARM_CORE); >> +} >> + >> +static int get_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one= _reg *reg) >> +{ >> + u32 __user *uaddr =3D (u32 __user *)(long)reg->addr; >> + struct kvm_regs *regs =3D &vcpu->arch.regs; >> + u64 off; >> + >> + if (KVM_REG_SIZE(reg->id) !=3D 4) >> + return -ENOENT; >> + >> + /* Our ID is an index into the kvm_regs struct. */ >> + off =3D core_reg_offset_from_id(reg->id); >> + if (off >=3D sizeof(*regs) / KVM_REG_SIZE(reg->id)) >> + return -ENOENT; >> + >> + return put_user(((u32 *)regs)[off], uaddr); >> +} >> + >> +static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one= _reg *reg) >> +{ >> + u32 __user *uaddr =3D (u32 __user *)(long)reg->addr; >> + struct kvm_regs *regs =3D &vcpu->arch.regs; >> + u64 off, val; >> + >> + if (KVM_REG_SIZE(reg->id) !=3D 4) >> + return -ENOENT; >> + >> + /* Our ID is an index into the kvm_regs struct. */ >> + off =3D core_reg_offset_from_id(reg->id); >> + if (off >=3D sizeof(*regs) / KVM_REG_SIZE(reg->id)) >> + return -ENOENT; >> + >> + if (get_user(val, uaddr) !=3D 0) >> + return -EFAULT; >> + >> + if (off =3D=3D KVM_REG_ARM_CORE_REG(cpsr)) { >> + if (__vcpu_mode(val) =3D=3D 0xf) >> + return -EINVAL; >> + } >> + >> + ((u32 *)regs)[off] =3D val; >> + return 0; >> +} >> + >> +int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_= regs *regs) >> +{ >> + return -EINVAL; >> +} >> + >> +int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_= regs *regs) >> +{ >> + return -EINVAL; >> +} > > Again, all looks like this should be implemented using regsets from w= hat I > can tell. > this API has been discussed to death on the KVM lists, and we can of=20 course revive that if the regset makes it nicer - I'd prefer getting=20 this upstream the way that it is now though, where GET_REG / SET_REG=20 seems to be the way forward from a KVM perspective. >> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c >> new file mode 100644 >> index 0000000..290a13d >> --- /dev/null >> +++ b/arch/arm/kvm/reset.c >> @@ -0,0 +1,74 @@ >> +/* >> + * Copyright (C) 2012 - Virtual Open Systems and Columbia Universit= y >> + * Author: Christoffer Dall >> + * >> + * This program is free software; you can redistribute it and/or mo= dify >> + * it under the terms of the GNU General Public License, version 2,= as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public Licens= e >> + * along with this program; if not, write to the Free Software >> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1= 301, USA. >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/******************************************************************= ************ >> + * Cortex-A15 Reset Values >> + */ >> + >> +static const int a15_max_cpu_idx =3D 3; >> + >> +static struct kvm_regs a15_regs_reset =3D { >> + .cpsr =3D SVC_MODE | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT, >> +}; >> + >> + >> +/******************************************************************= ************* >> + * Exported reset function >> + */ >> + >> +/** >> + * kvm_reset_vcpu - sets core registers and cp15 registers to reset= value >> + * @vcpu: The VCPU pointer >> + * >> + * This function finds the right table above and sets the registers= on the >> + * virtual CPU struct to their architectually defined reset values. >> + */ >> +int kvm_reset_vcpu(struct kvm_vcpu *vcpu) >> +{ >> + struct kvm_regs *cpu_reset; >> + >> + switch (vcpu->arch.target) { >> + case KVM_ARM_TARGET_CORTEX_A15: >> + if (vcpu->vcpu_id > a15_max_cpu_idx) >> + return -EINVAL; >> + cpu_reset =3D &a15_regs_reset; >> + vcpu->arch.midr =3D read_cpuid_id(); >> + break; >> + default: >> + return -ENODEV; >> + } >> + >> + /* Reset core registers */ >> + memcpy(&vcpu->arch.regs, cpu_reset, sizeof(vcpu->arch.regs))= ; >> + >> + /* Reset CP15 registers */ >> + kvm_reset_coprocs(vcpu); >> + >> + return 0; >> +} > > This is a nice way to plug in new CPUs but the way the rest of the co= de is > currently written, all the ARMv7 and Cortex-A15 code is merged togeth= er. I > *strongly* suggest you isolate this from the start, as it will help y= ou see > what is architected and what is implementation-specific. > not entirely sure what you mean. You want a separate coproc.c file for=20 Cortex-A15 specific stuff like coproc_a15.c? Thanks a bunch for the review! -Christoffer