From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/9] arm64: KVM: common infrastructure for handling AArch32 CP14/CP15
Date: Sun, 25 May 2014 16:34:59 +0100 [thread overview]
Message-ID: <20140525153459.GE3866@lvm> (raw)
In-Reply-To: <1400604945-25247-5-git-send-email-marc.zyngier@arm.com>
On Tue, May 20, 2014 at 05:55:40PM +0100, Marc Zyngier wrote:
> As we're about to trap a bunch of CP14 registers, let's rework
> the CP15 handling so it can be generalized and work with multiple
> tables.
>
> Reviewed-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/include/asm/kvm_asm.h | 2 +-
> arch/arm64/include/asm/kvm_coproc.h | 3 +-
> arch/arm64/include/asm/kvm_host.h | 9 ++-
> arch/arm64/kvm/handle_exit.c | 4 +-
> arch/arm64/kvm/sys_regs.c | 121 +++++++++++++++++++++++++++++-------
> 5 files changed, 111 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index e6b159a..12f9dd7 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -93,7 +93,7 @@
> #define c10_AMAIR0 (AMAIR_EL1 * 2) /* Aux Memory Attr Indirection Reg */
> #define c10_AMAIR1 (c10_AMAIR0 + 1)/* Aux Memory Attr Indirection Reg */
> #define c14_CNTKCTL (CNTKCTL_EL1 * 2) /* Timer Control Register (PL1) */
> -#define NR_CP15_REGS (NR_SYS_REGS * 2)
> +#define NR_COPRO_REGS (NR_SYS_REGS * 2)
>
> #define ARM_EXCEPTION_IRQ 0
> #define ARM_EXCEPTION_TRAP 1
> diff --git a/arch/arm64/include/asm/kvm_coproc.h b/arch/arm64/include/asm/kvm_coproc.h
> index 9a59301..0b52377 100644
> --- a/arch/arm64/include/asm/kvm_coproc.h
> +++ b/arch/arm64/include/asm/kvm_coproc.h
> @@ -39,7 +39,8 @@ void kvm_register_target_sys_reg_table(unsigned int target,
> struct kvm_sys_reg_target_table *table);
>
> int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu, struct kvm_run *run);
> -int kvm_handle_cp14_access(struct kvm_vcpu *vcpu, struct kvm_run *run);
> +int kvm_handle_cp14_32(struct kvm_vcpu *vcpu, struct kvm_run *run);
> +int kvm_handle_cp14_64(struct kvm_vcpu *vcpu, struct kvm_run *run);
> int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run);
> int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run);
> int kvm_handle_sys_reg(struct kvm_vcpu *vcpu, struct kvm_run *run);
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 4737961..31cff7a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -86,7 +86,7 @@ struct kvm_cpu_context {
> struct kvm_regs gp_regs;
> union {
> u64 sys_regs[NR_SYS_REGS];
> - u32 cp15[NR_CP15_REGS];
> + u32 copro[NR_COPRO_REGS];
> };
> };
>
> @@ -141,7 +141,12 @@ struct kvm_vcpu_arch {
>
> #define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
> #define vcpu_sys_reg(v,r) ((v)->arch.ctxt.sys_regs[(r)])
> -#define vcpu_cp15(v,r) ((v)->arch.ctxt.cp15[(r)])
> +/*
> + * CP14 and CP15 live in the same array, as they are backed by the
> + * same system registers.
> + */
> +#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r)])
> +#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r)])
>
> struct kvm_vm_stat {
> u32 remote_tlb_flush;
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 7bc41ea..f0ca49f 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -69,9 +69,9 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_EL2_EC_WFI] = kvm_handle_wfx,
> [ESR_EL2_EC_CP15_32] = kvm_handle_cp15_32,
> [ESR_EL2_EC_CP15_64] = kvm_handle_cp15_64,
> - [ESR_EL2_EC_CP14_MR] = kvm_handle_cp14_access,
> + [ESR_EL2_EC_CP14_MR] = kvm_handle_cp14_32,
> [ESR_EL2_EC_CP14_LS] = kvm_handle_cp14_load_store,
> - [ESR_EL2_EC_CP14_64] = kvm_handle_cp14_access,
> + [ESR_EL2_EC_CP14_64] = kvm_handle_cp14_64,
> [ESR_EL2_EC_HVC32] = handle_hvc,
> [ESR_EL2_EC_SMC32] = handle_smc,
> [ESR_EL2_EC_HVC64] = handle_hvc,
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index d46a965..429e38c 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -474,6 +474,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> NULL, reset_val, FPEXC32_EL2, 0x70 },
> };
>
> +/* Trapped cp14 registers */
> +static const struct sys_reg_desc cp14_regs[] = {
> +};
> +
> /*
> * Trapped cp15 registers. TTBR0/TTBR1 get a double encoding,
> * depending on the way they are accessed (as a 32bit or a 64bit
> @@ -581,26 +585,19 @@ int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu, struct kvm_run *run)
> return 1;
> }
>
> -int kvm_handle_cp14_access(struct kvm_vcpu *vcpu, struct kvm_run *run)
> -{
> - kvm_inject_undefined(vcpu);
> - return 1;
> -}
> -
> -static void emulate_cp15(struct kvm_vcpu *vcpu,
> - const struct sys_reg_params *params)
document the return value please?
> +static int emulate_cp(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *params,
> + const struct sys_reg_desc *table,
> + size_t num)
> {
> - size_t num;
> - const struct sys_reg_desc *table, *r;
> + const struct sys_reg_desc *r;
>
> - table = get_target_table(vcpu->arch.target, false, &num);
> + if (!table)
> + return -1; /* Not handled */
>
> - /* Search target-specific then generic table. */
> r = find_reg(params, table, num);
> - if (!r)
> - r = find_reg(params, cp15_regs, ARRAY_SIZE(cp15_regs));
>
> - if (likely(r)) {
> + if (r) {
> /*
> * Not having an accessor means that we have
> * configured a trap that we don't know how to
This is making me think: Do we really want that BUG_ON? It's certainly
a KVM bug, but not something that compromises the host kernel state is
it? We can safely just kill the VM and exit with an internal error
could we not? Do we care?
Anyway, something we can fix independently.
> @@ -612,12 +609,37 @@ static void emulate_cp15(struct kvm_vcpu *vcpu,
> if (likely(r->access(vcpu, params, r))) {
> /* Skip instruction, since it was emulated */
> kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
> - return;
> }
> - /* If access function fails, it should complain. */
> +
> + /* Handled */
> + return 0;
> }
>
> - kvm_err("Unsupported guest CP15 access at: %08lx\n", *vcpu_pc(vcpu));
> + /* Not handled */
> + return -1;
> +}
> +
> +static void unhandled_cp_access(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *params)
> +{
> + u8 hsr_ec = kvm_vcpu_trap_get_class(vcpu);
> + int cp;
> +
> + switch(hsr_ec) {
> + case ESR_EL2_EC_CP15_32:
> + case ESR_EL2_EC_CP15_64:
> + cp = 15;
> + break;
> + case ESR_EL2_EC_CP14_MR:
> + case ESR_EL2_EC_CP14_64:
> + cp = 14;
> + break;
> + default:
> + WARN_ON((cp = -1));
> + }
> +
> + kvm_err("Unsupported guest CP%d access at: %08lx\n",
> + cp, *vcpu_pc(vcpu));
> print_sys_reg_instr(params);
> kvm_inject_undefined(vcpu);
> }
> @@ -627,7 +649,11 @@ static void emulate_cp15(struct kvm_vcpu *vcpu,
> * @vcpu: The VCPU pointer
> * @run: The kvm_run struct
> */
> -int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +static int kvm_handle_cp_64(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *global,
> + size_t nr_global,
> + const struct sys_reg_desc *target_specific,
> + size_t nr_specific)
> {
> struct sys_reg_params params;
> u32 hsr = kvm_vcpu_get_hsr(vcpu);
> @@ -656,8 +682,14 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> *vcpu_reg(vcpu, params.Rt) = val;
> }
>
> - emulate_cp15(vcpu, ¶ms);
> + if (!emulate_cp(vcpu, ¶ms, target_specific, nr_specific))
> + goto out;
> + if (!emulate_cp(vcpu, ¶ms, global, nr_global))
> + goto out;
> +
> + unhandled_cp_access(vcpu, ¶ms);
>
> +out:
> /* Do the opposite hack for the read side */
> if (!params.is_write) {
> u64 val = *vcpu_reg(vcpu, params.Rt);
> @@ -673,7 +705,11 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> * @vcpu: The VCPU pointer
> * @run: The kvm_run struct
> */
I think you forgot to modify the kdcos function name
> -int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +static int kvm_handle_cp_32(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *global,
> + size_t nr_global,
> + const struct sys_reg_desc *target_specific,
> + size_t nr_specific)
> {
> struct sys_reg_params params;
> u32 hsr = kvm_vcpu_get_hsr(vcpu);
> @@ -688,10 +724,51 @@ int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
> params.Op1 = (hsr >> 14) & 0x7;
> params.Op2 = (hsr >> 17) & 0x7;
>
> - emulate_cp15(vcpu, ¶ms);
> + if (!emulate_cp(vcpu, ¶ms, target_specific, nr_specific))
> + return 1;
> + if (!emulate_cp(vcpu, ¶ms, global, nr_global))
> + return 1;
> +
> + unhandled_cp_access(vcpu, ¶ms);
> return 1;
> }
>
> +int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + const struct sys_reg_desc *target_specific;
> + size_t num;
> +
> + target_specific = get_target_table(vcpu->arch.target, false, &num);
> + return kvm_handle_cp_64(vcpu,
> + cp15_regs, ARRAY_SIZE(cp15_regs),
> + target_specific, num);
> +}
> +
> +int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + const struct sys_reg_desc *target_specific;
> + size_t num;
> +
> + target_specific = get_target_table(vcpu->arch.target, false, &num);
> + return kvm_handle_cp_32(vcpu,
> + cp15_regs, ARRAY_SIZE(cp15_regs),
> + target_specific, num);
> +}
> +
> +int kvm_handle_cp14_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + return kvm_handle_cp_64(vcpu,
> + cp14_regs, ARRAY_SIZE(cp14_regs),
> + NULL, 0);
> +}
> +
> +int kvm_handle_cp14_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + return kvm_handle_cp_32(vcpu,
> + cp14_regs, ARRAY_SIZE(cp14_regs),
> + NULL, 0);
> +}
> +
> static int emulate_sys_reg(struct kvm_vcpu *vcpu,
> const struct sys_reg_params *params)
> {
> --
> 1.8.3.4
>
Besides the nit:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
next prev parent reply other threads:[~2014-05-25 15:34 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-20 16:55 [PATCH v2 0/9] arm64: KVM: debug infrastructure support Marc Zyngier
2014-05-20 16:55 ` [PATCH v2 1/9] arm64: KVM: rename pm_fake handler to trap_raz_wi Marc Zyngier
2014-05-25 15:34 ` Christoffer Dall
2014-05-20 16:55 ` [PATCH v2 2/9] arm64: move DBG_MDSCR_* to asm/debug-monitors.h Marc Zyngier
2014-05-25 15:34 ` Christoffer Dall
2014-05-20 16:55 ` [PATCH v2 3/9] arm64: KVM: add trap handlers for AArch64 debug registers Marc Zyngier
2014-05-25 15:34 ` Christoffer Dall
2014-05-28 10:27 ` Marc Zyngier
2014-05-20 16:55 ` [PATCH v2 4/9] arm64: KVM: common infrastructure for handling AArch32 CP14/CP15 Marc Zyngier
2014-05-25 15:34 ` Christoffer Dall [this message]
2014-05-28 10:34 ` Marc Zyngier
2014-05-20 16:55 ` [PATCH v2 5/9] arm64: KVM: use separate tables for AArch32 32 and 64bit traps Marc Zyngier
2014-05-25 15:35 ` Christoffer Dall
2014-05-20 16:55 ` [PATCH v2 6/9] arm64: KVM: check ordering of all system register tables Marc Zyngier
2014-05-25 15:35 ` Christoffer Dall
2014-05-20 16:55 ` [PATCH v2 7/9] arm64: KVM: add trap handlers for AArch32 debug registers Marc Zyngier
2014-05-25 15:35 ` Christoffer Dall
2014-05-28 16:00 ` Marc Zyngier
2014-05-29 8:53 ` Christoffer Dall
2014-05-20 16:55 ` [PATCH v2 8/9] arm64: KVM: implement lazy world switch for " Marc Zyngier
2014-05-25 15:35 ` Christoffer Dall
2014-05-20 16:55 ` [PATCH v2 9/9] arm64: KVM: enable trapping of all " Marc Zyngier
2014-05-25 15:36 ` Christoffer Dall
2014-05-28 16:10 ` Marc Zyngier
2014-05-29 8:55 ` Christoffer Dall
2014-05-25 15:34 ` [PATCH v2 0/9] arm64: KVM: debug infrastructure support Christoffer Dall
2014-05-28 9:56 ` Marc Zyngier
2014-05-28 9:58 ` Christoffer Dall
2014-05-28 10:11 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140525153459.GE3866@lvm \
--to=christoffer.dall@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).