On Fri, 11 Oct 2019 at 14:48, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> This is TFSRE0_EL1, TFSR_EL1, TFSR_EL2, TFSR_EL3,
> RGSR_EL1, GCR_EL1, GMID_EL1, and PSTATE.TCO.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> v3: Add GMID; add access_mte.
> v4: Define only TCO at mte_insn_reg.
> ---
> target/arm/cpu.h | 3 ++
> target/arm/internals.h | 6 ++++
> target/arm/helper.c | 73 ++++++++++++++++++++++++++++++++++++++
> target/arm/translate-a64.c | 11 ++++++
> 4 files changed, 93 insertions(+)
> + { .name = "GMID_EL1", .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 1, .crn = 0, .crm = 0, .opc2 = 4,
> + .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = GMID_EL1_BS },
This should trap if HCR_EL2.TID5 is 1 (since we're adding
support for the TID* ID reg trap bits now).
> + REGINFO_SENTINEL
> +};
> +
> +static const ARMCPRegInfo mte_tco_reginfo[] = {
> + { .name = "TCO", .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 3, .crn = 4, .crm = 2, .opc2 = 7,
> + .type = ARM_CP_NO_RAW,
> + .access = PL0_RW, .readfn = tco_read, .writefn = tco_write },
> + REGINFO_SENTINEL
> +};
> #endif
>
> static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo
> *ri,
> @@ -6881,6 +6948,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> if (cpu_isar_feature(aa64_rndr, cpu)) {
> define_arm_cp_regs(cpu, rndr_reginfo);
> }
So, aa64_mte_insn_reg here is checking for ID_AA64PFR1_EL1 != 0
("instructions accessible at EL0 are implemented")
and aa64_mte is checking for >= 2 ("full implementation").
I think a couple of brief comments would clarify:
> + if (cpu_isar_feature(aa64_mte_insn_reg, cpu)) {
/* EL0-visible MTE registers, present even for dummy
implementation */
> + define_arm_cp_regs(cpu, mte_tco_reginfo);
> + }
> + if (cpu_isar_feature(aa64_mte, cpu)) {
/* MTE registers present for a full implementation */
> + define_arm_cp_regs(cpu, mte_reginfo);
> + }
(The other way to arrange this would be to have the 'real'
TCO regdef in mte_reginfo, and separately have "reginfo
if we only have the dummy visible-from-EL0-parts-only
which defines a constant 0 TCO" (and also make the MSR_i
code implement a RAZ/WI for this case, for consistency).
An implementation that allows the guest to toggle the PSTATE.TCO
bit to no visible effect is architecturally valid, though.)
> #endif
>
> /*
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index c85db69db4..62bdf50796 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -1611,6 +1611,17 @@ static void handle_msr_i(DisasContext *s, uint32_t
> insn,
> s->base.is_jmp = DISAS_UPDATE;
> break;
>
> + case 0x1c: /* TCO */
> + if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
> + goto do_unallocated;
> + }
> + if (crm & 1) {
> + set_pstate_bits(PSTATE_TCO);
> + } else {
> + clear_pstate_bits(PSTATE_TCO);
> + }
> + break;
> +
> default:
> do_unallocated:
> unallocated_encoding(s);
> --
> 2.17.1
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
