"Jason L. Wright'" <[email protected]> writes: > On Sun, Jun 07, 2026 at 06:42:30PM +0800, Zenghui Yu wrote: >> + Alexander and qemu-arm (for HVF/arm), >> >> On 5/29/26 7:47 PM, Peter Maydell wrote: >> > From: Jason Wright <[email protected]> >> > >> > Add an .accessfn to the RNDR and RNDRRS system registers that traps >> > reads to EL3 when SCR_EL3.TRNDR is set, as required by FEAT_RNG_TRAP. >> > Mark SCR_EL3.TRNDR (bit 40) as a writable field in scr_write() when >> > the CPU advertises the feature. The pseudocode in DDI0487 revision M.b >> > shows the trap firing from EL0, EL1, EL2, and EL3, so there is no >> > check of arm_current_el(). >> > >> > When FEAT_RNG_TRAP is implemented without FEAT_RNG, an RNDR/RNDRRS read >> > with SCR_EL3.TRNDR=0 should UNDEF rather than succeed; handle that case >> > in access_rndr(). Register the rndr_reginfo CP reg entries whenever either >> > FEAT_RNG or FEAT_RNG_TRAP is implemented, so the accessfn fires even on a >> > FEAT_RNG_TRAP-only CPU. >> > >> > When SCR_EL3.TRNDR is set, ID_AA64ISAR0_EL1.RNDR reads as 1 regardless >> > of whether FEAT_RNG is implemented; give ID_AA64ISAR0_EL1 a readfn so it >> > reports this at runtime, as we already do for ID_AA64PFR0_EL1. >> > >> > Suggested-by: Richard Henderson <[email protected]> >> > Suggested-by: Peter Maydell <[email protected]> >> > Signed-off-by: Jason Wright <[email protected]> >> > Reviewed-by: Richard Henderson <[email protected]> >> > Signed-off-by: Peter Maydell <[email protected]> >> > --- >> > target/arm/cpu-features.h | 5 ++++ >> > target/arm/helper.c | 58 +++++++++++++++++++++++++++++++++++---- >> > 2 files changed, 58 insertions(+), 5 deletions(-) >> > >> > diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h >> > index 4e8d844fea..38a695ded7 100644 >> > --- a/target/arm/cpu-features.h >> > +++ b/target/arm/cpu-features.h >> > @@ -908,6 +908,11 @@ static inline bool isar_feature_aa64_rndr(const >> > ARMISARegisters *id) >> > return FIELD_EX64_IDREG(id, ID_AA64ISAR0, RNDR) != 0; >> > } >> > >> > +static inline bool isar_feature_aa64_rng_trap(const ARMISARegisters *id) >> > +{ >> > + return FIELD_EX64_IDREG(id, ID_AA64PFR1, RNDR_TRAP) != 0; >> > +} >> > + >> > static inline bool isar_feature_aa64_tlbirange(const ARMISARegisters *id) >> > { >> > return FIELD_EX64_IDREG(id, ID_AA64ISAR0, TLB) == 2; >> > diff --git a/target/arm/helper.c b/target/arm/helper.c >> > index 34487eeaa3..9dd8fdfa41 100644 >> > --- a/target/arm/helper.c >> > +++ b/target/arm/helper.c >> > @@ -790,6 +790,9 @@ static void scr_write(CPUARMState *env, const >> > ARMCPRegInfo *ri, uint64_t value) >> > if (cpu_isar_feature(aa64_fpmr, cpu)) { >> > valid_mask |= SCR_ENFPM; >> > } >> > + if (cpu_isar_feature(aa64_rng_trap, cpu)) { >> > + valid_mask |= SCR_TRNDR; >> > + } >> > } else { >> > valid_mask &= ~(SCR_RW | SCR_ST); >> > if (cpu_isar_feature(aa32_ras, cpu)) { >> > @@ -5170,6 +5173,21 @@ static uint64_t id_aa64pfr0_read(CPUARMState *env, >> > const ARMCPRegInfo *ri) >> > } >> > return pfr0; >> > } >> > + >> > +static uint64_t id_aa64isar0_read(CPUARMState *env, const ARMCPRegInfo >> > *ri) >> > +{ >> > + ARMCPU *cpu = env_archcpu(env); >> > + uint64_t isar0 = GET_IDREG(&cpu->isar, ID_AA64ISAR0); >> > + >> > + /* >> > + * When FEAT_RNG_TRAP is active (SCR_EL3.TRNDR set), >> > ID_AA64ISAR0_EL1.RNDR >> > + * reads as 1 regardless of whether FEAT_RNG is implemented. >> > + */ >> > + if (env->cp15.scr_el3 & SCR_TRNDR) { >> > + isar0 = FIELD_DP64(isar0, ID_AA64ISAR0, RNDR, 1); >> > + } >> > + return isar0; >> > +} >> > #endif >> > >> > /* >> > @@ -5304,6 +5322,22 @@ static const ARMCPRegInfo pauth_reginfo[] = { >> > .fieldoffset = offsetof(CPUARMState, keys.apib.hi) }, >> > }; >> > >> > +static CPAccessResult access_rndr(CPUARMState *env, const ARMCPRegInfo >> > *ri, >> > + bool isread) >> > +{ >> > + if (env->cp15.scr_el3 & SCR_TRNDR) { >> > + return CP_ACCESS_TRAP_EL3; >> > + } >> > + /* >> > + * Note that FEAT_RNG_TRAP may be implemented without FEAT_RNG. >> > + * In that case, if the trap is not enabled, the read undefs. >> > + */ >> > + if (!cpu_isar_feature(aa64_rndr, env_archcpu(env))) { >> > + return CP_ACCESS_UNDEFINED; >> > + } >> > + return CP_ACCESS_OK; >> > +} >> > + >> > static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri) >> > { >> > Error *err = NULL; >> > @@ -5335,11 +5369,11 @@ static const ARMCPRegInfo rndr_reginfo[] = { >> > { .name = "RNDR", .state = ARM_CP_STATE_AA64, >> > .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END | ARM_CP_IO, >> > .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0, >> > - .access = PL0_R, .readfn = rndr_readfn }, >> > + .access = PL0_R, .accessfn = access_rndr, .readfn = rndr_readfn }, >> > { .name = "RNDRRS", .state = ARM_CP_STATE_AA64, >> > .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END | ARM_CP_IO, >> > .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1, >> > - .access = PL0_R, .readfn = rndr_readfn }, >> > + .access = PL0_R, .accessfn = access_rndr, .readfn = rndr_readfn }, >> > }; >> > >> > static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *ri, >> > @@ -6522,11 +6556,24 @@ void register_cp_regs_for_features(ARMCPU *cpu) >> > .access = PL1_R, .type = ARM_CP_CONST, >> > .accessfn = access_tid3, >> > .resetvalue = 0 }, >> > + /* >> > + * ID_AA64ISAR0_EL1 is not a plain ARM_CP_CONST in system >> > + * emulation because the RNDR field depends on SCR_EL3.TRNDR >> > + * at read time when FEAT_RNG_TRAP is implemented. >> > + */ >> > { .name = "ID_AA64ISAR0_EL1", .state = ARM_CP_STATE_AA64, >> > .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 0, >> > - .access = PL1_R, .type = ARM_CP_CONST, >> > + .access = PL1_R, >> > +#ifdef CONFIG_USER_ONLY >> > + .type = ARM_CP_CONST, >> > + .resetvalue = GET_IDREG(isar, ID_AA64ISAR0) >> > +#else >> > + .type = ARM_CP_NO_RAW, >> > .accessfn = access_tid3, >> > - .resetvalue = GET_IDREG(isar, ID_AA64ISAR0)}, >> > + .readfn = id_aa64isar0_read, >> > + .writefn = arm_cp_write_ignore >> > +#endif >> > + }, >> >> A new assert() was triggered when booting guest on M1 since this change: >> >> Assertion failed: (!(ri->type & ARM_CP_NO_RAW)), function >> hvf_arch_init_vcpu, file hvf.c, line 1442. >> >> Thanks, >> Zenghui >> > Thanks for the report and the bisect, Zenghui. I can reproduce on M1 with: > > qemu-system-aarch64 -M virt,accel=hvf -cpu host \ > -nographic -display none -bios /dev/null > > ID_AA64PFR0_EL1 has the same NO_RAW + readfn shape that the FEAT_RNG_TRAP > change gave ID_AA64ISAR0_EL1, and HVF already accommodates it by listing > the cpreg in the SYNC_NO_RAW_REGS block in target/arm/hvf/sysreg.c.inc > (so the assert loop skips it) and pushing QEMU's value to the vCPU at > init time. Mirroring that pattern for ID_AA64ISAR0_EL1 clears the assert > without disturbing the readfn semantics that the spec requires if a > FEAT_RNG_TRAP-only CPU eventually appears. > > I'll send a fix-up as [PATCH] target/arm/hvf shortly.
This highlights why we need some bare metal MacOS machines in the CI to exercise this code path. -- Alex Bennée Virtualisation Tech Lead @ Linaro
