On Thu, 23 Feb 2012 02:32:26 -0500, Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> From: Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx>
>
> Targets KVM support for Cortex A-15 processors.
>
> Contains no real functionality but all the framework components,
> make files, header files and some tracing functionality.
>
> “Nothing to see here. Move along, move along..."
>
> Most functionality is in arch/arm/kvm/* or arch/arm/include/asm/kvm_*.h.
>
> Signed-off-by: Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> arch/arm/Kconfig | 2
> arch/arm/Makefile | 1
> arch/arm/include/asm/kvm.h | 72 ++++++++++
> arch/arm/include/asm/kvm_asm.h | 28 ++++
> arch/arm/include/asm/kvm_emulate.h | 92 +++++++++++++
> arch/arm/include/asm/kvm_host.h | 116 ++++++++++++++++
> arch/arm/include/asm/kvm_para.h | 9 +
> arch/arm/include/asm/unified.h | 12 ++
> arch/arm/kvm/Kconfig | 45 ++++++
> arch/arm/kvm/Makefile | 17 ++
> arch/arm/kvm/arm.c | 256 ++++++++++++++++++++++++++++++++++++
> arch/arm/kvm/emulate.c | 121 +++++++++++++++++
> arch/arm/kvm/exports.c | 16 ++
> arch/arm/kvm/guest.c | 148 +++++++++++++++++++++
> arch/arm/kvm/init.S | 17 ++
> arch/arm/kvm/interrupts.S | 17 ++
> arch/arm/kvm/mmu.c | 15 ++
> arch/arm/kvm/trace.h | 52 +++++++
> arch/arm/mm/Kconfig | 8 +
> 19 files changed, 1044 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/kvm.h
> create mode 100644 arch/arm/include/asm/kvm_asm.h
> create mode 100644 arch/arm/include/asm/kvm_emulate.h
> create mode 100644 arch/arm/include/asm/kvm_host.h
> create mode 100644 arch/arm/include/asm/kvm_para.h
> create mode 100644 arch/arm/kvm/Kconfig
> create mode 100644 arch/arm/kvm/Makefile
> create mode 100644 arch/arm/kvm/arm.c
> create mode 100644 arch/arm/kvm/emulate.c
> create mode 100644 arch/arm/kvm/exports.c
> create mode 100644 arch/arm/kvm/guest.c
> create mode 100644 arch/arm/kvm/init.S
> create mode 100644 arch/arm/kvm/interrupts.S
> create mode 100644 arch/arm/kvm/mmu.c
> create mode 100644 arch/arm/kvm/trace.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e12bc34..81aa08f 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -2263,3 +2263,5 @@ source "security/Kconfig"
> source "crypto/Kconfig"
>
> source "lib/Kconfig"
> +
> +source "arch/arm/kvm/Kconfig"
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 40319d9..eca44e0 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -253,6 +253,7 @@ core-$(CONFIG_VFP) += arch/arm/vfp/
>
> # If we have a machine-specific directory, then include it in the build.
> core-y += arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
> +core-y += arch/arm/kvm/
> core-y += $(machdirs) $(platdirs)
>
> drivers-$(CONFIG_OPROFILE) += arch/arm/oprofile/
> diff --git a/arch/arm/include/asm/kvm.h b/arch/arm/include/asm/kvm.h
> new file mode 100644
> index 0000000..544cb2a
> --- /dev/null
> +++ b/arch/arm/include/asm/kvm.h
> @@ -0,0 +1,72 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * 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 License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> + *
> + */
> +
> +#ifndef __ARM_KVM_H__
> +#define __ARM_KVM_H__
> +
> +#include <asm/types.h>
> +
> +#define __KVM_HAVE_GUEST_DEBUG
> +
> +/*
> + * 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 mode bits!
> + */
> +enum vcpu_modes {
Nitpick: s/vcpu_modes/vcpu_mode/ ?
...
> +static inline unsigned char vcpu_mode(struct kvm_vcpu *vcpu)
static inline enum vcpu_mode vcpu_mode(struct kvm_vcpu *vcpu)...
> +{
> + u8 modes_table[16] = {
> + 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 */
> +
> + BUG_ON(modes_table[vcpu->arch.regs.cpsr & 0xf] == 0xf);
> + return modes_table[vcpu->arch.regs.cpsr & 0xf];
> +}
Like much of your code, I find this far too readable. How about:
enum vcpu_mode {
MODE_USR,
MODE_FIQ,
MODE_IRQ,
MODE_SVC,
MODE_ABT,
MODE_UND,
MODE_SYS
};
static inline enum vcpu_mode vcpu_mode(const struct kvm_vcpu *vcpu)
{
u8 mode = vcpu->arch.regs.cpsr & 0xf;
return (mode & 3) + (mode >> 2);
}
> +/*
> + * Return the SPSR for the specified mode of the virtual CPU.
> + */
> +static inline u32 *kvm_vcpu_spsr(struct kvm_vcpu *vcpu, u32 mode)
s/u32/enum vcpu_mode/
> +{
> + switch (mode) {
> + case MODE_SVC:
> + return &vcpu->arch.regs.svc_regs[2];
> + case MODE_ABT:
> + return &vcpu->arch.regs.svc_regs[2];
> + case MODE_UND:
> + return &vcpu->arch.regs.svc_regs[2];
> + case MODE_IRQ:
> + return &vcpu->arch.regs.svc_regs[2];
This reminds me: shouldn't these be:
case MODE_ABT:
return &vcpu->arch.regs.abt_regs[2];
case MODE_UND:
return &vcpu->arch.regs.und_regs[2];
case MODE_IRQ:
return &vcpu->arch.regs.irq_regs[2];
?
> +static inline u32 *vcpu_cpsr(struct kvm_vcpu *vcpu)
> +{
> + return &vcpu->arch.regs.cpsr;
> +}
In general, do you want accessors like this to all regs? I'd like to
know, because injecting an undefined insn trap in the guest needs to
access the pc...
> +struct kvm_vcpu;
> +u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
s/u32/enum vcpu_mode/
> +config KVM
> + tristate "Kernel-based Virtual Machine (KVM) support"
> + select PREEMPT_NOTIFIERS
> + select ANON_INODES
> + select KVM_ARM_HOST
> + select KVM_MMIO
> + depends on CPU_V7 && ARM_VIRT_EXT
> + ---help---
> + Support hosting virtualized guest machines. You will also
> + need to select one or more of the processor modules below.
> +
> + This module provides access to the hardware capabilities through
> + a character device node named /dev/kvm.
> +
> + If unsure, say N.
> +
> +config KVM_ARM_HOST
> + bool "KVM host support for ARM cpus."
> + depends on KVM
> + depends on MMU
> + depends on CPU_V7 && ARM_VIRT_EXT
> + ---help---
> + Provides host support for ARM processors.
I don't think KVM should select KVM_ARM_HOST (I assume there'll be a
KVM_ARM_GUEST one day). Otherwise remove the prompt after 'bool' in
KVM_ARM_HOST.
> +/*
> + * Return a pointer to the register number valid in the specified mode of
> + * the virtual CPU.
> + */
> +u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode)
> +{
> + BUG_ON(reg_num > 15);
> + BUG_ON(mode > MODE_SYS);
> +
> + return (u32 *)((void *)&vcpu->arch + vcpu_reg_offsets[mode][reg_num]);
> +}
Hmm.... I tried to neaten this (untested). Not sure if it's a win, your
call:
commit 7f99177bac6173968925f7329333e5bdf752be67
Author: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Date: Fri Feb 24 13:37:44 2012 +1030
ARM: KVM: neaten offset calculations.
If we do this the C-correct way, we use fewer casts and it's a bit clearer.
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 73aece6..6f5b08f 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -84,7 +84,11 @@ enum cp15_regs {
};
struct kvm_vcpu_arch {
- struct kvm_vcpu_regs regs;
+ /* We sometimes access these as an array for simplicity. */
+ union {
+ struct kvm_vcpu_regs regs;
+ u32 reg_array[sizeof(struct kvm_vcpu_regs) / sizeof(u32)];
+ };
/* System control coprocessor (cp15) */
u32 cp15[nr_cp15_regs];
diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
index 4f5f2de..108db38 100644
--- a/arch/arm/kvm/emulate.c
+++ b/arch/arm/kvm/emulate.c
@@ -22,8 +22,10 @@
#include "trace.h"
-#define USR_REG_OFFSET(_reg) \
- offsetof(struct kvm_vcpu_arch, regs.usr_regs[_reg])
+#define REG_OFFSET(_reg) \
+ (offsetof(struct kvm_vcpu_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] = {
/* FIQ Registers */
@@ -31,14 +33,14 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
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),
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[1]), /* r8 */
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[1]), /* r9 */
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[2]), /* r10 */
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[3]), /* r11 */
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[4]), /* r12 */
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[5]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.fiq_regs[6]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ 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 */
@@ -48,9 +50,9 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET(8),
USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSET(11),
USR_REG_OFFSET(12),
- offsetof(struct kvm_vcpu_arch, regs.irq_regs[0]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.irq_regs[1]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ REG_OFFSET(irq_regs[0]), /* r13 */
+ REG_OFFSET(irq_regs[1]), /* r14 */
+ REG_OFFSET(pc) /* r15 */
},
/* SVC Registers */
@@ -60,9 +62,9 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET(8),
USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSET(11),
USR_REG_OFFSET(12),
- offsetof(struct kvm_vcpu_arch, regs.svc_regs[0]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.svc_regs[1]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ REG_OFFSET(svc_regs[0]), /* r13 */
+ REG_OFFSET(svc_regs[1]), /* r14 */
+ REG_OFFSET(pc) /* r15 */
},
/* ABT Registers */
@@ -72,9 +74,9 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET(8),
USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSET(11),
USR_REG_OFFSET(12),
- offsetof(struct kvm_vcpu_arch, regs.abt_regs[0]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.abt_regs[1]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ REG_OFFSET(abt_regs[0]), /* r13 */
+ REG_OFFSET(abt_regs[1]), /* r14 */
+ REG_OFFSET(pc) /* r15 */
},
/* UND Registers */
@@ -84,9 +86,9 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET(8),
USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSET(11),
USR_REG_OFFSET(12),
- offsetof(struct kvm_vcpu_arch, regs.und_regs[0]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.und_regs[1]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ REG_OFFSET(und_regs[0]), /* r13 */
+ REG_OFFSET(und_regs[1]), /* r14 */
+ REG_OFFSET(pc) /* r15 */
},
/* USR Registers */
@@ -96,9 +98,9 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET(8),
USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSET(11),
USR_REG_OFFSET(12),
- offsetof(struct kvm_vcpu_arch, regs.usr_regs[13]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.usr_regs[14]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ REG_OFFSET(usr_regs[13]), /* r13 */
+ REG_OFFSET(usr_regs[14]), /* r14 */
+ REG_OFFSET(pc) /* r15 */
},
/* SYS Registers */
@@ -108,9 +110,9 @@ static const unsigned long vcpu_reg_offsets[MODE_SYS + 1][16] = {
USR_REG_OFFSET(6), USR_REG_OFFSET(7), USR_REG_OFFSET(8),
USR_REG_OFFSET(9), USR_REG_OFFSET(10), USR_REG_OFFSET(11),
USR_REG_OFFSET(12),
- offsetof(struct kvm_vcpu_arch, regs.usr_regs[13]), /* r13 */
- offsetof(struct kvm_vcpu_arch, regs.usr_regs[14]), /* r14 */
- offsetof(struct kvm_vcpu_arch, regs.pc) /* r15 */
+ REG_OFFSET(usr_regs[13]), /* r13 */
+ REG_OFFSET(usr_regs[14]), /* r14 */
+ REG_OFFSET(pc) /* r15 */
},
};
@@ -123,7 +125,7 @@ u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode)
BUG_ON(reg_num > 15);
BUG_ON(mode > MODE_SYS);
- return (u32 *)((void *)&vcpu->arch + vcpu_reg_offsets[mode][reg_num]);
+ return &vcpu->arch.reg_array[vcpu_reg_offsets[mode][reg_num]];
}
/******************************************************************************
> +config ARM_VIRT_EXT
> + bool "Support for ARM Virtualization Extensions"
> + depends on ARM_LPAE
> + help
> + Say Y if you have an ARMv7 processor supporting the ARM hardware
> + Virtualization extensions. KVM depends on this feature and will
> + not run without it being selected.
> +
> config ARCH_PHYS_ADDR_T_64BIT
> def_bool ARM_LPAE
If you say Y here, you won't boot on a machine which hasn't got VIRT,
right? That's changing, of course, with Dave's boot changes, but worth
mentioning either way I think.
Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
