[PATCH 15/25] KVM: arm64: Use the xarray as the primary sysreg/sysinsn walker
Joey Gouly
joey.gouly at arm.com
Wed Jan 24 08:48:10 PST 2024
On Mon, Jan 22, 2024 at 08:18:42PM +0000, Marc Zyngier wrote:
> Since we always start sysreg/sysinsn handling by searching the
> xarray, use it as the source of the index in the correct sys_reg_desc
> array.
>
> This allows some cleanup, such as moving the handling of unknown
> sysregs in a single location.
>
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> ---
> arch/arm64/include/asm/kvm_nested.h | 2 +-
> arch/arm64/kvm/emulate-nested.c | 36 +++++++++++-----
> arch/arm64/kvm/sys_regs.c | 64 +++++++++--------------------
> 3 files changed, 46 insertions(+), 56 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 4882905357f4..68465f87d308 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -60,7 +60,7 @@ static inline u64 translate_ttbr0_el2_to_ttbr0_el1(u64 ttbr0)
> return ttbr0 & ~GENMASK_ULL(63, 48);
> }
>
> -extern bool __check_nv_sr_forward(struct kvm_vcpu *vcpu);
> +extern bool __check_nv_sr_forward(struct kvm_vcpu *vcpu, int *sr_idx);
>
> int kvm_init_nv_sysregs(struct kvm *kvm);
>
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 342d43b66fda..54ab4d240fc6 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2001,7 +2001,7 @@ static bool check_fgt_bit(struct kvm *kvm, bool is_read,
> return !(kvm_get_sysreg_res0(kvm, sr) & BIT(tc.bit));
> }
>
> -bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
> +bool __check_nv_sr_forward(struct kvm_vcpu *vcpu, int *sr_index)
> {
> union trap_config tc;
> enum trap_behaviour b;
> @@ -2009,9 +2009,6 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
> u32 sysreg;
> u64 esr, val;
>
> - if (!vcpu_has_nv(vcpu) || is_hyp_ctxt(vcpu))
> - return false;
> -
> esr = kvm_vcpu_get_esr(vcpu);
> sysreg = esr_sys64_to_sysreg(esr);
> is_read = (esr & ESR_ELx_SYS64_ISS_DIR_MASK) == ESR_ELx_SYS64_ISS_DIR_READ;
> @@ -2022,13 +2019,16 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
> * A value of 0 for the whole entry means that we know nothing
> * for this sysreg, and that it cannot be re-injected into the
> * nested hypervisor. In this situation, let's cut it short.
> - *
> - * Note that ultimately, we could also make use of the xarray
> - * to store the index of the sysreg in the local descriptor
> - * array, avoiding another search... Hint, hint...
> */
> if (!tc.val)
> - return false;
> + goto local;
> +
> + /*
> + * If we're not nesting, immediately return to the caller, with the
> + * sysreg index, should we have it.
> + */
> + if (!vcpu_has_nv(vcpu) || is_hyp_ctxt(vcpu))
> + goto local;
>
> switch ((enum fgt_group_id)tc.fgt) {
> case __NO_FGT_GROUP__:
> @@ -2070,7 +2070,7 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
> case __NR_FGT_GROUP_IDS__:
> /* Something is really wrong, bail out */
> WARN_ONCE(1, "__NR_FGT_GROUP_IDS__");
> - return false;
> + goto local;
> }
>
> if (tc.fgt != __NO_FGT_GROUP__ && check_fgt_bit(vcpu->kvm, is_read,
> @@ -2083,6 +2083,22 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
> ((b & BEHAVE_FORWARD_WRITE) && !is_read))
> goto inject;
>
> +local:
> + if (!tc.msr) {
> + struct sys_reg_params params;
> +
> + params = esr_sys64_to_params(esr);
> +
> + // IMPDEF range. See ARM DDI 0487E.a, section D12.3.2
I know you're just moving this code, but can we update the reference? It's
DDI0487 J.a, D18.3.2 Reserved encodings for IMPLEMENTATION DEFINED registers
now. Feel free to drop the title of the section if it looks too long!
> + if (!(params.Op0 == 3 && (params.CRn & 0b1011) == 0b1011))
> + print_sys_reg_msg(¶ms,
> + "Unsupported guest access at: %lx\n",
> + *vcpu_pc(vcpu));
> + kvm_inject_undefined(vcpu);
> + return true;
> + }
> +
> + *sr_index = tc.msr - 1;
> return false;
>
> inject:
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 65319193e443..794d1f8c9bfe 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -3397,12 +3397,6 @@ int kvm_handle_cp14_32(struct kvm_vcpu *vcpu)
> return kvm_handle_cp_32(vcpu, ¶ms, cp14_regs, ARRAY_SIZE(cp14_regs));
> }
>
> -static bool is_imp_def_sys_reg(struct sys_reg_params *params)
> -{
> - // See ARM DDI 0487E.a, section D12.3.2
> - return params->Op0 == 3 && (params->CRn & 0b1011) == 0b1011;
> -}
> -
> /**
> * emulate_sys_reg - Emulate a guest access to an AArch64 system register
> * @vcpu: The VCPU pointer
> @@ -3411,44 +3405,22 @@ static bool is_imp_def_sys_reg(struct sys_reg_params *params)
> * Return: true if the system register access was successful, false otherwise.
> */
> static bool emulate_sys_reg(struct kvm_vcpu *vcpu,
> - struct sys_reg_params *params)
> + struct sys_reg_params *params)
> {
> const struct sys_reg_desc *r;
>
> r = find_reg(params, sys_reg_descs, ARRAY_SIZE(sys_reg_descs));
> -
> if (likely(r)) {
> perform_access(vcpu, params, r);
> return true;
> }
>
> - if (is_imp_def_sys_reg(params)) {
> - kvm_inject_undefined(vcpu);
> - } else {
> - print_sys_reg_msg(params,
> - "Unsupported guest sys_reg access at: %lx [%08lx]\n",
> - *vcpu_pc(vcpu), *vcpu_cpsr(vcpu));
> - kvm_inject_undefined(vcpu);
> - }
> - return false;
> -}
> -
> -static int emulate_sys_instr(struct kvm_vcpu *vcpu, struct sys_reg_params *p)
> -{
> - const struct sys_reg_desc *r;
> -
> - /* Search from the system instruction table. */
> - r = find_reg(p, sys_insn_descs, ARRAY_SIZE(sys_insn_descs));
> + print_sys_reg_msg(params,
> + "Unsupported guest sys_reg access at: %lx [%08lx]\n",
> + *vcpu_pc(vcpu), *vcpu_cpsr(vcpu));
> + kvm_inject_undefined(vcpu);
>
> - if (likely(r)) {
> - perform_access(vcpu, p, r);
> - } else {
> - kvm_err("Unsupported guest sys instruction at: %lx\n",
> - *vcpu_pc(vcpu));
> - print_sys_reg_instr(p);
> - kvm_inject_undefined(vcpu);
> - }
> - return 1;
> + return false;
> }
>
> static void kvm_reset_id_regs(struct kvm_vcpu *vcpu)
> @@ -3504,31 +3476,33 @@ void kvm_reset_sys_regs(struct kvm_vcpu *vcpu)
> */
> int kvm_handle_sys_reg(struct kvm_vcpu *vcpu)
> {
> + const struct sys_reg_desc *desc = NULL;
> struct sys_reg_params params;
> unsigned long esr = kvm_vcpu_get_esr(vcpu);
> int Rt = kvm_vcpu_sys_get_rt(vcpu);
> + int sr_idx;
>
> trace_kvm_handle_sys_reg(esr);
>
> - if (__check_nv_sr_forward(vcpu))
> + if (__check_nv_sr_forward(vcpu, &sr_idx))
> return 1;
>
> params = esr_sys64_to_params(esr);
> params.regval = vcpu_get_reg(vcpu, Rt);
>
> - /* System register? */
> - if (params.Op0 == 2 || params.Op0 == 3) {
> - if (!emulate_sys_reg(vcpu, ¶ms))
> - return 1;
> + if (params.Op0 == 2 || params.Op0 == 3)
> + desc = &sys_reg_descs[sr_idx];
> + else
> + desc = &sys_insn_descs[sr_idx];
>
> - if (!params.is_write)
> - vcpu_set_reg(vcpu, Rt, params.regval);
> + perform_access(vcpu, ¶ms, desc);
>
> - return 1;
> - }
> + /* Read from system register? */
> + if (!params.is_write &&
> + (params.Op0 == 2 || params.Op0 == 3))
> + vcpu_set_reg(vcpu, Rt, params.regval);
>
> - /* Hints, PSTATE (Op0 == 0) and System instructions (Op0 == 1) */
> - return emulate_sys_instr(vcpu, ¶ms);
> + return 1;
> }
>
> /******************************************************************************
Other than the ref update:
Reviewed-by: Joey Gouly <joey.gouly at arm.com>
Thanks,
Joey
More information about the linux-arm-kernel
mailing list