Re: [PATCH v2 2/3] KVM: arm64: Plumb cp10 ID traps through the AArch64 sysreg handler

[Reiji Watanabe <reijiw@xxxxxxxxxx>]

On Thu, Mar 31, 2022 at 6:08 PM Oliver Upton <oupton@xxxxxxxxxx> wrote:
>
> In order to enable HCR_EL2.TID3 for AArch32 guests KVM needs to handle
> traps where ESR_EL2.EC=0x8, which corresponds to an attempted VMRS
> access from an ID group register. Specifically, the MVFR{0-2} registers
> are accessed this way from AArch32. Conveniently, these registers are
> architecturally mapped to MVFR{0-2}_EL1 in AArch64. Furthermore, KVM
> already handles reads to these aliases in AArch64.
>
> Plumb VMRS read traps through to the general AArch64 system register
> handler.
>
> Signed-off-by: Oliver Upton <oupton@xxxxxxxxxx>
> ---
>  arch/arm64/include/asm/kvm_host.h |  1 +
>  arch/arm64/kvm/handle_exit.c      |  1 +
>  arch/arm64/kvm/sys_regs.c         | 61 +++++++++++++++++++++++++++++++
>  3 files changed, 63 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 0e96087885fe..7a65ac268a22 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -673,6 +673,7 @@ int kvm_handle_cp14_64(struct kvm_vcpu *vcpu);
>  int kvm_handle_cp15_32(struct kvm_vcpu *vcpu);
>  int kvm_handle_cp15_64(struct kvm_vcpu *vcpu);
>  int kvm_handle_sys_reg(struct kvm_vcpu *vcpu);
> +int kvm_handle_cp10_id(struct kvm_vcpu *vcpu);
>
>  void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
>
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 97fe14aab1a3..5088a86ace5b 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -167,6 +167,7 @@ static exit_handle_fn arm_exit_handlers[] = {
>         [ESR_ELx_EC_CP15_64]    = kvm_handle_cp15_64,
>         [ESR_ELx_EC_CP14_MR]    = kvm_handle_cp14_32,
>         [ESR_ELx_EC_CP14_LS]    = kvm_handle_cp14_load_store,
> +       [ESR_ELx_EC_CP10_ID]    = kvm_handle_cp10_id,
>         [ESR_ELx_EC_CP14_64]    = kvm_handle_cp14_64,
>         [ESR_ELx_EC_HVC32]      = handle_hvc,
>         [ESR_ELx_EC_SMC32]      = handle_smc,
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 8b791256a5b4..4863592d060d 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2341,6 +2341,67 @@ static int kvm_handle_cp_64(struct kvm_vcpu *vcpu,
>
>  static int emulate_sys_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *params);
>
> +/*
> + * The CP10 ID registers are architecturally mapped to AArch64 feature
> + * registers. Abuse that fact so we can rely on the AArch64 handler for accesses
> + * from AArch32.
> + */
> +static bool kvm_esr_cp10_id_to_sys64(u32 esr, struct sys_reg_params *params)
> +{
> +       params->is_write = ((esr & 1) == 0);
> +       params->Op0 = 3;
> +       params->Op1 = 0;
> +       params->CRn = 0;
> +       params->CRm = 3;
> +
> +       switch ((esr >> 10) & 0xf) {
> +       /* MVFR0 */
> +       case 0b0111:
> +               params->Op2 = 0;
> +               break;
> +       /* MVFR1 */
> +       case 0b0110:
> +               params->Op2 = 1;
> +               break;
> +       /* MVFR2 */
> +       case 0b0101:
> +               params->Op2 = 2;
> +               break;
> +       default:
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
> +/**
> + * kvm_handle_cp10_id() - Handles a VMRS trap on guest access to a 'Media and
> + *                       VFP Register' from AArch32.
> + * @vcpu: The vCPU pointer
> + *
> + * MVFR{0-2} are architecturally mapped to the AArch64 MVFR{0-2}_EL1 registers.
> + * Work out the correct AArch64 system register encoding and reroute to the
> + * AArch64 system register emulation.
> + */
> +int kvm_handle_cp10_id(struct kvm_vcpu *vcpu)
> +{
> +       int Rt = kvm_vcpu_sys_get_rt(vcpu);
> +       u32 esr = kvm_vcpu_get_esr(vcpu);
> +       struct sys_reg_params params;
> +       int ret;
> +
> +       /* UNDEF on any unhandled register or an attempted write */
> +       if (!kvm_esr_cp10_id_to_sys64(esr, &params) || params.is_write) {
> +               kvm_inject_undefined(vcpu);

Nit: For debugging, it might be more useful to use unhandled_cp_access()
(, which needs to be changed to support ESR_ELx_EC_CP10_ID though)
rather than directly calling kvm_inject_undefined().

Reviewed-by: Reiji Watanabe <reijiw@xxxxxxxxxx>

Thanks,
Reiji



> +               return 1;
> +       }
> +
> +       ret = emulate_sys_reg(vcpu, &params);
> +
> +       vcpu_set_reg(vcpu, Rt, params.regval);
> +       return ret;
> +}
> +
>  /**
>   * kvm_emulate_cp15_id_reg() - Handles an MRC trap on a guest CP15 access where
>   *                            CRn=0, which corresponds to the AArch32 feature
> --
> 2.35.1.1094.g7c7d902a7c-goog
>