[PATCH 00/20] arm: Fixes and preparatory cleanups for split-accel

[PATCH 00/20] arm: Fixes and preparatory cleanups for split-accel

[Philippe Mathieu-Daudé]

Omnibus series of ARM-related patches (noticed during the
"split accel" PoC work).

- Usual prototypes cleanups
- Check TCG for EL2/EL3 features (and not !KVM or !HVF)
- Improve HVF debugging
- Correct HVF 'dtb_compatible' value for Linux
- Fix HVF GTimer frequency (My M1 hardware has 24 MHz)
  (this implies accel/ rework w.r.t. QDev vCPU REALIZE)

Regards,

Phil.

Philippe Mathieu-Daudé (20):
  target/arm: Remove arm_handle_psci_call() stub
  target/arm: Reduce arm_cpu_post_init() declaration scope
  target/arm: Unify gen_exception_internal()
  target/arm/hvf: Simplify GIC hvf_arch_init_vcpu()
  target/arm/hvf: Directly re-lock BQL after hv_vcpu_run()
  target/arm/hvf: Trace hv_vcpu_run() failures
  accel/hvf: Trace VM memory mapping
  target/arm/hvf: Log $pc in hvf_unknown_hvc() trace event
  target/arm/hvf: Correct dtb_compatible value
  target/arm: Restrict system register properties to system binary
  target/arm: Create GTimers *after* features finalized / accel realized
  accel: Keep reference to AccelOpsClass in AccelClass
  accel: Introduce AccelOpsClass::cpu_target_realize() hook
  accel/hvf: Add hvf_arch_cpu_realize() stubs
  target/arm/hvf: Really set Generic Timer counter frequency
  hw/arm/virt: Only require TCG || QTest to use TrustZone
  hw/arm/virt: Only require TCG || QTest to use virtualization extension
  hw/arm/virt: Rename cpu_post_init() -> post_cpus_gic_realized()
  hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
  tests/functional/sbsa-ref: Move where machine type is set

 meson.build                              |  1 +
 accel/hvf/trace.h                        |  2 +
 include/qemu/accel.h                     |  3 +
 include/system/accel-ops.h               |  4 +-
 include/system/hvf.h                     |  3 +
 target/arm/cpu.h                         |  2 -
 target/arm/internals.h                   |  6 +-
 target/arm/tcg/translate.h               |  1 +
 accel/accel-common.c                     |  4 ++
 accel/accel-system.c                     |  3 +-
 accel/hvf/hvf-accel-ops.c                |  8 +++
 accel/tcg/tcg-accel-ops.c                |  4 +-
 hw/arm/sbsa-ref.c                        |  8 ++-
 hw/arm/virt.c                            |  9 +--
 target/arm/cpu.c                         | 71 ++++++++++++------------
 target/arm/hvf/hvf.c                     | 46 +++++++++++----
 target/arm/tcg/translate-a64.c           |  6 --
 target/arm/tcg/translate.c               |  2 +-
 target/i386/hvf/hvf.c                    |  5 ++
 accel/hvf/trace-events                   |  7 +++
 target/arm/hvf/trace-events              |  3 +-
 tests/functional/test_aarch64_sbsaref.py |  2 +-
 22 files changed, 129 insertions(+), 71 deletions(-)
 create mode 100644 accel/hvf/trace.h
 create mode 100644 accel/hvf/trace-events

-- 
2.49.0

[PATCH 01/20] target/arm: Remove arm_handle_psci_call() stub

[Philippe Mathieu-Daudé]

