On 1/7/25 11:39, Peter Maydell wrote:
On Mon, 23 Jun 2025 at 13:19, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:Only update the ID_AA64PFR0_EL1 register when a GIC is provided. Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> Reviewed-by: Richard Henderson <richard.hender...@linaro.org> --- target/arm/hvf/hvf.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 42258cc2d88..c1ed8b510db 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -1057,11 +1057,15 @@ 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 |= 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); + if (env->gicv3state) { + ret = hv_vcpu_get_sys_reg(cpu->accel->fd, + HV_SYS_REG_ID_AA64PFR0_EL1, &pfr); + assert_hvf_ok(ret); + pfr = FIELD_DP64(pfr, ID_AA64PFR0, GIC, 1); + ret = hv_vcpu_set_sys_reg(cpu->accel->fd, + HV_SYS_REG_ID_AA64PFR0_EL1, pfr); + assert_hvf_ok(ret); + }This doesn't seem like a simplification to me... Looking at the code, I suspect what we should really be doing is setting the GIC field to either 0 or 1 depending on whether env->gicv3state. Currently if hvf hands us an initial value with the GIC field set to 1 but we don't have a gicv3state we won't correctly clear it to 0. i.e. we should change the current pfr |= env->gicv3state ? (1 << 24) : 0; to pfr = FIELD_DP64(pfr, ID_AA64PFR0, GIC, env->gicv3state ? 1 : 0);
Good idea.
