On Thu, Jul 12, 2012 at 7:54 AM, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> This keeps the information in one place, but of course it means we
> have entries in the register table which are never trapped.
>
> Minor notes:
> 1) We reserve a value of 0 as an invalid cp15 offset, to catch bugs in our
> table, at cost of 4 bytes per vcpu.
>
> 2) Added comments on the table indicating how we handle each register, for
> simplicity of understanding.
>
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> ---
> arch/arm/include/asm/kvm_host.h | 4 +-
> arch/arm/kvm/coproc.c | 152 ++++++++++++++++++++++++++++++++++++++-
> arch/arm/kvm/reset.c | 64 +----------------
> 3 files changed, 153 insertions(+), 67 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 73b6fa3..af2a6df 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -38,6 +38,7 @@ 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 */
> @@ -79,8 +80,9 @@ struct kvm_vcpu_regs {
> u32 cpsr; /* The guest CPSR */
> } __packed;
>
> +/* 0 is reserved as an invalid value. */
> enum cp15_regs {
> - c0_MIDR, /* Main ID Register */
> + c0_MIDR=1, /* Main ID Register */
> c0_MPIDR, /* MultiProcessor ID Register */
> c1_SCTLR, /* System Control Register */
> c1_ACTLR, /* Auxilliary Control Register */
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 4f06dd2..cfa2b32 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -20,6 +20,7 @@
> #include <asm/kvm_host.h>
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_coproc.h>
> +#include <asm/cputype.h>
> #include <trace/events/kvm.h>
>
> #include "trace.h"
> @@ -40,6 +41,7 @@ struct coproc_params {
> };
>
> struct coproc_reg {
> + /* MRC/MCR/MRRC/MCRR instruction which accesses it. */
> unsigned long CRn;
> unsigned long CRm;
> unsigned long Op1;
> @@ -47,9 +49,19 @@ struct coproc_reg {
>
> bool is_64;
>
> + /* Trapped access from guest, if non-NULL. */
> bool (*access)(struct kvm_vcpu *,
> const struct coproc_params *,
> const struct coproc_reg *);
> +
> + /* Initialization for vcpu. */
> + void (*reset)(struct kvm_vcpu *, const struct coproc_reg *);
> +
> + /* Offset of register in vcpu->arch.cp15[], or 0. */
0 means hide from user space or not backed by vcpu->arch entry for
whatever other reason, right? perhaps worth including in the comment,
the "or 0" is not very helpful
> + enum cp15_regs reg;
> +
> + /* Value (usually reset value) */
> + u64 val;
> };
>
> static void print_cp_instr(const struct coproc_params *p)
> @@ -234,6 +246,43 @@ static bool pm_fake(struct kvm_vcpu *vcpu,
> #define access_pmintenset pm_fake
> #define access_pmintenclr pm_fake
>
> +/* Reset functions */
> +static void reset_unknown(struct kvm_vcpu *vcpu, const struct coproc_reg *r)
> +{
> + BUG_ON(!r->reg);
> + BUG_ON(r->reg >= ARRAY_SIZE(vcpu->arch.cp15));
> + vcpu->arch.cp15[r->reg] = 0xdecafbad;
> +}
> +
> +static void reset_val(struct kvm_vcpu *vcpu, const struct coproc_reg *r)
> +{
> + BUG_ON(!r->reg);
> + BUG_ON(r->reg >= ARRAY_SIZE(vcpu->arch.cp15));
> + vcpu->arch.cp15[r->reg] = r->val;
> +}
> +
> +static void reset_unknown64(struct kvm_vcpu *vcpu, const struct coproc_reg *r)
> +{
> + BUG_ON(!r->reg);
> + BUG_ON(r->reg + 1 >= ARRAY_SIZE(vcpu->arch.cp15));
> +
> + vcpu->arch.cp15[r->reg] = 0xdecafbad;
> + vcpu->arch.cp15[r->reg+1] = 0xd0c0ffee;
> +}
> +
> +static void reset_mpidr(struct kvm_vcpu *vcpu, const struct coproc_reg *r)
> +{
> + /*
> + * Compute guest MPIDR:
> + * (Even if we present only one VCPU to the guest on an SMP
> + * host we don't set the U bit in the MPIDR, or vice versa, as
> + * revealing the underlying hardware properties is likely to
> + * be the best choice).
> + */
> + vcpu->arch.cp15[c0_MPIDR] = (read_cpuid_mpidr() & ~MPIDR_CPUID)
> + | (vcpu->vcpu_id & MPIDR_CPUID);
> +}
> +
> #define CRn(_x) .CRn = _x
> #define CRm(_x) .CRm = _x
> #define Op1(_x) .Op1 = _x
> @@ -243,6 +292,33 @@ static bool pm_fake(struct kvm_vcpu *vcpu,
>
> /* Architected CP15 registers. */
> static const struct coproc_reg cp15_regs[] = {
> + /* TTBCR: swapped by interrupt.S. */
> + { CRn( 2), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_val, c2_TTBCR, 0x00000000 },
why the very long version of 0 ?
> +
> + /* TTBR0/TTBR1: swapped by interrupt.S. */
> + { CRm( 2), Op1( 0), is64, NULL, reset_unknown64, c2_TTBR0 },
> + { CRm( 2), Op1( 1), is64, NULL, reset_unknown64, c2_TTBR1 },
> +
> + /* DACR: swapped by interrupt.S. */
> + { CRn( 3), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_unknown, c3_DACR },
> +
> + /* DFSR/IFSR/ADFSR/AIFSR: swapped by interrupt.S. */
> + { CRn( 5), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_unknown, c5_DFSR },
> + { CRn( 5), CRm( 0), Op1( 0), Op2( 1), is32,
> + NULL, reset_unknown, c5_IFSR },
> + { CRn( 5), CRm( 1), Op1( 0), Op2( 0), is32,
> + NULL, reset_unknown, c5_ADFSR },
> + { CRn( 5), CRm( 1), Op1( 0), Op2( 1), is32,
> + NULL, reset_unknown, c5_AIFSR },
> +
> + /* DFAR/IFAR: swapped by interrupt.S. */
> + { CRn( 6), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_unknown, c6_DFAR },
> + { CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
> + NULL, reset_unknown, c6_IFAR },
> /*
> * DC{C,I,CI}SW operations:
> */
> @@ -265,14 +341,46 @@ static const struct coproc_reg cp15_regs[] = {
> { CRn( 9), CRm(14), Op1( 0), Op2( 0), is32, access_pmuserenr},
> { CRn( 9), CRm(14), Op1( 0), Op2( 1), is32, access_pmintenset},
> { CRn( 9), CRm(14), Op1( 0), Op2( 2), is32, access_pmintenclr},
> +
> + /* PRRR/NMRR (aka MAIR0/MAIR1): swapped by interrupt.S. */
> + { CRn(10), CRm( 2), Op1( 0), Op2( 0), is32,
> + NULL, reset_unknown, c10_PRRR},
> + { CRn(10), CRm( 2), Op1( 0), Op2( 1), is32,
> + NULL, reset_unknown, c10_NMRR},
> +
> + /* VBAR: swapped by interrupt.S. */
> + { CRn(12), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_val, c12_VBAR, 0x00000000 },
> +
> + /* CONTEXTIDR/TPIDRURW/TPIDRURO/TPIDRPRW: swapped by interrupt.S. */
> + { CRn(13), CRm( 0), Op1( 0), Op2( 1), is32,
> + NULL, reset_val, c13_CID, 0x00000000 },
> + { CRn(13), CRm( 0), Op1( 0), Op2( 2), is32,
> + NULL, reset_unknown, c13_TID_URW },
> + { CRn(13), CRm( 0), Op1( 0), Op2( 3), is32,
> + NULL, reset_unknown, c13_TID_URO },
> + { CRn(13), CRm( 0), Op1( 0), Op2( 4), is32,
> + NULL, reset_unknown, c13_TID_PRIV },
> };
>
> /* A15-specific CP15 registers. */
> static const struct coproc_reg cp15_cortex_a15_regs[] = {
> - /*
> - * ACTRL access:
> - */
> + /* MIDR: we use VPIDR for guest access. */
> + { CRn( 0), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_val, c0_MIDR, 0x412FC0F0 },
> + /* MPIDR: we use VMPIDR for guest access. */
> + { CRn( 0), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_mpidr, c0_MPIDR },
> +
> + /* SCTLR: swapped by interrupt.S. */
> + { CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
> + NULL, reset_val, c1_SCTLR, 0x00C50078 },
> + /* ACTLR: trapped by HCR.TAC bit. */
> { CRn( 1), CRm( 0), Op1( 0), Op2( 1), is32, access_actlr},
> + /* CPACR: swapped by interrupt.S. */
> + { CRn( 1), CRm( 0), Op1( 0), Op2( 2), is32,
> + NULL, reset_val, c1_CPACR, 0x00000000 },
> +
> /*
> * L2CTLR access (guest wants to know #CPUs).
> */
> @@ -335,6 +443,9 @@ static int emulate_cp15(struct kvm_vcpu *vcpu,
> r = find_reg(params, cp15_regs, ARRAY_SIZE(cp15_regs));
>
> if (likely(r)) {
> + /* If we don't have an accessor, we should never get here! */
> + BUG_ON(!r->access);
> +
> if (likely(r->access(vcpu, params, r))) {
> /* Skip instruction, since it was emulated */
> int instr_len = ((vcpu->arch.hsr >> 25) & 1) ? 4 : 2;
> @@ -374,6 +485,41 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> return emulate_cp15(vcpu, ¶ms);
> }
>
> +static void reset_table(struct kvm_vcpu *vcpu,
> + const struct coproc_reg *table, size_t num)
> +{
> + unsigned long i;
> +
> + for (i = 0; i < num; i++)
> + if (table[i].reset)
> + table[i].reset(vcpu, &table[i]);
> +}
> +
> +/**
> + * kvm_reset_coprocs - sets 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.
> + */
> +void kvm_reset_coprocs(struct kvm_vcpu *vcpu)
> +{
> + size_t num;
> + const struct coproc_reg *table;
> +
> + /* Catch someone adding a register without putting in reset entry. */
> + memset(vcpu->arch.cp15, 0x42, sizeof(vcpu->arch.cp15));
> +
> + /* Generic chip reset first (so target could override). */
> + reset_table(vcpu, cp15_regs, ARRAY_SIZE(cp15_regs));
> +
> + table = get_target_table(vcpu->arch.target, &num);
> + reset_table(vcpu, table, num);
> +
> + for (num = 1; num < nr_cp15_regs; num++)
> + if (vcpu->arch.cp15[num] == 0x42424242)
> + panic("Didn't reset vcpu->arch.cp15[%zi]", num);
> +}
> /**
> * kvm_handle_cp15_32 -- handles a mrc/mcr trap on a guest CP15 access
> * @vcpu: The VCPU pointer
> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c
> index 4262d14..8eb0a56 100644
> --- a/arch/arm/kvm/reset.c
> +++ b/arch/arm/kvm/reset.c
> @@ -23,14 +23,8 @@
>
> #include <asm/unified.h>
> #include <asm/ptrace.h>
> -#include <asm/cputype.h>
> #include <asm/kvm_arm.h>
>
> -#define CT_ASSERT(expr, name) extern char name[(expr) ? 1 : -1]
> -#define CP15_REGS_ASSERT(_array, _name) \
> - CT_ASSERT((sizeof(_array) / sizeof(_array[0])) == nr_cp15_regs, _name)
> -#define UNKNOWN 0xdecafbad
> -
> /******************************************************************************
> * Cortex-A15 Register Reset Values
> */
> @@ -41,47 +35,6 @@ static struct kvm_vcpu_regs a15_regs_reset = {
> .cpsr = SVC_MODE | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
> };
>
> -static u32 a15_cp15_regs_reset[][2] = {
> - { c0_MIDR, 0x412FC0F0 },
> - { c0_MPIDR, 0x00000000 }, /* see kvm_arch_vcpu_init */
> - { c1_SCTLR, 0x00C50078 },
> - { c1_ACTLR, 0x00000000 },
> - { c1_CPACR, 0x00000000 },
> - { c2_TTBR0, UNKNOWN },
> - { c2_TTBR0_high, UNKNOWN },
> - { c2_TTBR1, UNKNOWN },
> - { c2_TTBR1_high, UNKNOWN },
> - { c2_TTBCR, 0x00000000 },
> - { c3_DACR, UNKNOWN },
> - { c5_DFSR, UNKNOWN },
> - { c5_IFSR, UNKNOWN },
> - { c5_ADFSR, UNKNOWN },
> - { c5_AIFSR, UNKNOWN },
> - { c6_DFAR, UNKNOWN },
> - { c6_IFAR, UNKNOWN },
> - { c10_PRRR, 0x00098AA4 },
> - { c10_NMRR, 0x44E048E0 },
> - { c12_VBAR, 0x00000000 },
> - { c13_CID, 0x00000000 },
> - { c13_TID_URW, UNKNOWN },
> - { c13_TID_URO, UNKNOWN },
> - { c13_TID_PRIV, UNKNOWN },
> -};
> -CP15_REGS_ASSERT(a15_cp15_regs_reset, a15_cp15_regs_reset_init);
> -
> -static void a15_reset_vcpu(struct kvm_vcpu *vcpu)
> -{
> - /*
> - * Compute guest MPIDR:
> - * (Even if we present only one VCPU to the guest on an SMP
> - * host we don't set the U bit in the MPIDR, or vice versa, as
> - * revealing the underlying hardware properties is likely to
> - * be the best choice).
> - */
> - vcpu->arch.cp15[c0_MPIDR] = (read_cpuid_mpidr() & ~MPIDR_CPUID)
> - | (vcpu->vcpu_id & MPIDR_CPUID);
> -}
> -
>
> /*******************************************************************************
> * Exported reset function
> @@ -96,18 +49,13 @@ static void a15_reset_vcpu(struct kvm_vcpu *vcpu)
> */
> int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> {
> - unsigned int i;
> struct kvm_vcpu_regs *cpu_reset;
> - u32 (*cp15_reset)[2];
> - void (*cpu_reset_vcpu)(struct kvm_vcpu *vcpu);
>
> switch (vcpu->arch.target) {
> case KVM_ARM_TARGET_CORTEX_A15:
> if (vcpu->vcpu_id > a15_max_cpu_idx)
> return -EINVAL;
> cpu_reset = &a15_regs_reset;
> - cp15_reset = a15_cp15_regs_reset;
> - cpu_reset_vcpu = a15_reset_vcpu;
> break;
> default:
> return -ENODEV;
> @@ -117,17 +65,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> memcpy(&vcpu->arch.regs, cpu_reset, sizeof(vcpu->arch.regs));
>
> /* Reset CP15 registers */
> - for (i = 0; i < nr_cp15_regs; i++) {
> - if (cp15_reset[i][0] != i) {
> - kvm_err("CP15 field %d is %d, expected %d\n",
> - i, cp15_reset[i][0], i);
> - return -ENXIO;
> - }
> - vcpu->arch.cp15[i] = cp15_reset[i][1];
> - }
> -
> - /* Physical CPU specific runtime reset operations */
> - cpu_reset_vcpu(vcpu);
> + kvm_reset_coprocs(vcpu);
>
> return 0;
> }
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
