[PATCH v2] target/arm/hvf: seed NO_RAW ID registers from isar.idregs[] on vCPU init

Jason Wright posted 1 patch 3 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260629205213.82391-1-wrigjl@proton.me
Maintainers: Alexander Graf <agraf@csgraf.de>, Peter Maydell <peter.maydell@linaro.org>
target/arm/hvf/hvf.c        | 10 ++++------
target/arm/hvf/sysreg.c.inc |  2 +-
2 files changed, 5 insertions(+), 7 deletions(-)
[PATCH v2] target/arm/hvf: seed NO_RAW ID registers from isar.idregs[] on vCPU init
Posted by Jason Wright 3 weeks, 5 days ago
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)
Re: [PATCH v2] target/arm/hvf: seed NO_RAW ID registers from isar.idregs[] on vCPU init
Posted by Peter Maydell 3 weeks, 2 days ago
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
Re: [PATCH v2] target/arm/hvf: seed NO_RAW ID registers from isar.idregs[] on vCPU init
Posted by Zenghui Yu 3 weeks, 3 days ago
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
Re: [PATCH v2] target/arm/hvf: seed NO_RAW ID registers from isar.idregs[] on vCPU init
Posted by Richard Henderson 3 weeks, 4 days ago
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~