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 <zenghui.yu@linux.dev>
Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
Signed-off-by: Jason Wright <wrigjl@proton.me>
---
 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);
+
     /* We're limited to underlying hardware caps, override internal versions */
     ret = hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_ID_AA64MMFR0_EL1,
                               &arm_cpu->isar.idregs[ID_AA64MMFR0_EL1_IDX]);
diff --git a/target/arm/hvf/sysreg.c.inc b/target/arm/hvf/sysreg.c.inc
index c11dbf274e..acd5a41364 100644
--- a/target/arm/hvf/sysreg.c.inc
+++ b/target/arm/hvf/sysreg.c.inc
@@ -89,13 +89,13 @@ DEF_SYSREG(HV_SYS_REG_MDCCINT_EL1, 2, 0, 0, 2, 0)
 DEF_SYSREG(HV_SYS_REG_MIDR_EL1, 3, 0, 0, 0, 0)
 DEF_SYSREG(HV_SYS_REG_MPIDR_EL1, 3, 0, 0, 0, 5)
 DEF_SYSREG(HV_SYS_REG_ID_AA64PFR0_EL1, 3, 0, 0, 4, 0)
+DEF_SYSREG(HV_SYS_REG_ID_AA64ISAR0_EL1, 3, 0, 0, 6, 0)
 #endif
 
 DEF_SYSREG(HV_SYS_REG_ID_AA64PFR1_EL1, 3, 0, 0, 4, 1)
 /* Add ID_AA64PFR2_EL1 here when HVF supports it */
 DEF_SYSREG(HV_SYS_REG_ID_AA64DFR0_EL1, 3, 0, 0, 5, 0)
 DEF_SYSREG(HV_SYS_REG_ID_AA64DFR1_EL1, 3, 0, 0, 5, 1)
-DEF_SYSREG(HV_SYS_REG_ID_AA64ISAR0_EL1, 3, 0, 0, 6, 0)
 DEF_SYSREG(HV_SYS_REG_ID_AA64ISAR1_EL1, 3, 0, 0, 6, 1)
 
 #ifdef SYNC_NO_MMFR0
-- 
2.50.1 (Apple Git-155)

On Sun, 7 Jun 2026 at 19:22, Jason Wright <wrigjl@proton.me> 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 <zenghui.yu@linux.dev>
> Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
> Signed-off-by: Jason Wright <wrigjl@proton.me>
> ---
>  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 ?

thanks
-- PMM

On Mon, Jun 08, 2026 at 02:07:03PM +0100, Peter Maydell wrote:
> On Sun, 7 Jun 2026 at 19:22, Jason Wright <wrigjl@proton.me> 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 <zenghui.yu@linux.dev>
> > Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
> > Signed-off-by: Jason Wright <wrigjl@proton.me>
> > ---
> >  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.

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.

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.

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.

The three methods track different requirements, so I matched
the closest existing pattern for ID_AA64ISAR0_EL1 and left
the others alone.

On Mon, 8 Jun 2026 at 16:56, Jason L. Wright' <wrigjl@proton.me> wrote:
>
> On Mon, Jun 08, 2026 at 02:07:03PM +0100, Peter Maydell wrote:
> > On Sun, 7 Jun 2026 at 19:22, Jason Wright <wrigjl@proton.me> 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 <zenghui.yu@linux.dev>
> > > Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
> > > Signed-off-by: Jason Wright <wrigjl@proton.me>
> > > ---
> > >  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

Hi,

On 8/6/26 18:05, Peter Maydell wrote:
> On Mon, 8 Jun 2026 at 16:56, Jason L. Wright' <wrigjl@proton.me> wrote:
>>
>> On Mon, Jun 08, 2026 at 02:07:03PM +0100, Peter Maydell wrote:
>>> On Sun, 7 Jun 2026 at 19:22, Jason Wright <wrigjl@proton.me> 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 <zenghui.yu@linux.dev>
>>>> Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
>>>> Signed-off-by: Jason Wright <wrigjl@proton.me>
>>>> ---
>>>>   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:

Should we revert the commit until this is figured 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
>

On Tue, 23 Jun 2026 at 09:27, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> Hi,
>
> On 8/6/26 18:05, Peter Maydell wrote:
> > On Mon, 8 Jun 2026 at 16:56, Jason L. Wright' <wrigjl@proton.me> wrote:
> >> 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:
>
> Should we revert the commit until this is figured out

I don't think it's difficult. Somebody who cares about hvf
should just do it, and submit this patch with whatever the right
approach is.

-- PMM

