On Mon, 8 Jun 2026 at 16:56, Jason L. Wright' <[email protected]> wrote: > > On Mon, Jun 08, 2026 at 02:07:03PM +0100, Peter Maydell wrote: > > On Sun, 7 Jun 2026 at 19:22, Jason Wright <[email protected]> wrote: > > > > > > Commit 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS") > > > gave ID_AA64ISAR0_EL1 a readfn so the RNDR field can reflect SCR_EL3.TRNDR > > > at read time, and marked the cpreg ARM_CP_NO_RAW in the system-emulation > > > path. HVF then trips its hvf_arch_init_vcpu() assertion that no ID > > > register in hvf_sreg_list[] is NO_RAW, aborting on boot on Apple Silicon: > > > > > > Assertion failed: (!(ri->type & ARM_CP_NO_RAW)), > > > function hvf_arch_init_vcpu, file hvf.c, line 1442. > > > > > > Reproduce with: > > > > > > qemu-system-aarch64 -M virt,accel=hvf -cpu host \ > > > -nographic -display none -bios /dev/null > > > > > > Mirror the existing treatment of ID_AA64PFR0_EL1: move > > > HV_SYS_REG_ID_AA64ISAR0_EL1 into the SYNC_NO_RAW_REGS block in > > > sysreg.c.inc so the assert loop skips it, and push QEMU's view of the > > > register to the vCPU at init time. HVF does not expose EL3, so > > > SCR_EL3.TRNDR is never set and the readfn is functionally static there. > > > > > > Reported-by: Zenghui Yu <[email protected]> > > > Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS") > > > Signed-off-by: Jason Wright <[email protected]> > > > --- > > > target/arm/hvf/hvf.c | 4 ++++ > > > target/arm/hvf/sysreg.c.inc | 2 +- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c > > > index d88cbe7c82..afa1120c8a 100644 > > > --- a/target/arm/hvf/hvf.c > > > +++ b/target/arm/hvf/hvf.c > > > @@ -1485,6 +1485,10 @@ int hvf_arch_init_vcpu(CPUState *cpu) > > > ret = hv_vcpu_set_sys_reg(cpu->accel->fd, > > > HV_SYS_REG_ID_AA64PFR0_EL1, pfr); > > > assert_hvf_ok(ret); > > > > > > + ret = hv_vcpu_set_sys_reg(cpu->accel->fd, > > > HV_SYS_REG_ID_AA64ISAR0_EL1, > > > + GET_IDREG(&arm_cpu->isar, ID_AA64ISAR0)); > > > + assert_hvf_ok(ret); > > > + > > > > For ID_AA64PFR0_EL1, we do "read the value from hvf, update it, > > write it back", and we do not either read or write the isar.idregs[] > > entry for it. > > > > For ID_AA64MMFR0_EL1, we read the hvf value into the isar.idregs[] > > array entry, update it there, and write it back to hvf. > > > > For ID_AA64ISAR0_EL1, we write whatever is in the isar.idregs[] > > array entry into hvf. > > > > Why do we do three different things for these three registers ? > > > I think it's each call doing the minimum work required for that > register. > > MIDR_EL1 / MPIDR_EL1 are QEMU-defined identity values; HVF has > nothing useful to contribute, so we just write.
These don't have isar.idregs[] entries either, so it makes sense that they're not quite the same. > ID_AA64PFR0_EL1 needs HVF's value as the base (host feature set) > and then ORs in the GIC sysreg-iface bit, which depends on > env->gicv3state runtime overlay, hence the get/modify/set. But why do we not update isar.idregs[] to match ? > ID_AA64MMFR0_EL1 also needs HVF's value as the base, and the > modification (clamping PARANGE to the configured IPA size) > consults isar.idregs[], so we read into idregs[] before clamping > and then write back. But why does the modification consult isar.idregs[] rather than taking a value that it's passed in as an argument ? > ID_AA64ISAR0_EL1 in this patch has no runtime overlay on the HVF > path: SCR_EL3.TRNDR is permanently 0 since HVF does not expose > EL3, so the readfn is a constant equal to > isar.idregs[ID_AA64ISAR0_EL1_IDX], which was already seeded from > the host during realize. But if the values in isar.idregs[] are seeded from HVF during realize, why do we not trust the isar.idregs[] value when we're handling ID_AA64PFR0_EL1? > The three methods track different requirements, so I matched > the closest existing pattern for ID_AA64ISAR0_EL1 and left > the others alone. It seems to me that you've added a third thing that's different from either of the ways that we handle existing ID registers with an isar.idregs[] field. I think we need to figure out: (1) can we trust that the isar.idregs[] values at this point are the ones that hvf uses? If so we don't need to re-read the hvf values. If we can't trust them we should consistently read the hvf values. (2) do we need to update the isar.idregs[] values when we modify the values to write back to hvf? If so, we should do it consistently for all these regs. If not, we should not update any of them. Currently ID_AA64PFR0_EL1 neither trusts isar.idregs[] nor updates it; ID_AA64MMFR0_EL1 does not trust isar.idregs[] but does update it; and ID_AA64ISAR0_EL1 trusts isar.idregs[] and has no need to update it. thanks -- PMM
