| Message ID | 20191011134744.2477-4-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,01/22] target/arm: Add MTE_ACTIVE to tb_flags | expand |
On Fri, 11 Oct 2019 at 14:48, Richard Henderson <richard.henderson@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.henderson@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.maydell@linaro.org> thanks -- PMM
On 12/3/19 3:48 AM, Peter Maydell wrote: >> + { .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). Done. > 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: Done. > (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). Done. I agree this is a better way to treat the EL0-only case... > An implementation that allows the guest to toggle the PSTATE.TCO > bit to no visible effect is architecturally valid, though. ... because this could turn out to be slightly confusing. r~
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 408d749b7a..d99bb5e956 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -486,6 +486,9 @@ typedef struct CPUARMState { uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */ uint64_t vpidr_el2; /* Virtualization Processor ID Register */ uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */ + uint64_t tfsr_el[4]; /* tfsrel0_el1 is index 0. */ + uint64_t gcr_el1; + uint64_t rgsr_el1; } cp15; struct { diff --git a/target/arm/internals.h b/target/arm/internals.h index 9486680b87..bfa243be06 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -1079,4 +1079,10 @@ void arm_log_exception(int idx); #endif /* !CONFIG_USER_ONLY */ +/* + * The log2 of the words in the tag block, for GMID_EL1.BS. + * The is the maximum, 256 bytes, which manipulates 64-bits of tags. + */ +#define GMID_EL1_BS 6 + #endif diff --git a/target/arm/helper.c b/target/arm/helper.c index f9dee51ede..f435a8d8bd 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5921,6 +5921,73 @@ static const ARMCPRegInfo rndr_reginfo[] = { .access = PL0_R, .readfn = rndr_readfn }, REGINFO_SENTINEL }; + +static CPAccessResult access_mte(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + int el = arm_current_el(env); + + if (el < 2 && + arm_feature(env, ARM_FEATURE_EL2) && + !(arm_hcr_el2_eff(env) & HCR_ATA)) { + return CP_ACCESS_TRAP_EL2; + } + if (el < 3 && + arm_feature(env, ARM_FEATURE_EL3) && + !(env->cp15.scr_el3 & SCR_ATA)) { + return CP_ACCESS_TRAP_EL3; + } + return CP_ACCESS_OK; +} + +static uint64_t tco_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ + return env->pstate & PSTATE_TCO; +} + +static void tco_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t val) +{ + env->pstate = (env->pstate & ~PSTATE_TCO) | (val & PSTATE_TCO); +} + +static const ARMCPRegInfo mte_reginfo[] = { + { .name = "TFSRE0_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 6, .crm = 6, .opc2 = 1, + .access = PL1_RW, .accessfn = access_mte, + .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[0]) }, + { .name = "TFSR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 6, .crm = 5, .opc2 = 0, + .access = PL1_RW, .accessfn = access_mte, + .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[1]) }, + { .name = "TFSR_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 5, .opc2 = 0, + .access = PL2_RW, .accessfn = access_mte, + .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[2]) }, + { .name = "TFSR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 6, .crm = 6, .opc2 = 0, + .access = PL3_RW, + .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[3]) }, + { .name = "RGSR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 5, + .access = PL1_RW, .accessfn = access_mte, + .fieldoffset = offsetof(CPUARMState, cp15.rgsr_el1) }, + { .name = "GCR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 6, + .access = PL1_RW, .accessfn = access_mte, + .fieldoffset = offsetof(CPUARMState, cp15.gcr_el1) }, + { .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 }, + 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); } + if (cpu_isar_feature(aa64_mte_insn_reg, cpu)) { + define_arm_cp_regs(cpu, mte_tco_reginfo); + } + if (cpu_isar_feature(aa64_mte, cpu)) { + define_arm_cp_regs(cpu, mte_reginfo); + } #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);
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.henderson@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(+) -- 2.17.1