| Message ID | 20220409000742.293691-12-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement features Debugv8p4, RAS, IESB | expand |
On Sat, 9 Apr 2022 at 01:14, Richard Henderson <richard.henderson@linaro.org> wrote: > > Add only the system registers required to implement zero error > records. This means we need to save state for ERRSELR, but all > values are out of range, so none of the indexed error record > registers need be implemented. > > Add the EL2 registers required for injecting virtual SError. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > +/* > + * Minimal RAS implementation with no Error Records. > + * Which means that all of the Error Record registers: > + * ERXADDR_EL1 > + * ERXCTLR_EL1 > + * ERXFR_EL1 > + * ERXMISC0_EL1 > + * ERXMISC1_EL1 > + * ERXMISC2_EL1 > + * ERXMISC3_EL1 > + * ERXPFGCDN_EL1 (RASv1p1) > + * ERXPFGCTL_EL1 (RASv1p1) > + * ERXPFGF_EL1 (RASv1p1) > + * ERXSTATUS_EL1 > + * may generate UNDEFINED, which is the effect we get by not > + * listing them at all. > + */ > +static const ARMCPRegInfo minimal_ras_reginfo_el1[] = { > + { .name = "DISR_EL1", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 0, .crn = 0xc, .crm = 1, .opc2 = 1, ".crn = 12", please -- no other reginfo struct uses hex here. Similarly below. > + .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.disr_el1), > + .readfn = disr_read, .writefn = disr_write, .raw_writefn = raw_write }, > + { .name = "ERRIDR_EL1", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 3, .opc2 = 0, > + .access = PL1_R, .accessfn = access_terr, > + .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "ERRSELR_EL1", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 3, .opc2 = 1, > + .access = PL1_RW, .accessfn = access_terr, > + .fieldoffset = offsetof(CPUARMState, cp15.errselr_el1) }, By my reading of the spec we could make ERRSELR_EL1 RAZ/WI, because writing an over-large number has a number of behaviours including that the value the guest can read back is UNKNOWN. That would save having the CPU state struct field. > + REGINFO_SENTINEL > +}; > + > +static const ARMCPRegInfo minimal_ras_reginfo_el2[] = { > + { .name = "VDISR_EL2", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 4, .crn = 0xc, .crm = 1, .opc2 = 1, > + .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.vdisr_el2) }, > + { .name = "VSESR_EL2", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 3, > + .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.vsesr_el2) }, > + REGINFO_SENTINEL > +}; > + > +static const ARMCPRegInfo minimal_ras_reginfo_no_el2[] = { > + { .name = "VDISR_EL2", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 4, .crn = 0xc, .crm = 1, .opc2 = 1, > + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "VSESR_EL2", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 3, > + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + REGINFO_SENTINEL > +}; > + > /* Return the exception level to which exceptions should be taken > * via SVEAccessTrap. If an exception should be routed through > * AArch64.AdvSIMDFPAccessTrap, return 0; fp_exception_el should > @@ -8452,6 +8550,15 @@ void register_cp_regs_for_features(ARMCPU *cpu) > define_one_arm_cp_reg(cpu, &ssbs_reginfo); > } > > + if (cpu_isar_feature(any_ras, cpu)) { > + define_arm_cp_regs(cpu, minimal_ras_reginfo_el1); > + if (arm_feature(env, ARM_FEATURE_EL2)) { > + define_arm_cp_regs(cpu, minimal_ras_reginfo_el2); > + } else { > + define_arm_cp_regs(cpu, minimal_ras_reginfo_no_el2); > + } > + } > + > if (cpu_isar_feature(aa64_vh, cpu) || > cpu_isar_feature(aa64_debugv8p2, cpu)) { > if (arm_feature(env, ARM_FEATURE_EL2)) { > -- > 2.25.1 Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 4/11/22 08:49, Peter Maydell wrote: >> + { .name = "ERRSELR_EL1", .state = ARM_CP_STATE_BOTH, >> + .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 3, .opc2 = 1, >> + .access = PL1_RW, .accessfn = access_terr, >> + .fieldoffset = offsetof(CPUARMState, cp15.errselr_el1) }, > > By my reading of the spec we could make ERRSELR_EL1 RAZ/WI, because > writing an over-large number has a number of behaviours including > that the value the guest can read back is UNKNOWN. That would save > having the CPU state struct field. Good point, I should have read the fine print myself: If ERRIDR_EL1 indicates that zero error records are implemented, then it is IMPLEMENTATION DEFINED whether ERRSELR_EL1 is UNDEFINED or RES 0. so perhaps it's better to leave it UNDEFINED. r~
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 890001f26b..66becc47f2 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -524,6 +524,12 @@ typedef struct CPUArchState { uint64_t tfsr_el[4]; /* tfsre0_el1 is index 0. */ uint64_t gcr_el1; uint64_t rgsr_el1; + + /* Minimal RAS registers */ + uint64_t disr_el1; + uint64_t errselr_el1; + uint64_t vdisr_el2; + uint64_t vsesr_el2; } cp15; struct { diff --git a/target/arm/helper.c b/target/arm/helper.c index 210c139818..01f8558fca 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6147,6 +6147,104 @@ static const ARMCPRegInfo debug_lpae_cp_reginfo[] = { REGINFO_SENTINEL }; +/* + * Check for traps to RAS registers, which are controlled + * by HCR_EL2.TERR and SCR_EL3.TERR. + */ +static CPAccessResult access_terr(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + int el = arm_current_el(env); + + if (el < 2 && (arm_hcr_el2_eff(env) & HCR_TERR)) { + return CP_ACCESS_TRAP_EL2; + } + if (el < 3 && (env->cp15.scr_el3 & SCR_TERR)) { + return CP_ACCESS_TRAP_EL3; + } + return CP_ACCESS_OK; +} + +static uint64_t disr_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ + int el = arm_current_el(env); + + if (el < 2 && (arm_hcr_el2_eff(env) & HCR_AMO)) { + return env->cp15.vdisr_el2; + } + if (el < 3 && (env->cp15.scr_el3 & SCR_EA)) { + return 0; /* RAZ/WI */ + } + return env->cp15.disr_el1; +} + +static void disr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t val) +{ + int el = arm_current_el(env); + + if (el < 2 && (arm_hcr_el2_eff(env) & HCR_AMO)) { + env->cp15.vdisr_el2 = val; + return; + } + if (el < 3 && (env->cp15.scr_el3 & SCR_EA)) { + return; /* RAZ/WI */ + } + env->cp15.disr_el1 = val; +} + +/* + * Minimal RAS implementation with no Error Records. + * Which means that all of the Error Record registers: + * ERXADDR_EL1 + * ERXCTLR_EL1 + * ERXFR_EL1 + * ERXMISC0_EL1 + * ERXMISC1_EL1 + * ERXMISC2_EL1 + * ERXMISC3_EL1 + * ERXPFGCDN_EL1 (RASv1p1) + * ERXPFGCTL_EL1 (RASv1p1) + * ERXPFGF_EL1 (RASv1p1) + * ERXSTATUS_EL1 + * may generate UNDEFINED, which is the effect we get by not + * listing them at all. + */ +static const ARMCPRegInfo minimal_ras_reginfo_el1[] = { + { .name = "DISR_EL1", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 0, .crn = 0xc, .crm = 1, .opc2 = 1, + .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.disr_el1), + .readfn = disr_read, .writefn = disr_write, .raw_writefn = raw_write }, + { .name = "ERRIDR_EL1", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 3, .opc2 = 0, + .access = PL1_R, .accessfn = access_terr, + .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ERRSELR_EL1", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 3, .opc2 = 1, + .access = PL1_RW, .accessfn = access_terr, + .fieldoffset = offsetof(CPUARMState, cp15.errselr_el1) }, + REGINFO_SENTINEL +}; + +static const ARMCPRegInfo minimal_ras_reginfo_el2[] = { + { .name = "VDISR_EL2", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 4, .crn = 0xc, .crm = 1, .opc2 = 1, + .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.vdisr_el2) }, + { .name = "VSESR_EL2", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 3, + .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.vsesr_el2) }, + REGINFO_SENTINEL +}; + +static const ARMCPRegInfo minimal_ras_reginfo_no_el2[] = { + { .name = "VDISR_EL2", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 4, .crn = 0xc, .crm = 1, .opc2 = 1, + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "VSESR_EL2", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 3, + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + REGINFO_SENTINEL +}; + /* Return the exception level to which exceptions should be taken * via SVEAccessTrap. If an exception should be routed through * AArch64.AdvSIMDFPAccessTrap, return 0; fp_exception_el should @@ -8452,6 +8550,15 @@ void register_cp_regs_for_features(ARMCPU *cpu) define_one_arm_cp_reg(cpu, &ssbs_reginfo); } + if (cpu_isar_feature(any_ras, cpu)) { + define_arm_cp_regs(cpu, minimal_ras_reginfo_el1); + if (arm_feature(env, ARM_FEATURE_EL2)) { + define_arm_cp_regs(cpu, minimal_ras_reginfo_el2); + } else { + define_arm_cp_regs(cpu, minimal_ras_reginfo_no_el2); + } + } + if (cpu_isar_feature(aa64_vh, cpu) || cpu_isar_feature(aa64_debugv8p2, cpu)) { if (arm_feature(env, ARM_FEATURE_EL2)) {
Add only the system registers required to implement zero error records. This means we need to save state for ERRSELR, but all values are out of range, so none of the indexed error record registers need be implemented. Add the EL2 registers required for injecting virtual SError. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 6 +++ target/arm/helper.c | 107 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+)