Hi Oliver,
On Mon, Mar 28, 2022 at 6:13 PM Oliver Upton <oupton@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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(¶ms))
> + return kvm_emulate_cp15_id_reg(vcpu, ¶ms);
> +
> if (!emulate_cp(vcpu, ¶ms, global, nr_global)) {
> if (!params.is_write)
> vcpu_set_reg(vcpu, Rt, params.regval);
> --
> 2.35.1.1021.g381101b075-goog
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