Since commit 0c1aaa66c24 ("target/arm: wrap psci call with
tcg_enabled") the arm_handle_psci_call() call is elided
when TCG is disabled.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/internals.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 3360de9150f..5ed25d33208 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -650,16 +650,12 @@ static inline bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
 {
     return false;
 }
-static inline void arm_handle_psci_call(ARMCPU *cpu)
-{
-    g_assert_not_reached();
-}
 #else
 /* Return true if the r0/x0 value indicates that this SMC/HVC is a PSCI call. */
 bool arm_is_psci_call(ARMCPU *cpu, int excp_type);
+#endif
 /* Actually handle a PSCI call */
 void arm_handle_psci_call(ARMCPU *cpu);
-#endif
 
 /**
  * arm_clear_exclusive: clear the exclusive monitor
-- 
2.49.0

Re: [PATCH 01/20] target/arm: Remove arm_handle_psci_call() stub

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Since commit 0c1aaa66c24 ("target/arm: wrap psci call with
> tcg_enabled") the arm_handle_psci_call() call is elided
> when TCG is disabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/internals.h | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)

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

r~

[PATCH 02/20] target/arm: Reduce arm_cpu_post_init() declaration scope

[Philippe Mathieu-Daudé]

arm_cpu_post_init() is only used within the same file unit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu.h | 2 --
 target/arm/cpu.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 302c24e2324..c31f69912b8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1150,8 +1150,6 @@ void arm_gt_sel2vtimer_cb(void *opaque);
 unsigned int gt_cntfrq_period_ns(ARMCPU *cpu);
 void gt_rme_post_el_change(ARMCPU *cpu, void *opaque);
 
-void arm_cpu_post_init(Object *obj);
-
 #define ARM_AFF0_SHIFT 0
 #define ARM_AFF0_MASK  (0xFFULL << ARM_AFF0_SHIFT)
 #define ARM_AFF1_SHIFT 8
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e025e241eda..eb0639de719 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1713,7 +1713,7 @@ static void arm_cpu_propagate_feature_implications(ARMCPU *cpu)
     }
 }
 
-void arm_cpu_post_init(Object *obj)
+static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
 
-- 
2.49.0

Re: [PATCH 02/20] target/arm: Reduce arm_cpu_post_init() declaration scope

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> arm_cpu_post_init() is only used within the same file unit.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/arm/cpu.h | 2 --
>   target/arm/cpu.c | 2 +-
>   2 files changed, 1 insertion(+), 3 deletions(-)

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

r~

[PATCH 03/20] target/arm: Unify gen_exception_internal()

[Philippe Mathieu-Daudé]

Same code, use the generic variant.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/tcg/translate.h     | 1 +
 target/arm/tcg/translate-a64.c | 6 ------
 target/arm/tcg/translate.c     | 2 +-
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h
index 1bfdb0fb9bb..0004a97219b 100644
--- a/target/arm/tcg/translate.h
+++ b/target/arm/tcg/translate.h
@@ -347,6 +347,7 @@ void arm_jump_cc(DisasCompare *cmp, TCGLabel *label);
 void arm_gen_test_cc(int cc, TCGLabel *label);
 MemOp pow2_align(unsigned i);
 void unallocated_encoding(DisasContext *s);
+void gen_exception_internal(int excp);
 void gen_exception_insn_el(DisasContext *s, target_long pc_diff, int excp,
                            uint32_t syn, uint32_t target_el);
 void gen_exception_insn(DisasContext *s, target_long pc_diff,
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index ac80f572a2d..7c79b8c4401 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -433,12 +433,6 @@ static void gen_rebuild_hflags(DisasContext *s)
     gen_helper_rebuild_hflags_a64(tcg_env, tcg_constant_i32(s->current_el));
 }
 
-static void gen_exception_internal(int excp)
-{
-    assert(excp_is_internal(excp));
-    gen_helper_exception_internal(tcg_env, tcg_constant_i32(excp));
-}
-
 static void gen_exception_internal_insn(DisasContext *s, int excp)
 {
     gen_a64_update_pc(s, 0);
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 9962f43b1d0..f7d6d8ce196 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -372,7 +372,7 @@ static void gen_rebuild_hflags(DisasContext *s, bool new_el)
     }
 }
 
-static void gen_exception_internal(int excp)
+void gen_exception_internal(int excp)
 {
     assert(excp_is_internal(excp));
     gen_helper_exception_internal(tcg_env, tcg_constant_i32(excp));
-- 
2.49.0

Re: [PATCH 03/20] target/arm: Unify gen_exception_internal()

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Same code, use the generic variant.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/arm/tcg/translate.h     | 1 +
>   target/arm/tcg/translate-a64.c | 6 ------
>   target/arm/tcg/translate.c     | 2 +-
>   3 files changed, 2 insertions(+), 7 deletions(-)

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

r~

[PATCH 04/20] target/arm/hvf: Simplify GIC hvf_arch_init_vcpu()

[Philippe Mathieu-Daudé]

Only update the ID_AA64PFR0_EL1 register when a GIC is provided.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 42258cc2d88..06cfb7ba253 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1057,11 +1057,13 @@ 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);
+    }
 
     /* 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,
-- 
2.49.0

[PATCH 05/20] target/arm/hvf: Directly re-lock BQL after hv_vcpu_run()

[Philippe Mathieu-Daudé]

Keep bql_unlock() / bql_lock() close.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 06cfb7ba253..30dfd97bebf 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1912,7 +1912,9 @@ int hvf_vcpu_exec(CPUState *cpu)
     flush_cpu_state(cpu);
 
     bql_unlock();
-    assert_hvf_ok(hv_vcpu_run(cpu->accel->fd));
+    r = hv_vcpu_run(cpu->accel->fd);
+    bql_lock();
+    assert_hvf_ok(r);
 
     /* handle VMEXIT */
     uint64_t exit_reason = hvf_exit->reason;
@@ -1920,7 +1922,6 @@ int hvf_vcpu_exec(CPUState *cpu)
     uint32_t ec = syn_get_ec(syndrome);
 
     ret = 0;
-    bql_lock();
     switch (exit_reason) {
     case HV_EXIT_REASON_EXCEPTION:
         /* This is the main one, handle below. */
-- 
2.49.0

[PATCH 06/20] target/arm/hvf: Trace hv_vcpu_run() failures

[Philippe Mathieu-Daudé]

Allow distinguishing HV_ILLEGAL_GUEST_STATE in trace events.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c        | 10 +++++++++-
 target/arm/hvf/trace-events |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 30dfd97bebf..1ff3ff7b91a 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1914,7 +1914,15 @@ int hvf_vcpu_exec(CPUState *cpu)
     bql_unlock();
     r = hv_vcpu_run(cpu->accel->fd);
     bql_lock();
-    assert_hvf_ok(r);
+    switch (r) {
+    case HV_SUCCESS:
+        break;
+    case HV_ILLEGAL_GUEST_STATE:
+        trace_hvf_illegal_guest_state();
+        /* fall through */
+    default:
+        g_assert_not_reached();
+    }
 
     /* handle VMEXIT */
     uint64_t exit_reason = hvf_exit->reason;
diff --git a/target/arm/hvf/trace-events b/target/arm/hvf/trace-events
index 4fbbe4b45ec..a4870e0a5c4 100644
--- a/target/arm/hvf/trace-events
+++ b/target/arm/hvf/trace-events
@@ -11,3 +11,4 @@ hvf_exit(uint64_t syndrome, uint32_t ec, uint64_t pc) "exit: 0x%"PRIx64" [ec=0x%
 hvf_psci_call(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, uint32_t cpuid) "PSCI Call x0=0x%016"PRIx64" x1=0x%016"PRIx64" x2=0x%016"PRIx64" x3=0x%016"PRIx64" cpu=0x%x"
 hvf_vgic_write(const char *name, uint64_t val) "vgic write to %s [val=0x%016"PRIx64"]"
 hvf_vgic_read(const char *name, uint64_t val) "vgic read from %s [val=0x%016"PRIx64"]"
+hvf_illegal_guest_state(void) "HV_ILLEGAL_GUEST_STATE"
-- 
2.49.0

Re: [PATCH 06/20] target/arm/hvf: Trace hv_vcpu_run() failures

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Allow distinguishing HV_ILLEGAL_GUEST_STATE in trace events.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/arm/hvf/hvf.c        | 10 +++++++++-
>   target/arm/hvf/trace-events |  1 +
>   2 files changed, 10 insertions(+), 1 deletion(-)

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

r~

[PATCH 07/20] accel/hvf: Trace VM memory mapping

[Philippe Mathieu-Daudé]

Trace memory mapped / unmapped in the guest.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 meson.build               | 1 +
 accel/hvf/trace.h         | 2 ++
 accel/hvf/hvf-accel-ops.c | 6 ++++++
 accel/hvf/trace-events    | 7 +++++++
 4 files changed, 16 insertions(+)
 create mode 100644 accel/hvf/trace.h
 create mode 100644 accel/hvf/trace-events

diff --git a/meson.build b/meson.build
index 34729c2a3dd..5004678a26b 100644
--- a/meson.build
+++ b/meson.build
@@ -3633,6 +3633,7 @@ if have_block
 endif
 if have_system
   trace_events_subdirs += [
+    'accel/hvf',
     'accel/kvm',
     'audio',
     'backends',
diff --git a/accel/hvf/trace.h b/accel/hvf/trace.h
new file mode 100644
index 00000000000..83a1883343a
--- /dev/null
+++ b/accel/hvf/trace.h
@@ -0,0 +1,2 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include "trace/trace-accel_hvf.h"
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index d60446b85b8..b38977207d2 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -59,6 +59,7 @@
 #include "system/hvf_int.h"
 #include "system/runstate.h"
 #include "qemu/guest-random.h"
+#include "trace.h"
 
 HVFState *hvf_state;
 
@@ -97,6 +98,7 @@ static int do_hvf_set_memory(hvf_slot *slot, hv_memory_flags_t flags)
     if (macslot->present) {
         if (macslot->size != slot->size) {
             macslot->present = 0;
+            trace_hvf_vm_unmap(macslot->gpa_start, macslot->size);
             ret = hv_vm_unmap(macslot->gpa_start, macslot->size);
             assert_hvf_ok(ret);
         }
@@ -109,6 +111,10 @@ static int do_hvf_set_memory(hvf_slot *slot, hv_memory_flags_t flags)
     macslot->present = 1;
     macslot->gpa_start = slot->start;
     macslot->size = slot->size;
+    trace_hvf_vm_map(slot->start, slot->size, slot->mem, flags,
+                     flags & HV_MEMORY_READ ?  'R' : '-',
+                     flags & HV_MEMORY_WRITE ? 'W' : '-',
+                     flags & HV_MEMORY_EXEC ?  'E' : '-');
     ret = hv_vm_map(slot->mem, slot->start, slot->size, flags);
     assert_hvf_ok(ret);
     return 0;
diff --git a/accel/hvf/trace-events b/accel/hvf/trace-events
new file mode 100644
index 00000000000..3c11f69f305
--- /dev/null
+++ b/accel/hvf/trace-events
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# See docs/devel/tracing.rst for syntax documentation.
+
+# hvf-accel-ops.c
+hvf_vm_map(uint64_t paddr, uint64_t size, void *vaddr, uint8_t flags, const char r, const char w, const char e) "paddr:0x%016llx size:0x%08llx vaddr:%p flags:0x%02x/%c%c%c"
+hvf_vm_unmap(uint64_t paddr, uint64_t size) "paddr:0x%016llx size:0x%08llx"
-- 
2.49.0

Re: [PATCH 07/20] accel/hvf: Trace VM memory mapping

[Philippe Mathieu-Daudé]

On 19/6/25 15:13, Philippe Mathieu-Daudé wrote:
> Trace memory mapped / unmapped in the guest.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   meson.build               | 1 +
>   accel/hvf/trace.h         | 2 ++
>   accel/hvf/hvf-accel-ops.c | 6 ++++++
>   accel/hvf/trace-events    | 7 +++++++
>   4 files changed, 16 insertions(+)
>   create mode 100644 accel/hvf/trace.h
>   create mode 100644 accel/hvf/trace-events
> 
> diff --git a/meson.build b/meson.build
> index 34729c2a3dd..5004678a26b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3633,6 +3633,7 @@ if have_block
>   endif
>   if have_system
>     trace_events_subdirs += [
> +    'accel/hvf',
>       'accel/kvm',
>       'audio',
>       'backends',
> diff --git a/accel/hvf/trace.h b/accel/hvf/trace.h
> new file mode 100644
> index 00000000000..83a1883343a
> --- /dev/null
> +++ b/accel/hvf/trace.h
> @@ -0,0 +1,2 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include "trace/trace-accel_hvf.h"
> diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
> index d60446b85b8..b38977207d2 100644
> --- a/accel/hvf/hvf-accel-ops.c
> +++ b/accel/hvf/hvf-accel-ops.c
> @@ -59,6 +59,7 @@
>   #include "system/hvf_int.h"
>   #include "system/runstate.h"
>   #include "qemu/guest-random.h"
> +#include "trace.h"
>   
>   HVFState *hvf_state;
>   
> @@ -97,6 +98,7 @@ static int do_hvf_set_memory(hvf_slot *slot, hv_memory_flags_t flags)
>       if (macslot->present) {
>           if (macslot->size != slot->size) {
>               macslot->present = 0;
> +            trace_hvf_vm_unmap(macslot->gpa_start, macslot->size);
>               ret = hv_vm_unmap(macslot->gpa_start, macslot->size);
>               assert_hvf_ok(ret);
>           }
> @@ -109,6 +111,10 @@ static int do_hvf_set_memory(hvf_slot *slot, hv_memory_flags_t flags)
>       macslot->present = 1;
>       macslot->gpa_start = slot->start;
>       macslot->size = slot->size;
> +    trace_hvf_vm_map(slot->start, slot->size, slot->mem, flags,
> +                     flags & HV_MEMORY_READ ?  'R' : '-',
> +                     flags & HV_MEMORY_WRITE ? 'W' : '-',
> +                     flags & HV_MEMORY_EXEC ?  'E' : '-');
>       ret = hv_vm_map(slot->mem, slot->start, slot->size, flags);
>       assert_hvf_ok(ret);
>       return 0;
> diff --git a/accel/hvf/trace-events b/accel/hvf/trace-events
> new file mode 100644
> index 00000000000..3c11f69f305
> --- /dev/null
> +++ b/accel/hvf/trace-events
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# See docs/devel/tracing.rst for syntax documentation.
> +
> +# hvf-accel-ops.c
> +hvf_vm_map(uint64_t paddr, uint64_t size, void *vaddr, uint8_t flags, const char r, const char w, const char e) "paddr:0x%016llx size:0x%08llx vaddr:%p flags:0x%02x/%c%c%c"
> +hvf_vm_unmap(uint64_t paddr, uint64_t size) "paddr:0x%016llx size:0x%08llx"

For some reason this is still cross-built as 32-bit (although HVF isn't
supported there, see commit 7f800d34aa9c from 2021). Anyway, please read
instead (and thanks Pierrick):

hvf_vm_map(uint64_t paddr, uint64_t size, void *vaddr, uint8_t flags, 
const char r, const char w, const char e) "paddr:0x%016"PRIx64" 
size:0x%08"PRIx64" vaddr:%p flags:0x%02x/%c%c%c"
hvf_vm_unmap(uint64_t paddr, uint64_t size) "paddr:0x%016"PRIx64" 
size:0x%08"PRIx64

[PATCH 08/20] target/arm/hvf: Log $pc in hvf_unknown_hvc() trace event

[Philippe Mathieu-Daudé]

Tracing $PC for unknown HVC instructions to not have to
look at the disassembled flow of instructions.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c        | 4 ++--
 target/arm/hvf/trace-events | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 1ff3ff7b91a..ccdadce4766 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -2069,12 +2069,12 @@ int hvf_vcpu_exec(CPUState *cpu)
         cpu_synchronize_state(cpu);
         if (arm_cpu->psci_conduit == QEMU_PSCI_CONDUIT_HVC) {
             if (!hvf_handle_psci_call(cpu)) {
-                trace_hvf_unknown_hvc(env->xregs[0]);
+                trace_hvf_unknown_hvc(env->pc, env->xregs[0]);
                 /* SMCCC 1.3 section 5.2 says every unknown SMCCC call returns -1 */
                 env->xregs[0] = -1;
             }
         } else {
-            trace_hvf_unknown_hvc(env->xregs[0]);
+            trace_hvf_unknown_hvc(env->pc, env->xregs[0]);
             hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
         }
         break;
diff --git a/target/arm/hvf/trace-events b/target/arm/hvf/trace-events
index a4870e0a5c4..245338c5bf4 100644
--- a/target/arm/hvf/trace-events
+++ b/target/arm/hvf/trace-events
@@ -5,7 +5,7 @@ hvf_inject_irq(void) "injecting IRQ"
 hvf_data_abort(uint64_t pc, uint64_t va, uint64_t pa, bool isv, bool iswrite, bool s1ptw, uint32_t len, uint32_t srt) "data abort: [pc=0x%"PRIx64" va=0x%016"PRIx64" pa=0x%016"PRIx64" isv=%d iswrite=%d s1ptw=%d len=%d srt=%d]"
 hvf_sysreg_read(uint32_t reg, uint32_t op0, uint32_t op1, uint32_t crn, uint32_t crm, uint32_t op2, uint64_t val) "sysreg read 0x%08x (op0=%d op1=%d crn=%d crm=%d op2=%d) = 0x%016"PRIx64
 hvf_sysreg_write(uint32_t reg, uint32_t op0, uint32_t op1, uint32_t crn, uint32_t crm, uint32_t op2, uint64_t val) "sysreg write 0x%08x (op0=%d op1=%d crn=%d crm=%d op2=%d, val=0x%016"PRIx64")"
-hvf_unknown_hvc(uint64_t x0) "unknown HVC! 0x%016"PRIx64
+hvf_unknown_hvc(uint64_t pc, uint64_t x0) "pc=0x%"PRIx64" unknown HVC! 0x%016"PRIx64
 hvf_unknown_smc(uint64_t x0) "unknown SMC! 0x%016"PRIx64
 hvf_exit(uint64_t syndrome, uint32_t ec, uint64_t pc) "exit: 0x%"PRIx64" [ec=0x%x pc=0x%"PRIx64"]"
 hvf_psci_call(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, uint32_t cpuid) "PSCI Call x0=0x%016"PRIx64" x1=0x%016"PRIx64" x2=0x%016"PRIx64" x3=0x%016"PRIx64" cpu=0x%x"
-- 
2.49.0

Re: [PATCH 08/20] target/arm/hvf: Log $pc in hvf_unknown_hvc() trace event

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Tracing $PC for unknown HVC instructions to not have to
> look at the disassembled flow of instructions.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/arm/hvf/hvf.c        | 4 ++--
>   target/arm/hvf/trace-events | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)

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

r~

[PATCH 09/20] target/arm/hvf: Correct dtb_compatible value

[Philippe Mathieu-Daudé]

Linux kernel knows how to parse "arm,armv8", not "arm,arm-v8".

Fixes: 585df85efea ("hvf: arm: Implement -cpu host")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ccdadce4766..1fbfdaed59f 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -879,7 +879,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
     hv_vcpu_exit_t *exit;
     int i;
 
-    ahcf->dtb_compatible = "arm,arm-v8";
+    ahcf->dtb_compatible = "arm,armv8";
     ahcf->features = (1ULL << ARM_FEATURE_V8) |
                      (1ULL << ARM_FEATURE_NEON) |
                      (1ULL << ARM_FEATURE_AARCH64) |
-- 
2.49.0

Re: [PATCH 09/20] target/arm/hvf: Correct dtb_compatible value

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Linux kernel knows how to parse "arm,armv8", not "arm,arm-v8".
> 
> Fixes: 585df85efea ("hvf: arm: Implement -cpu host")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/hvf/hvf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index ccdadce4766..1fbfdaed59f 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -879,7 +879,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>       hv_vcpu_exit_t *exit;
>       int i;
>   
> -    ahcf->dtb_compatible = "arm,arm-v8";
> +    ahcf->dtb_compatible = "arm,armv8";
>       ahcf->features = (1ULL << ARM_FEATURE_V8) |
>                        (1ULL << ARM_FEATURE_NEON) |
>                        (1ULL << ARM_FEATURE_AARCH64) |

If so, it's equally wrong in kvm.


r~

[PATCH 10/20] target/arm: Restrict system register properties to system binary

[Philippe Mathieu-Daudé]

Do not expose system-specific properties on user-mode binaries.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index eb0639de719..c99981d88f8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1500,6 +1500,7 @@ static void arm_cpu_initfn(Object *obj)
  * 0 means "unset, use the default value". That default might vary depending
  * on the CPU type, and is set in the realize fn.
  */
+#ifndef CONFIG_USER_ONLY
 static const Property arm_cpu_gt_cntfrq_property =
             DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz, 0);
 
@@ -1509,7 +1510,6 @@ static const Property arm_cpu_reset_cbar_property =
 static const Property arm_cpu_reset_hivecs_property =
             DEFINE_PROP_BOOL("reset-hivecs", ARMCPU, reset_hivecs, false);
 
-#ifndef CONFIG_USER_ONLY
 static const Property arm_cpu_has_el2_property =
             DEFINE_PROP_BOOL("has_el2", ARMCPU, has_el2, true);
 
@@ -1532,6 +1532,7 @@ static const Property arm_cpu_has_neon_property =
 static const Property arm_cpu_has_dsp_property =
             DEFINE_PROP_BOOL("dsp", ARMCPU, has_dsp, true);
 
+#ifndef CONFIG_USER_ONLY
 static const Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
@@ -1544,6 +1545,7 @@ static const Property arm_cpu_pmsav7_dregion_property =
             DEFINE_PROP_UNSIGNED_NODEFAULT("pmsav7-dregion", ARMCPU,
                                            pmsav7_dregion,
                                            qdev_prop_uint32, uint32_t);
+#endif
 
 static bool arm_get_pmu(Object *obj, Error **errp)
 {
-- 
2.49.0

Re: [PATCH 10/20] target/arm: Restrict system register properties to system binary

[Philippe Mathieu-Daudé]

On 19/6/25 15:13, Philippe Mathieu-Daudé wrote:
> Do not expose system-specific properties on user-mode binaries.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/cpu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index eb0639de719..c99981d88f8 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1500,6 +1500,7 @@ static void arm_cpu_initfn(Object *obj)
>    * 0 means "unset, use the default value". That default might vary depending
>    * on the CPU type, and is set in the realize fn.
>    */
> +#ifndef CONFIG_USER_ONLY
>   static const Property arm_cpu_gt_cntfrq_property =
>               DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz, 0);
>   
> @@ -1509,7 +1510,6 @@ static const Property arm_cpu_reset_cbar_property =
>   static const Property arm_cpu_reset_hivecs_property =
>               DEFINE_PROP_BOOL("reset-hivecs", ARMCPU, reset_hivecs, false);
>   
> -#ifndef CONFIG_USER_ONLY
>   static const Property arm_cpu_has_el2_property =
>               DEFINE_PROP_BOOL("has_el2", ARMCPU, has_el2, true);
>   
> @@ -1532,6 +1532,7 @@ static const Property arm_cpu_has_neon_property =
>   static const Property arm_cpu_has_dsp_property =
>               DEFINE_PROP_BOOL("dsp", ARMCPU, has_dsp, true);
>   
> +#ifndef CONFIG_USER_ONLY
>   static const Property arm_cpu_has_mpu_property =
>               DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>   
> @@ -1544,6 +1545,7 @@ static const Property arm_cpu_pmsav7_dregion_property =
>               DEFINE_PROP_UNSIGNED_NODEFAULT("pmsav7-dregion", ARMCPU,
>                                              pmsav7_dregion,
>                                              qdev_prop_uint32, uint32_t);
> +#endif

Sorry, this patch is incomplete...

[PATCH 11/20] target/arm: Create GTimers *after* features finalized / accel realized

[Philippe Mathieu-Daudé]

Call generic (including accelerator) cpu_realize() handlers
*before* setting @gt_cntfrq_hz default

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu.c | 65 ++++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index c99981d88f8..102f45442a9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1986,26 +1986,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (!cpu->gt_cntfrq_hz) {
-        /*
-         * 0 means "the board didn't set a value, use the default". (We also
-         * get here for the CONFIG_USER_ONLY case.)
-         * ARMv8.6 and later CPUs architecturally must use a 1GHz timer; before
-         * that it was an IMPDEF choice, and QEMU initially picked 62.5MHz,
-         * which gives a 16ns tick period.
-         *
-         * We will use the back-compat value:
-         *  - for QEMU CPU types added before we standardized on 1GHz
-         *  - for versioned machine types with a version of 9.0 or earlier
-         */
-        if (arm_feature(env, ARM_FEATURE_BACKCOMPAT_CNTFRQ) ||
-            cpu->backcompat_cntfrq) {
-            cpu->gt_cntfrq_hz = GTIMER_BACKCOMPAT_HZ;
-        } else {
-            cpu->gt_cntfrq_hz = GTIMER_DEFAULT_HZ;
-        }
-    }
-
 #ifndef CONFIG_USER_ONLY
     /* The NVIC and M-profile CPU are two halves of a single piece of
      * hardware; trying to use one without the other is a command line
@@ -2052,7 +2032,40 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
             return;
         }
     }
+#endif
 
+    cpu_exec_realizefn(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    arm_cpu_finalize_features(cpu, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+#ifndef CONFIG_USER_ONLY
+    if (!cpu->gt_cntfrq_hz) {
+        /*
+         * 0 means "the board didn't set a value, use the default". (We also
+         * get here for the CONFIG_USER_ONLY case.)
+         * ARMv8.6 and later CPUs architecturally must use a 1GHz timer; before
+         * that it was an IMPDEF choice, and QEMU initially picked 62.5MHz,
+         * which gives a 16ns tick period.
+         *
+         * We will use the back-compat value:
+         *  - for QEMU CPU types added before we standardized on 1GHz
+         *  - for versioned machine types with a version of 9.0 or earlier
+         */
+        if (arm_feature(env, ARM_FEATURE_BACKCOMPAT_CNTFRQ) ||
+            cpu->backcompat_cntfrq) {
+            cpu->gt_cntfrq_hz = GTIMER_BACKCOMPAT_HZ;
+        } else {
+            cpu->gt_cntfrq_hz = GTIMER_DEFAULT_HZ;
+        }
+    }
     {
         uint64_t scale = gt_cntfrq_period_ns(cpu);
 
@@ -2073,18 +2086,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    cpu_exec_realizefn(cs, &local_err);
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    arm_cpu_finalize_features(cpu, &local_err);
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
 #ifdef CONFIG_USER_ONLY
     /*
      * User mode relies on IC IVAU instructions to catch modification of
-- 
2.49.0

[PATCH 12/20] accel: Keep reference to AccelOpsClass in AccelClass

[Philippe Mathieu-Daudé]

Allow dereferencing AccelOpsClass outside of accel/accel-system.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/accel.h       | 3 +++
 include/system/accel-ops.h | 3 ++-
 accel/accel-common.c       | 1 +
 accel/accel-system.c       | 3 ++-
 accel/tcg/tcg-accel-ops.c  | 4 +++-
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index fbd3d897fef..9dea3145429 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -37,6 +37,9 @@ typedef struct AccelClass {
     /*< public >*/
 
     const char *name;
+    /* Cached by accel_init_ops_interfaces() when created */
+    AccelOpsClass *ops;
+
     int (*init_machine)(MachineState *ms);
     bool (*cpu_common_realize)(CPUState *cpu, Error **errp);
     void (*cpu_common_unrealize)(CPUState *cpu);
diff --git a/include/system/accel-ops.h b/include/system/accel-ops.h
index 4c99d25aeff..44b37592d02 100644
--- a/include/system/accel-ops.h
+++ b/include/system/accel-ops.h
@@ -10,6 +10,7 @@
 #ifndef ACCEL_OPS_H
 #define ACCEL_OPS_H
 
+#include "qemu/accel.h"
 #include "exec/vaddr.h"
 #include "qom/object.h"
 
@@ -31,7 +32,7 @@ struct AccelOpsClass {
     /*< public >*/
 
     /* initialization function called when accel is chosen */
-    void (*ops_init)(AccelOpsClass *ops);
+    void (*ops_init)(AccelClass *ac);
 
     bool (*cpus_are_resettable)(void);
     void (*cpu_reset_hold)(CPUState *cpu);
diff --git a/accel/accel-common.c b/accel/accel-common.c
index 4894b98d64a..56d88940f92 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -10,6 +10,7 @@
 #include "qemu/osdep.h"
 #include "qemu/accel.h"
 #include "qemu/target-info.h"
+#include "system/accel-ops.h"
 #include "accel/accel-cpu.h"
 #include "accel-internal.h"
 
diff --git a/accel/accel-system.c b/accel/accel-system.c
index a0f562ae9ff..64bc991b1ce 100644
--- a/accel/accel-system.c
+++ b/accel/accel-system.c
@@ -85,8 +85,9 @@ void accel_init_ops_interfaces(AccelClass *ac)
      * non-NULL create_vcpu_thread operation.
      */
     ops = ACCEL_OPS_CLASS(oc);
+    ac->ops = ops;
     if (ops->ops_init) {
-        ops->ops_init(ops);
+        ops->ops_init(ac);
     }
     cpus_register_accel(ops);
 }
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index b24d6a75625..da2e22a7dff 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -198,8 +198,10 @@ static inline void tcg_remove_all_breakpoints(CPUState *cpu)
     cpu_watchpoint_remove_all(cpu, BP_GDB);
 }
 
-static void tcg_accel_ops_init(AccelOpsClass *ops)
+static void tcg_accel_ops_init(AccelClass *ac)
 {
+    AccelOpsClass *ops = ac->ops;
+
     if (qemu_tcg_mttcg_enabled()) {
         ops->create_vcpu_thread = mttcg_start_vcpu_thread;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
-- 
2.49.0

[PATCH 13/20] accel: Introduce AccelOpsClass::cpu_target_realize() hook

[Philippe Mathieu-Daudé]

Allow accelerators to set vCPU properties before its realization.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/accel-ops.h | 1 +
 accel/accel-common.c       | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/system/accel-ops.h b/include/system/accel-ops.h
index 44b37592d02..a863fe59388 100644
--- a/include/system/accel-ops.h
+++ b/include/system/accel-ops.h
@@ -35,6 +35,7 @@ struct AccelOpsClass {
     void (*ops_init)(AccelClass *ac);
 
     bool (*cpus_are_resettable)(void);
+    bool (*cpu_target_realize)(CPUState *cpu, Error **errp);
     void (*cpu_reset_hold)(CPUState *cpu);
 
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
diff --git a/accel/accel-common.c b/accel/accel-common.c
index 56d88940f92..55d21b63a48 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -105,6 +105,9 @@ bool accel_cpu_common_realize(CPUState *cpu, Error **errp)
     if (acc->cpu_common_realize && !acc->cpu_common_realize(cpu, errp)) {
         return false;
     }
+    if (acc->ops->cpu_target_realize && !acc->ops->cpu_target_realize(cpu, errp)) {
+        return false;
+    }
 
     return true;
 }
-- 
2.49.0

[PATCH 14/20] accel/hvf: Add hvf_arch_cpu_realize() stubs

[Philippe Mathieu-Daudé]

Implement HVF AccelOpsClass::cpu_target_realize() hook as
empty stubs. Target implementations will come separately.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/hvf.h      | 3 +++
 accel/hvf/hvf-accel-ops.c | 2 ++
 target/arm/hvf/hvf.c      | 5 +++++
 target/i386/hvf/hvf.c     | 5 +++++
 4 files changed, 15 insertions(+)

diff --git a/include/system/hvf.h b/include/system/hvf.h
index a9a502f0c8f..8c4409a13f1 100644
--- a/include/system/hvf.h
+++ b/include/system/hvf.h
@@ -72,6 +72,9 @@ void hvf_arch_update_guest_debug(CPUState *cpu);
  * Return whether the guest supports debugging.
  */
 bool hvf_arch_supports_guest_debug(void);
+
+bool hvf_arch_cpu_realize(CPUState *cpu, Error **errp);
+
 #endif /* COMPILING_PER_TARGET */
 
 #endif
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index b38977207d2..b9511103a75 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -588,6 +588,8 @@ static void hvf_accel_ops_class_init(ObjectClass *oc, const void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
+    ops->cpu_target_realize = hvf_arch_cpu_realize;
+
     ops->create_vcpu_thread = hvf_start_vcpu_thread;
     ops->kick_vcpu_thread = hvf_kick_vcpu_thread;
 
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 1fbfdaed59f..632751d1eab 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1078,6 +1078,11 @@ int hvf_arch_init_vcpu(CPUState *cpu)
     return 0;
 }
 
+bool hvf_arch_cpu_realize(CPUState *cs, Error **errp)
+{
+    return true;
+}
+
 void hvf_kick_vcpu_thread(CPUState *cpu)
 {
     cpus_kick_thread(cpu);
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 99e37a33e50..28484496710 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -367,6 +367,11 @@ int hvf_arch_init_vcpu(CPUState *cpu)
     return 0;
 }
 
+bool hvf_arch_cpu_realize(CPUState *cs, Error **errp)
+{
+    return true;
+}
+
 static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t idtvec_info)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
-- 
2.49.0

[PATCH 15/20] target/arm/hvf: Really set Generic Timer counter frequency

[Philippe Mathieu-Daudé]

Setting ARMCPU::gt_cntfrq_hz in hvf_arch_init_vcpu() is
not correct because the timers have already be initialized
with the default frequency.

Set it earlier in the AccelOpsClass::cpu_target_realize()
handler instead, and assert the value is correct when
reaching hvf_arch_init_vcpu().

Fixes: a1477da3dde ("hvf: Add Apple Silicon support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 632751d1eab..fce02d22ef9 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1008,12 +1008,16 @@ int hvf_arch_init_vcpu(CPUState *cpu)
     CPUARMState *env = &arm_cpu->env;
     uint32_t sregs_match_len = ARRAY_SIZE(hvf_sreg_match);
     uint32_t sregs_cnt = 0;
+    uint64_t freq_hz = 0;
     uint64_t pfr;
     hv_return_t ret;
     int i;
 
     env->aarch64 = true;
-    asm volatile("mrs %0, cntfrq_el0" : "=r"(arm_cpu->gt_cntfrq_hz));
+
+    /* system count frequency sanity check */
+    asm volatile("mrs %0, cntfrq_el0" : "=r"(freq_hz));
+    assert(arm_cpu->gt_cntfrq_hz == freq_hz);
 
     /* Allocate enough space for our sysreg sync */
     arm_cpu->cpreg_indexes = g_renew(uint64_t, arm_cpu->cpreg_indexes,
@@ -1080,6 +1084,10 @@ int hvf_arch_init_vcpu(CPUState *cpu)
 
 bool hvf_arch_cpu_realize(CPUState *cs, Error **errp)
 {
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    asm volatile("mrs %0, cntfrq_el0" : "=r"(cpu->gt_cntfrq_hz));
+
     return true;
 }
 
-- 
2.49.0

Re: [PATCH 15/20] target/arm/hvf: Really set Generic Timer counter frequency

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Setting ARMCPU::gt_cntfrq_hz in hvf_arch_init_vcpu() is
> not correct because the timers have already be initialized
> with the default frequency.
> 
> Set it earlier in the AccelOpsClass::cpu_target_realize()
> handler instead, and assert the value is correct when
> reaching hvf_arch_init_vcpu().
> 
> Fixes: a1477da3dde ("hvf: Add Apple Silicon support")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/hvf/hvf.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 632751d1eab..fce02d22ef9 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -1008,12 +1008,16 @@ int hvf_arch_init_vcpu(CPUState *cpu)
>       CPUARMState *env = &arm_cpu->env;
>       uint32_t sregs_match_len = ARRAY_SIZE(hvf_sreg_match);
>       uint32_t sregs_cnt = 0;
> +    uint64_t freq_hz = 0;
>       uint64_t pfr;
>       hv_return_t ret;
>       int i;
>   
>       env->aarch64 = true;
> -    asm volatile("mrs %0, cntfrq_el0" : "=r"(arm_cpu->gt_cntfrq_hz));
> +
> +    /* system count frequency sanity check */
> +    asm volatile("mrs %0, cntfrq_el0" : "=r"(freq_hz));
> +    assert(arm_cpu->gt_cntfrq_hz == freq_hz);
>   
>       /* Allocate enough space for our sysreg sync */
>       arm_cpu->cpreg_indexes = g_renew(uint64_t, arm_cpu->cpreg_indexes,
> @@ -1080,6 +1084,10 @@ int hvf_arch_init_vcpu(CPUState *cpu)
>   
>   bool hvf_arch_cpu_realize(CPUState *cs, Error **errp)
>   {
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    asm volatile("mrs %0, cntfrq_el0" : "=r"(cpu->gt_cntfrq_hz));

Worth turning the inline asm into an inline function to avoid replication?

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

r~

> +
>       return true;
>   }
>   

[PATCH 16/20] hw/arm/virt: Only require TCG || QTest to use TrustZone

[Philippe Mathieu-Daudé]

We only need TCG (or QTest) to use TrustZone, whether
KVM or HVF are used is not relevant.

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 99fde5836c9..b49d8579161 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2203,7 +2203,7 @@ static void machvirt_init(MachineState *machine)
         exit(1);
     }
 
-    if (vms->secure && (kvm_enabled() || hvf_enabled())) {
+    if (vms->secure && !tcg_enabled() && !qtest_enabled()) {
         error_report("mach-virt: %s does not support providing "
                      "Security extensions (TrustZone) to the guest CPU",
                      current_accel_name());
-- 
2.49.0

Re: [PATCH 16/20] hw/arm/virt: Only require TCG || QTest to use TrustZone

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> We only need TCG (or QTest) to use TrustZone, whether
> KVM or HVF are used is not relevant.
> 
> Reported-by: Alex Bennée<alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/virt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

[PATCH 17/20] hw/arm/virt: Only require TCG || QTest to use virtualization extension

[Philippe Mathieu-Daudé]

We can not start in EL2 / EL3 with anything but TCG (or QTest);
whether KVM or HVF are used is not relevant.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/arm/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b49d8579161..a9099570faa 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2210,7 +2210,7 @@ static void machvirt_init(MachineState *machine)
         exit(1);
     }
 
-    if (vms->virt && (kvm_enabled() || hvf_enabled())) {
+    if (vms->virt && !tcg_enabled() && !qtest_enabled()) {
         error_report("mach-virt: %s does not support providing "
                      "Virtualization extensions to the guest CPU",
                      current_accel_name());
-- 
2.49.0

[PATCH 18/20] hw/arm/virt: Rename cpu_post_init() -> post_cpus_gic_realized()

[Philippe Mathieu-Daudé]

QDev uses _post_init() during instance creation, before being
realized. Since here both vCPUs and GIC are REALIZED, rename
as virt_post_cpus_gic_realized() for clarity.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a9099570faa..da453768cce 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2032,7 +2032,8 @@ static void finalize_gic_version(VirtMachineState *vms)
  * virt_cpu_post_init() must be called after the CPUs have
  * been realized and the GIC has been created.
  */
-static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
+static void virt_post_cpus_gic_realized(VirtMachineState *vms,
+                                        MemoryRegion *sysmem)
 {
     int max_cpus = MACHINE(vms)->smp.max_cpus;
     bool aarch64, pmu, steal_time;
@@ -2349,7 +2350,7 @@ static void machvirt_init(MachineState *machine)
 
     create_gic(vms, sysmem);
 
-    virt_cpu_post_init(vms, sysmem);
+    virt_post_cpus_gic_realized(vms, sysmem);
 
     fdt_add_pmu_nodes(vms);
 
-- 
2.49.0

[PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

[Philippe Mathieu-Daudé]

Define RAMLIMIT_BYTES using the TiB definition and display
the error parsed with size_to_str():

  $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
  qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 TiB of RAM

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/sbsa-ref.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index deae5cf9861..3b7d4e7bf1d 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qemu/datadir.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -53,8 +54,7 @@
 #include "target/arm/cpu-qom.h"
 #include "target/arm/gtimer.h"
 
-#define RAMLIMIT_GB 8192
-#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
+#define RAMLIMIT_BYTES (8 * TiB)
 
 #define NUM_IRQS        256
 #define NUM_SMMU_IRQS   4
@@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
     sms->smp_cpus = smp_cpus;
 
     if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
-        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
+        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
+
+        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
         exit(1);
     }
 
-- 
2.49.0

Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

[Leif Lindholm]

On Thu, 19 Jun 2025 at 14:15, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Define RAMLIMIT_BYTES using the TiB definition and display
> the error parsed with size_to_str():
>
>   $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
>   qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 TiB of RAM
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>

/
    Leif

> ---
>  hw/arm/sbsa-ref.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index deae5cf9861..3b7d4e7bf1d 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -19,6 +19,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qemu/datadir.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> @@ -53,8 +54,7 @@
>  #include "target/arm/cpu-qom.h"
>  #include "target/arm/gtimer.h"
>
> -#define RAMLIMIT_GB 8192
> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> +#define RAMLIMIT_BYTES (8 * TiB)
>
>  #define NUM_IRQS        256
>  #define NUM_SMMU_IRQS   4
> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>      sms->smp_cpus = smp_cpus;
>
>      if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
> -        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
> +
> +        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
>          exit(1);
>      }
>
> --
> 2.49.0
>

Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

[Richard Henderson]

On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Define RAMLIMIT_BYTES using the TiB definition and display
> the error parsed with size_to_str():
> 
>    $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
>    qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 TiB of RAM
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/sbsa-ref.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index deae5cf9861..3b7d4e7bf1d 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -19,6 +19,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>   #include "qemu/datadir.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
> @@ -53,8 +54,7 @@
>   #include "target/arm/cpu-qom.h"
>   #include "target/arm/gtimer.h"
>   
> -#define RAMLIMIT_GB 8192
> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> +#define RAMLIMIT_BYTES (8 * TiB)
>   
>   #define NUM_IRQS        256
>   #define NUM_SMMU_IRQS   4
> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>       sms->smp_cpus = smp_cpus;
>   
>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
> -        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
> +
> +        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
>           exit(1);

Not a bug bug, but autofree has no effect because the block doesn't end before the call to 
exit.


r~

Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

[Philippe Mathieu-Daudé]

On 19/6/25 23:09, Richard Henderson wrote:
> On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
>> Define RAMLIMIT_BYTES using the TiB definition and display
>> the error parsed with size_to_str():
>>
>>    $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
>>    qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 
>> TiB of RAM
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/sbsa-ref.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
>> index deae5cf9861..3b7d4e7bf1d 100644
>> --- a/hw/arm/sbsa-ref.c
>> +++ b/hw/arm/sbsa-ref.c
>> @@ -19,6 +19,7 @@
>>    */
>>   #include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>>   #include "qemu/datadir.h"
>>   #include "qapi/error.h"
>>   #include "qemu/error-report.h"
>> @@ -53,8 +54,7 @@
>>   #include "target/arm/cpu-qom.h"
>>   #include "target/arm/gtimer.h"
>> -#define RAMLIMIT_GB 8192
>> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
>> +#define RAMLIMIT_BYTES (8 * TiB)
>>   #define NUM_IRQS        256
>>   #define NUM_SMMU_IRQS   4
>> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>>       sms->smp_cpus = smp_cpus;
>>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
>> -        error_report("sbsa-ref: cannot model more than %dGB RAM", 
>> RAMLIMIT_GB);
>> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
>> +
>> +        error_report("sbsa-ref: cannot model more than %s of RAM", 
>> size_str);
>>           exit(1);
> 
> Not a bug bug, but autofree has no effect because the block doesn't end 
> before the call to exit.

Right. Isn't it better to use g_autofree as a general code pattern?

Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

[Richard Henderson]

On 6/19/25 14:20, Philippe Mathieu-Daudé wrote:
>>> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>>>       sms->smp_cpus = smp_cpus;
>>>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
>>> -        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
>>> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
>>> +
>>> +        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
>>>           exit(1);
>>
>> Not a bug bug, but autofree has no effect because the block doesn't end before the call 
>> to exit.
> 
> Right. Isn't it better to use g_autofree as a general code pattern?
> 

It's a case of "this doesn't do what you think it does", which is bad form.

If you are actually interested in freeing the string to avoid a false positive during leak 
analysis, wrap the two lines in another block:


     if (...) {
         {
             g_autofree ...
             error_report(...)
         }
         exit(1);
     }


r~

Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

[Philippe Mathieu-Daudé]

On 19/6/25 23:28, Richard Henderson wrote:
> On 6/19/25 14:20, Philippe Mathieu-Daudé wrote:
>>>> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>>>>       sms->smp_cpus = smp_cpus;
>>>>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
>>>> -        error_report("sbsa-ref: cannot model more than %dGB RAM", 
>>>> RAMLIMIT_GB);
>>>> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
>>>> +
>>>> +        error_report("sbsa-ref: cannot model more than %s of RAM", 
>>>> size_str);
>>>>           exit(1);
>>>
>>> Not a bug bug, but autofree has no effect because the block doesn't 
>>> end before the call to exit.
>>
>> Right. Isn't it better to use g_autofree as a general code pattern?
>>
> 
> It's a case of "this doesn't do what you think it does", which is bad form.

I see.

> 
> If you are actually interested in freeing the string to avoid a false 
> positive during leak analysis, wrap the two lines in another block:
> 
> 
>      if (...) {
>          {
>              g_autofree ...
>              error_report(...)
>          }
>          exit(1);
>      }

Interesting, thank you!

[PATCH 20/20] tests/functional/sbsa-ref: Move where machine type is set

[Philippe Mathieu-Daudé]

fetch_firmware() is only about fetching firmware.
Set the machine type in test_sbsaref_edk2_firmware().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/functional/test_aarch64_sbsaref.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/test_aarch64_sbsaref.py b/tests/functional/test_aarch64_sbsaref.py
index e6a55aecfac..5536ec622e6 100755
--- a/tests/functional/test_aarch64_sbsaref.py
+++ b/tests/functional/test_aarch64_sbsaref.py
@@ -40,7 +40,6 @@ def fetch_firmware(test):
         with open(path, "ab+") as fd:
             fd.truncate(256 << 20)  # Expand volumes to 256MiB
 
-    test.set_machine('sbsa-ref')
     test.vm.set_console()
     test.vm.add_args(
         "-drive", f"if=pflash,file={fs0_path},format=raw",
@@ -70,6 +69,7 @@ def test_sbsaref_edk2_firmware(self):
 
         fetch_firmware(self)
 
+        self.set_machine('sbsa-ref')
         self.vm.add_args('-cpu', 'cortex-a57')
         self.vm.launch()
 
-- 
2.49.0

Re: [PATCH 20/20] tests/functional/sbsa-ref: Move where machine type is set

[Philippe Mathieu-Daudé]

On Thu, 19 Jun 2025 at 15:15, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> fetch_firmware() is only about fetching firmware.
> Set the machine type in test_sbsaref_edk2_firmware().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/functional/test_aarch64_sbsaref.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/functional/test_aarch64_sbsaref.py b/tests/functional/test_aarch64_sbsaref.py
> index e6a55aecfac..5536ec622e6 100755
> --- a/tests/functional/test_aarch64_sbsaref.py
> +++ b/tests/functional/test_aarch64_sbsaref.py
> @@ -40,7 +40,6 @@ def fetch_firmware(test):
>          with open(path, "ab+") as fd:
>              fd.truncate(256 << 20)  # Expand volumes to 256MiB
>
> -    test.set_machine('sbsa-ref')
>      test.vm.set_console()
>      test.vm.add_args(
>          "-drive", f"if=pflash,file={fs0_path},format=raw",
> @@ -70,6 +69,7 @@ def test_sbsaref_edk2_firmware(self):
>
>          fetch_firmware(self)
>
> +        self.set_machine('sbsa-ref')
>          self.vm.add_args('-cpu', 'cortex-a57')
>          self.vm.launch()

Sorry, last patch, I forgot to squash:

-- >8 --
diff --git a/tests/functional/test_aarch64_sbsaref_alpine.py
b/tests/functional/test_aarch64_sbsaref_alpine.py
index 6108ec65a54..98762eedb67 100755
--- a/tests/functional/test_aarch64_sbsaref_alpine.py
+++ b/tests/functional/test_aarch64_sbsaref_alpine.py
@@ -32,2 +32,3 @@ def boot_alpine_linux(self, cpu=None):

+        self.set_machine('sbsa-ref')
         self.vm.set_console()
diff --git a/tests/functional/test_aarch64_sbsaref_freebsd.py
b/tests/functional/test_aarch64_sbsaref_freebsd.py
index 26dfc5878bb..1c243d0d459 100755
--- a/tests/functional/test_aarch64_sbsaref_freebsd.py
+++ b/tests/functional/test_aarch64_sbsaref_freebsd.py
@@ -32,2 +32,3 @@ def boot_freebsd14(self, cpu=None):

+        self.set_machine('sbsa-ref')
         self.vm.set_console()
---

Re: [PATCH 20/20] tests/functional/sbsa-ref: Move where machine type is set

[Leif Lindholm]

On Thu, 19 Jun 2025 at 14:23, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On Thu, 19 Jun 2025 at 15:15, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >
> > fetch_firmware() is only about fetching firmware.
> > Set the machine type in test_sbsaref_edk2_firmware().
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >  tests/functional/test_aarch64_sbsaref.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/functional/test_aarch64_sbsaref.py b/tests/functional/test_aarch64_sbsaref.py
> > index e6a55aecfac..5536ec622e6 100755
> > --- a/tests/functional/test_aarch64_sbsaref.py
> > +++ b/tests/functional/test_aarch64_sbsaref.py
> > @@ -40,7 +40,6 @@ def fetch_firmware(test):
> >          with open(path, "ab+") as fd:
> >              fd.truncate(256 << 20)  # Expand volumes to 256MiB
> >
> > -    test.set_machine('sbsa-ref')
> >      test.vm.set_console()
> >      test.vm.add_args(
> >          "-drive", f"if=pflash,file={fs0_path},format=raw",
> > @@ -70,6 +69,7 @@ def test_sbsaref_edk2_firmware(self):
> >
> >          fetch_firmware(self)
> >
> > +        self.set_machine('sbsa-ref')
> >          self.vm.add_args('-cpu', 'cortex-a57')
> >          self.vm.launch()
>
> Sorry, last patch, I forgot to squash:

With the squash:
Reviewed-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>

/
    Leif
> -- >8 --
> diff --git a/tests/functional/test_aarch64_sbsaref_alpine.py
> b/tests/functional/test_aarch64_sbsaref_alpine.py
> index 6108ec65a54..98762eedb67 100755
> --- a/tests/functional/test_aarch64_sbsaref_alpine.py
> +++ b/tests/functional/test_aarch64_sbsaref_alpine.py
> @@ -32,2 +32,3 @@ def boot_alpine_linux(self, cpu=None):
>
> +        self.set_machine('sbsa-ref')
>          self.vm.set_console()
> diff --git a/tests/functional/test_aarch64_sbsaref_freebsd.py
> b/tests/functional/test_aarch64_sbsaref_freebsd.py
> index 26dfc5878bb..1c243d0d459 100755
> --- a/tests/functional/test_aarch64_sbsaref_freebsd.py
> +++ b/tests/functional/test_aarch64_sbsaref_freebsd.py
> @@ -32,2 +32,3 @@ def boot_freebsd14(self, cpu=None):
>
> +        self.set_machine('sbsa-ref')
>          self.vm.set_console()
> ---