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.