On Tue, Jun 23, 2026 at 09:42:47AM +0100, Peter Maydell wrote:
> On Tue, 23 Jun 2026 at 09:27, Philippe Mathieu-Daudé
> <philmd@oss.qualcomm.com> wrote:
> >
> > Hi,
> >
> > On 8/6/26 18:05, Peter Maydell wrote:
> > > On Mon, 8 Jun 2026 at 16:56, Jason L. Wright' <wrigjl@proton.me> wrote:
> > >> 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:
> >
> > Should we revert the commit until this is figured out
> 
> I don't think it's difficult. Somebody who cares about hvf
> should just do it, and submit this patch with whatever the right
> approach is.
> 
I'll have another look at it this week. I think we can unify the
approaches, but I need to make sure I understand the other registers
better (PFR0 and MMFR0).

--Jason Wright

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 1441.

Reproduce with:

  qemu-system-aarch64 -M virt,accel=hvf -cpu host \
                      -nographic -display none -bios /dev/null

Fix it the same way ID_AA64PFR0_EL1 already is: list
HV_SYS_REG_ID_AA64ISAR0_EL1 in the SYNC_NO_RAW_REGS block in sysreg.c.inc
so the assert loop skips it, and seed the vCPU's copy at init time.

While here, unify how the three isar.idregs[]-backed ID registers are
seeded.  isar.idregs[] already holds QEMU's intended value for each (the
host caps, probed once at realize via hv_vcpu_config_get_feature_reg(),
plus any QEMU adjustment), so there is no need to read each register back
from the vCPU first.  Seed PFR0, ISAR0 and MMFR0 directly from
isar.idregs[], dropping the two per-vCPU hv_vcpu_get_sys_reg() reads:

  - PFR0: take the GIC sysreg-interface bit from env->gicv3state, as the
    id_aa64pfr0_read() readfn does.  Identical to the previous code
    whenever a GICv3 sysreg interface is present (the configuration HVF
    runs in practice); it differs only in that a vCPU with no GICv3 now
    reports ID_AA64PFR0_EL1.GIC == 0 instead of inheriting the host's
    value, which matches the field's meaning.
  - ISAR0: no overlay is needed; HVF does not expose EL3, so
    SCR_EL3.TRNDR is never set and the readfn is constant.
  - MMFR0: still clamp PARANGE to the chosen IPA size, updating
    isar.idregs[] in place because the page-table walker and the
    ID_AA64MMFR0_EL1 cpreg resetvalue read PARANGE back from there.

Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
Signed-off-by: Jason Wright <wrigjl@proton.me>
---
v1 of this fix-up added a third way of seeding an ID register into the
vCPU at init, and Peter asked why hvf_arch_init_vcpu() should now have
three different shapes for the isar.idregs[]-backed registers, and whether
the existing per-register hv_vcpu_get_sys_reg() re-reads are needed at all.

v2 unifies the three.  isar.idregs[] is seeded once at realize from
hv_vcpu_config_get_feature_reg() (plus QEMU's own adjustments), so it
already holds the value we want to push; the init-time re-reads of PFR0
and MMFR0 are redundant.  v2 drops both and seeds PFR0, ISAR0 and MMFR0
directly from isar.idregs[].

I confirmed the re-reads are redundant by logging, at init, the seeded
isar.idregs[] value against hv_vcpu_get_sys_reg() for each register on an
Apple Silicon host (-cpu host):

  PFR0  seed=1101000010110011 live=1101000011110011  (differ in GIC only)
  MMFR0 seed=000010000f100022 live=000010000f100022  (identical)
  ISAR0 seed=0221100110212120 live=0221100110212120  (identical)

PFR0 differs only in the GIC field [27:24], which QEMU already overlays
from env->gicv3state in id_aa64pfr0_read(); MMFR0 and ISAR0 are identical.

Two things worth flagging:

  - PFR0 now takes the GIC bit purely from env->gicv3state rather than
    OR-ing it onto the host's value.  Identical for any guest with a GICv3
    sysreg interface (what HVF runs in practice); it only changes a vCPU
    with no GICv3, which now reports ID_AA64PFR0_EL1.GIC == 0 instead of
    inheriting the host bit -- arguably the correct value.

  - MMFR0 still clamps PARANGE to the chosen IPA size and writes it back
    into isar.idregs[], because the page-table walker and the
    ID_AA64MMFR0_EL1 cpreg resetvalue read PARANGE from there.  It is the
    one register that still updates isar.idregs[]; that tracks a real
    consumer, not an inconsistency.

Tested on macOS arm64 (Darwin 25.5.0), master 20553466cc:
  - HVF -cpu host vCPU init: no assertion (was SIGABRT before).
  - meson test --suite qtest-aarch64: 29/29 pass (3 skipped).
  - checkpatch clean.

v2:
  - Unify PFR0/ISAR0/MMFR0 seeding; drop the redundant
    hv_vcpu_get_sys_reg() re-reads for PFR0 and MMFR0 (Peter Maydell).
  - Take PFR0.GIC from env->gicv3state directly.
  - Reword the subject and commit message accordingly.

 target/arm/hvf/hvf.c        | 10 ++++------
 target/arm/hvf/sysreg.c.inc |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 8b902c6882..6310cfaf3e 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1478,20 +1478,18 @@ int hvf_arch_init_vcpu(CPUState *cpu)
                               arm_cpu->mp_affinity);
     assert_hvf_ok(ret);
 
