[PATCH 1/3] KVM: arm64: Wire up CP15 feature registers to their AArch64 equivalents

[Reiji Watanabe]

Hi Oliver,

On Mon, Mar 28, 2022 at 6:13 PM Oliver Upton <oupton at google.com> wrote:
>
> KVM currently does not trap ID register accesses from an AArch32 EL1.
> This is painful for a couple of reasons. Certain unimplemented features
> are visible to AArch32 EL1, as we limit PMU to version 3 and the debug
> architecture to v8.0. Additionally, we attempt to paper over
> heterogeneous systems by using register values that are safe
> system-wide. All this hard work is completely sidestepped because KVM
> does not set TID3 for AArch32 guests.
>
> Fix up handling of CP15 feature registers by simply rerouting to their
> AArch64 aliases. Punt setting HCR_EL2.TID3 to a later change, as we need
> to fix up the oddball CP10 feature registers still.
>
> Signed-off-by: Oliver Upton <oupton at google.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 66 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index dd34b5ab51d4..30771f950027 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2339,6 +2339,65 @@ static int kvm_handle_cp_64(struct kvm_vcpu *vcpu,
>         return 1;
>  }
>
> +static int emulate_sys_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *params);
> +
> +/**
> + * kvm_emulate_cp15_id_reg() - Handles an MRC trap on a guest CP15 access where
> + *                            CRn=0, which corresponds to the AArch32 feature
> + *                            registers.
> + * @vcpu: the vCPU pointer
> + * @params: the system register access parameters.
> + *
> + * Our cp15 system register tables do not enumerate the AArch32 feature
> + * registers. Conveniently, our AArch64 table does, and the AArch32 system
> + * register encoding can be trivially remapped into the AArch64 for the feature
> + * registers: Append op0=3, leaving op1, CRn, CRm, and op2 the same.
> + *
> + * According to DDI0487G.b G7.3.1, paragraph "Behavior of VMSAv8-32 32-bit
> + * System registers with (coproc=0b1111, CRn==c0)", read accesses from this
> + * range are either UNKNOWN or RES0. Rerouting remains architectural as we
> + * treat undefined registers in this range as RAZ.
> + */
> +static int kvm_emulate_cp15_id_reg(struct kvm_vcpu *vcpu,
> +                                  struct sys_reg_params *params)
> +{
> +       int Rt = kvm_vcpu_sys_get_rt(vcpu);
> +       int ret = 1;
> +
> +       params->Op0 = 3;

Nit: Shouldn't we restore the original Op0 after emulate_sys_reg() ?
(unhandled_cp_access() prints Op0. Restoring the original one
 would be more robust against future changes)

> +
> +       /*
> +        * All registers where CRm > 3 are known to be UNKNOWN/RAZ from AArch32.
> +        * Avoid conflicting with future expansion of AArch64 feature registers
> +        * and simply treat them as RAZ here.
> +        */
> +       if (params->CRm > 3)
> +               params->regval = 0;
> +       else
> +               ret = emulate_sys_reg(vcpu, params);
> +
> +       /* Treat impossible writes to RO registers as UNDEFINED */
> +       if (params->is_write)

This checking can be done even before calling emulate_sys_reg().
BTW, __access_id_reg() also injects UNDEFINED when p->is_write is true.

> +               unhandled_cp_access(vcpu, params);
> +       else
> +               vcpu_set_reg(vcpu, Rt, params->regval);
> +
> +       return ret;
> +}
> +
> +/**
> + * kvm_is_cp15_id_reg() - Returns true if the specified CP15 register is an
> + *                       AArch32 ID register.
> + * @params: the system register access parameters
> + *
> + * Note that CP15 ID registers where CRm=0 are excluded from this check, as they
> + * are already correctly handled in the CP15 register table.

I don't think this is true for all of the registers:)
I think at least some of them are not trapped (TCMTR, TLBTR,
REVIDR, etc), and I don't think they are handled in the CP15
register table.

Thanks,
Reiji


> + */
> +static inline bool kvm_is_cp15_id_reg(struct sys_reg_params *params)
> +{
> +       return params->CRn == 0 && params->Op1 == 0 && params->CRm != 0;
> +}
> +
>  /**
>   * kvm_handle_cp_32 -- handles a mrc/mcr trap on a guest CP14/CP15 access
>   * @vcpu: The VCPU pointer
> @@ -2360,6 +2419,13 @@ static int kvm_handle_cp_32(struct kvm_vcpu *vcpu,
>         params.Op1 = (esr >> 14) & 0x7;
>         params.Op2 = (esr >> 17) & 0x7;
>
> +       /*
> +        * Certain AArch32 ID registers are handled by rerouting to the AArch64
> +        * system register table.
> +        */
> +       if (ESR_ELx_EC(esr) == ESR_ELx_EC_CP15_32 && kvm_is_cp15_id_reg(&params))
> +               return kvm_emulate_cp15_id_reg(vcpu, &params);
> +
>         if (!emulate_cp(vcpu, &params, global, nr_global)) {
>                 if (!params.is_write)
>                         vcpu_set_reg(vcpu, Rt, params.regval);
> --
> 2.35.1.1021.g381101b075-goog
>