-    ret = hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_ID_AA64PFR0_EL1, &pfr);
-    assert_hvf_ok(ret);
+    pfr = GET_IDREG(&arm_cpu->isar, ID_AA64PFR0);
     pfr |= env->gicv3state ? (1 << 24) : 0;
     ret = hv_vcpu_set_sys_reg(cpu->accel->fd, HV_SYS_REG_ID_AA64PFR0_EL1, pfr);
     assert_hvf_ok(ret);
 
-    /* We're limited to underlying hardware caps, override internal versions */
-    ret = hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_ID_AA64MMFR0_EL1,
-                              &arm_cpu->isar.idregs[ID_AA64MMFR0_EL1_IDX]);
+    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);
 
     clamp_id_aa64mmfr0_parange_to_ipa_size(&arm_cpu->isar);
     ret = hv_vcpu_set_sys_reg(cpu->accel->fd, HV_SYS_REG_ID_AA64MMFR0_EL1,
-                              arm_cpu->isar.idregs[ID_AA64MMFR0_EL1_IDX]);
+                              GET_IDREG(&arm_cpu->isar, ID_AA64MMFR0));
     assert_hvf_ok(ret);
 
     if (!hvf_irqchip_in_kernel()) {
diff --git a/target/arm/hvf/sysreg.c.inc b/target/arm/hvf/sysreg.c.inc
index c11dbf274e..acd5a41364 100644
--- a/target/arm/hvf/sysreg.c.inc
+++ b/target/arm/hvf/sysreg.c.inc
@@ -89,13 +89,13 @@ DEF_SYSREG(HV_SYS_REG_MDCCINT_EL1, 2, 0, 0, 2, 0)
 DEF_SYSREG(HV_SYS_REG_MIDR_EL1, 3, 0, 0, 0, 0)
 DEF_SYSREG(HV_SYS_REG_MPIDR_EL1, 3, 0, 0, 0, 5)
 DEF_SYSREG(HV_SYS_REG_ID_AA64PFR0_EL1, 3, 0, 0, 4, 0)
+DEF_SYSREG(HV_SYS_REG_ID_AA64ISAR0_EL1, 3, 0, 0, 6, 0)
 #endif
 
 DEF_SYSREG(HV_SYS_REG_ID_AA64PFR1_EL1, 3, 0, 0, 4, 1)
 /* Add ID_AA64PFR2_EL1 here when HVF supports it */
 DEF_SYSREG(HV_SYS_REG_ID_AA64DFR0_EL1, 3, 0, 0, 5, 0)
 DEF_SYSREG(HV_SYS_REG_ID_AA64DFR1_EL1, 3, 0, 0, 5, 1)
-DEF_SYSREG(HV_SYS_REG_ID_AA64ISAR0_EL1, 3, 0, 0, 6, 0)
 DEF_SYSREG(HV_SYS_REG_ID_AA64ISAR1_EL1, 3, 0, 0, 6, 1)
 
 #ifdef SYNC_NO_MMFR0
-- 
2.50.1 (Apple Git-155)

On Mon, 29 Jun 2026 at 21:52, Jason Wright <wrigjl@proton.me> 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 1441.
>
> Reproduce with:
>
>   qemu-system-aarch64 -M virt,accel=hvf -cpu host \
>                       -nographic -display none -bios /dev/null
>
> Fix it the same way ID_AA64PFR0_EL1 already is: list
> HV_SYS_REG_ID_AA64ISAR0_EL1 in the SYNC_NO_RAW_REGS block in sysreg.c.inc
> so the assert loop skips it, and seed the vCPU's copy at init time.
>
> While here, unify how the three isar.idregs[]-backed ID registers are
> seeded.  isar.idregs[] already holds QEMU's intended value for each (the
> host caps, probed once at realize via hv_vcpu_config_get_feature_reg(),
> plus any QEMU adjustment), so there is no need to read each register back
> from the vCPU first.  Seed PFR0, ISAR0 and MMFR0 directly from
> isar.idregs[], dropping the two per-vCPU hv_vcpu_get_sys_reg() reads:
>
>   - PFR0: take the GIC sysreg-interface bit from env->gicv3state, as the
>     id_aa64pfr0_read() readfn does.  Identical to the previous code
>     whenever a GICv3 sysreg interface is present (the configuration HVF
>     runs in practice); it differs only in that a vCPU with no GICv3 now
>     reports ID_AA64PFR0_EL1.GIC == 0 instead of inheriting the host's
>     value, which matches the field's meaning.
>   - ISAR0: no overlay is needed; HVF does not expose EL3, so
>     SCR_EL3.TRNDR is never set and the readfn is constant.
>   - MMFR0: still clamp PARANGE to the chosen IPA size, updating
>     isar.idregs[] in place because the page-table walker and the
>     ID_AA64MMFR0_EL1 cpreg resetvalue read PARANGE back from there.
>
> Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
> Signed-off-by: Jason Wright <wrigjl@proton.me>

Thanks for respinning this. I've applied it to target-arm.next,
so it should get upstream within the next week.

-- PMM

On 6/30/26 4:52 AM, Jason Wright 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 1441.
> 
> Reproduce with:
> 
>   qemu-system-aarch64 -M virt,accel=hvf -cpu host \
>                       -nographic -display none -bios /dev/null
> 
> Fix it the same way ID_AA64PFR0_EL1 already is: list
> HV_SYS_REG_ID_AA64ISAR0_EL1 in the SYNC_NO_RAW_REGS block in sysreg.c.inc
> so the assert loop skips it, and seed the vCPU's copy at init time.
> 
> While here, unify how the three isar.idregs[]-backed ID registers are
> seeded.  isar.idregs[] already holds QEMU's intended value for each (the
> host caps, probed once at realize via hv_vcpu_config_get_feature_reg(),
> plus any QEMU adjustment), so there is no need to read each register back
> from the vCPU first.  Seed PFR0, ISAR0 and MMFR0 directly from
> isar.idregs[], dropping the two per-vCPU hv_vcpu_get_sys_reg() reads:
> 
>   - PFR0: take the GIC sysreg-interface bit from env->gicv3state, as the
>     id_aa64pfr0_read() readfn does.  Identical to the previous code
>     whenever a GICv3 sysreg interface is present (the configuration HVF
>     runs in practice); it differs only in that a vCPU with no GICv3 now
>     reports ID_AA64PFR0_EL1.GIC == 0 instead of inheriting the host's
>     value, which matches the field's meaning.
>   - ISAR0: no overlay is needed; HVF does not expose EL3, so
>     SCR_EL3.TRNDR is never set and the readfn is constant.
>   - MMFR0: still clamp PARANGE to the chosen IPA size, updating
>     isar.idregs[] in place because the page-table walker and the
>     ID_AA64MMFR0_EL1 cpreg resetvalue read PARANGE back from there.
> 
> Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
> Signed-off-by: Jason Wright <wrigjl@proton.me>

Tested-by: Zenghui Yu <zenghui.yu@linux.dev>

Thank you for the fix!

Zenghui

On 6/29/26 13:52, Jason Wright 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 1441.
> 
> Reproduce with:
> 
>    qemu-system-aarch64 -M virt,accel=hvf -cpu host \
>                        -nographic -display none -bios /dev/null
> 
> Fix it the same way ID_AA64PFR0_EL1 already is: list
> HV_SYS_REG_ID_AA64ISAR0_EL1 in the SYNC_NO_RAW_REGS block in sysreg.c.inc
> so the assert loop skips it, and seed the vCPU's copy at init time.
> 
> While here, unify how the three isar.idregs[]-backed ID registers are
> seeded.  isar.idregs[] already holds QEMU's intended value for each (the
> host caps, probed once at realize via hv_vcpu_config_get_feature_reg(),
> plus any QEMU adjustment), so there is no need to read each register back
> from the vCPU first.  Seed PFR0, ISAR0 and MMFR0 directly from
> isar.idregs[], dropping the two per-vCPU hv_vcpu_get_sys_reg() reads:
> 
>    - PFR0: take the GIC sysreg-interface bit from env->gicv3state, as the
>      id_aa64pfr0_read() readfn does.  Identical to the previous code
>      whenever a GICv3 sysreg interface is present (the configuration HVF
>      runs in practice); it differs only in that a vCPU with no GICv3 now
>      reports ID_AA64PFR0_EL1.GIC == 0 instead of inheriting the host's
>      value, which matches the field's meaning.
>    - ISAR0: no overlay is needed; HVF does not expose EL3, so
>      SCR_EL3.TRNDR is never set and the readfn is constant.
>    - MMFR0: still clamp PARANGE to the chosen IPA size, updating
>      isar.idregs[] in place because the page-table walker and the
>      ID_AA64MMFR0_EL1 cpreg resetvalue read PARANGE back from there.
> 
> Reported-by: Zenghui Yu<zenghui.yu@linux.dev>
> Suggested-by: Peter Maydell<peter.maydell@linaro.org>
> Fixes: 887eaa8a29 ("target/arm: implement FEAT_RNG_TRAP for RNDR/RNDRRS")
> Signed-off-by: Jason Wright<wrigjl@proton.me>
> ---

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~