[PATCH 00/13] single-binary: compile target/arm twice

[PATCH 00/13] single-binary: compile target/arm twice

[Pierrick Bouvier]

More work toward single-binary.
This series convert target/arm/cpu.c.

Built on {linux, windows, macos} x {x86_64, aarch64}
Fully tested on linux-x86_64
https://github.com/pbo-linaro/qemu/actions/runs/14722101993

Philippe Mathieu-Daudé (1):
  target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

Pierrick Bouvier (12):
  include/system/hvf: missing vaddr include
  meson: add common libs for target and target_system
  target/arm: move kvm stubs and remove CONFIG_KVM from kvm_arm.h
  target/arm/kvm_arm: copy definitions from kvm headers
  target/arm/kvm-stub: add missing stubs
  target/arm/cpu: remove CONFIG_KVM from arm_cpu_kvm_set_irq
  accel/hvf: add hvf_enabled() for common code
  target/arm/cpu: get endianness from cpu state
  target/arm/cpu: remove TARGET_AARCH64 around aarch64_cpu_dump_state
    common
  target/arm/cpu: remove TARGET_AARCH64 in arm_cpu_finalize_features
  target/arm/cpu: compile file twice (user, system) only
  target/arm/cpu32-stubs.c: compile file twice (user, system)

 meson.build              | 78 ++++++++++++++++++++++++-------
 include/system/hvf.h     | 15 ++++--
 target/arm/internals.h   |  6 +--
 target/arm/kvm_arm.h     | 99 +++++++---------------------------------
 accel/hvf/hvf-stub.c     |  3 ++
 target/arm/cpu.c         | 37 +++++----------
 target/arm/cpu32-stubs.c | 24 ++++++++++
 target/arm/hyp_gdbstub.c |  6 +--
 target/arm/kvm-stub.c    | 87 +++++++++++++++++++++++++++++++++++
 accel/hvf/meson.build    |  1 +
 target/arm/meson.build   | 15 ++++--
 11 files changed, 233 insertions(+), 138 deletions(-)
 create mode 100644 accel/hvf/hvf-stub.c
 create mode 100644 target/arm/cpu32-stubs.c

-- 
2.47.2

[PATCH 01/13] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

[Pierrick Bouvier]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

CPUARMState::pc is of type uint64_t.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/internals.h   | 6 +++---
 target/arm/hyp_gdbstub.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 4d3d84ffebd..c30689c9fcd 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1949,9 +1949,9 @@ extern GArray *hw_breakpoints, *hw_watchpoints;
 #define get_hw_bp(i)    (&g_array_index(hw_breakpoints, HWBreakpoint, i))
 #define get_hw_wp(i)    (&g_array_index(hw_watchpoints, HWWatchpoint, i))
 
-bool find_hw_breakpoint(CPUState *cpu, target_ulong pc);
-int insert_hw_breakpoint(target_ulong pc);
-int delete_hw_breakpoint(target_ulong pc);
+bool find_hw_breakpoint(CPUState *cpu, uint64_t pc);
+int insert_hw_breakpoint(uint64_t pc);
+int delete_hw_breakpoint(uint64_t pc);
 
 bool check_watchpoint_in_range(int i, vaddr addr);
 CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, vaddr addr);
diff --git a/target/arm/hyp_gdbstub.c b/target/arm/hyp_gdbstub.c
index 0512d67f8cf..4d8fd933868 100644
--- a/target/arm/hyp_gdbstub.c
+++ b/target/arm/hyp_gdbstub.c
@@ -54,7 +54,7 @@ GArray *hw_breakpoints, *hw_watchpoints;
  * here so future PC comparisons will work properly.
  */
 
-int insert_hw_breakpoint(target_ulong addr)
+int insert_hw_breakpoint(uint64_t addr)
 {
     HWBreakpoint brk = {
         .bcr = 0x1,                             /* BCR E=1, enable */
@@ -80,7 +80,7 @@ int insert_hw_breakpoint(target_ulong addr)
  * Delete a breakpoint and shuffle any above down
  */
 
-int delete_hw_breakpoint(target_ulong pc)
+int delete_hw_breakpoint(uint64_t pc)
 {
     int i;
     for (i = 0; i < hw_breakpoints->len; i++) {
@@ -226,7 +226,7 @@ int delete_hw_watchpoint(vaddr addr, vaddr len, int type)
     return -ENOENT;
 }
 
-bool find_hw_breakpoint(CPUState *cpu, target_ulong pc)
+bool find_hw_breakpoint(CPUState *cpu, uint64_t pc)
 {
     int i;
 
-- 
2.47.2

[PATCH 02/13] include/system/hvf: missing vaddr include

[Pierrick Bouvier]

On MacOS x86_64:
In file included from ../target/i386/hvf/x86_task.c:13:
/Users/runner/work/qemu/qemu/include/system/hvf.h:42:5: error: unknown type name 'vaddr'
    vaddr pc;
    ^
/Users/runner/work/qemu/qemu/include/system/hvf.h:43:5: error: unknown type name 'vaddr'
    vaddr saved_insn;
    ^
/Users/runner/work/qemu/qemu/include/system/hvf.h:45:5: error: type name requires a specifier or qualifier
    QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
    ^
/Users/runner/work/qemu/qemu/include/system/hvf.h:45:18: error: a parameter list without types is only allowed in a function definition
    QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
                 ^
/Users/runner/work/qemu/qemu/include/system/hvf.h:45:36: error: expected ';' at end of declaration list
    QTAILQ_ENTRY(hvf_sw_breakpoint) entry;

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/system/hvf.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/system/hvf.h b/include/system/hvf.h
index 730f927f034..356fced63e3 100644
--- a/include/system/hvf.h
+++ b/include/system/hvf.h
@@ -15,6 +15,7 @@
 
 #include "qemu/accel.h"
 #include "qom/object.h"
+#include "exec/vaddr.h"
 
 #ifdef COMPILING_PER_TARGET
 #include "cpu.h"
-- 
2.47.2

[PATCH 03/13] meson: add common libs for target and target_system

[Pierrick Bouvier]

Following what we did for hw/, we need target specific common libraries
for target. We need 2 different libraries:
- code common to a base architecture
- system code common to a base architecture

For user code, it can stay compiled per target for now.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build | 78 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/meson.build b/meson.build
index 68d36ac140f..7b2cf3cd7d1 100644
--- a/meson.build
+++ b/meson.build
@@ -3684,6 +3684,8 @@ target_arch = {}
 target_system_arch = {}
 target_user_arch = {}
 hw_common_arch = {}
+target_common_arch = {}
+target_common_system_arch = {}
 
 # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
 # that is filled in by qapi/.
@@ -4087,29 +4089,59 @@ common_all = static_library('common',
 
 # construct common libraries per base architecture
 hw_common_arch_libs = {}
+target_common_arch_libs = {}
+target_common_system_arch_libs = {}
 foreach target : target_dirs
   config_target = config_target_mak[target]
   target_base_arch = config_target['TARGET_BASE_ARCH']
+  target_inc = [include_directories('target' / target_base_arch)]
+  inc = [common_user_inc + target_inc]
 
-  # check if already generated
-  if target_base_arch in hw_common_arch_libs
-    continue
-  endif
+  # prevent common code to access cpu compile time definition,
+  # but still allow access to cpu.h
+  target_c_args = ['-DCPU_DEFS_H']
+  target_system_c_args = target_c_args + ['-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU']
 
   if target_base_arch in hw_common_arch
-    target_inc = [include_directories('target' / target_base_arch)]
-    src = hw_common_arch[target_base_arch]
-    lib = static_library(
-      'hw_' + target_base_arch,
-      build_by_default: false,
-      sources: src.all_sources() + genh,
-      include_directories: common_user_inc + target_inc,
-      implicit_include_directories: false,
-      # prevent common code to access cpu compile time
-      # definition, but still allow access to cpu.h
-      c_args: ['-DCPU_DEFS_H', '-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU'],
-      dependencies: src.all_dependencies())
-    hw_common_arch_libs += {target_base_arch: lib}
+    if target_base_arch not in hw_common_arch_libs
+      src = hw_common_arch[target_base_arch]
+      lib = static_library(
+        'hw_' + target_base_arch,
+        build_by_default: false,
+        sources: src.all_sources() + genh,
+        include_directories: inc,
+        c_args: target_system_c_args,
+        dependencies: src.all_dependencies())
+      hw_common_arch_libs += {target_base_arch: lib}
+    endif
+  endif
+
+  if target_base_arch in target_common_arch
+    if target_base_arch not in target_common_arch_libs
+      src = target_common_arch[target_base_arch]
+      lib = static_library(
+        'target_' + target_base_arch,
+        build_by_default: false,
+        sources: src.all_sources() + genh,
+        include_directories: inc,
+        c_args: target_c_args,
+        dependencies: src.all_dependencies())
+      target_common_arch_libs += {target_base_arch: lib}
+    endif
+  endif
+
+  if target_base_arch in target_common_system_arch
+    if target_base_arch not in target_common_system_arch_libs
+      src = target_common_system_arch[target_base_arch]
+      lib = static_library(
+        'target_system_' + target_base_arch,
+        build_by_default: false,
+        sources: src.all_sources() + genh,
+        include_directories: inc,
+        c_args: target_system_c_args,
+        dependencies: src.all_dependencies())
+      target_common_system_arch_libs += {target_base_arch: lib}
+    endif
   endif
 endforeach
 
@@ -4282,12 +4314,24 @@ foreach target : target_dirs
   target_common = common_ss.apply(config_target, strict: false)
   objects = [common_all.extract_objects(target_common.sources())]
   arch_deps += target_common.dependencies()
+  if target_base_arch in target_common_arch_libs
+    src = target_common_arch[target_base_arch].apply(config_target, strict: false)
+    lib = target_common_arch_libs[target_base_arch]
+    objects += lib.extract_objects(src.sources())
+    arch_deps += src.dependencies()
+  endif
   if target_type == 'system' and target_base_arch in hw_common_arch_libs
     src = hw_common_arch[target_base_arch].apply(config_target, strict: false)
     lib = hw_common_arch_libs[target_base_arch]
     objects += lib.extract_objects(src.sources())
     arch_deps += src.dependencies()
   endif
+  if target_type == 'system' and target_base_arch in target_common_system_arch_libs
+    src = target_common_system_arch[target_base_arch].apply(config_target, strict: false)
+    lib = target_common_system_arch_libs[target_base_arch]
+    objects += lib.extract_objects(src.sources())
+    arch_deps += src.dependencies()
+  endif
 
   target_specific = specific_ss.apply(config_target, strict: false)
   arch_srcs += target_specific.sources()
-- 
2.47.2

[PATCH 04/13] target/arm: move kvm stubs and remove CONFIG_KVM from kvm_arm.h

[Pierrick Bouvier]

We have to be careful to expose struct kvm_vcpu_init only when kvm is
possible, thus the additional CONFIG_KVM_IS_POSSIBLE around
kvm_arm_create_scratch_host_vcpu.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/kvm_arm.h  | 84 ++-----------------------------------------
 target/arm/kvm-stub.c | 77 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 82 deletions(-)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 05c3de8cd46..c8ddf8beb2e 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -94,7 +94,7 @@ void kvm_arm_cpu_post_load(ARMCPU *cpu);
  */
 void kvm_arm_reset_vcpu(ARMCPU *cpu);
 
-#ifdef CONFIG_KVM
+#ifdef CONFIG_KVM_IS_POSSIBLE
 /**
  * kvm_arm_create_scratch_host_vcpu:
  * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
@@ -116,6 +116,7 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu);
 bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
                                       int *fdarray,
                                       struct kvm_vcpu_init *init);
+#endif /* CONFIG_KVM_IS_POSSIBLE */
 
 /**
  * kvm_arm_destroy_scratch_host_vcpu:
@@ -221,85 +222,4 @@ int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
 
 void kvm_arm_enable_mte(Object *cpuobj, Error **errp);
 
-#else
-
-/*
- * It's safe to call these functions without KVM support.
- * They should either do nothing or return "not supported".
- */
-static inline bool kvm_arm_aarch32_supported(void)
-{
-    return false;
-}
-
-static inline bool kvm_arm_pmu_supported(void)
-{
-    return false;
-}
-
-static inline bool kvm_arm_sve_supported(void)
-{
-    return false;
-}
-
-static inline bool kvm_arm_mte_supported(void)
-{
-    return false;
-}
-
-/*
- * These functions should never actually be called without KVM support.
- */
-static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
-{
-    g_assert_not_reached();
-}
-
-static inline void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
-{
-    g_assert_not_reached();
-}
-
-static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
-{
-    g_assert_not_reached();
-}
-
-static inline int kvm_arm_vgic_probe(void)
-{
-    g_assert_not_reached();
-}
-
-static inline void kvm_arm_pmu_set_irq(ARMCPU *cpu, int irq)
-{
-    g_assert_not_reached();
-}
-
-static inline void kvm_arm_pmu_init(ARMCPU *cpu)
-{
-    g_assert_not_reached();
-}
-
-static inline void kvm_arm_pvtime_init(ARMCPU *cpu, uint64_t ipa)
-{
-    g_assert_not_reached();
-}
-
-static inline void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp)
-{
-    g_assert_not_reached();
-}
-
-static inline uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
-{
-    g_assert_not_reached();
-}
-
-static inline void kvm_arm_enable_mte(Object *cpuobj, Error **errp)
-{
-    g_assert_not_reached();
-}
-
-#endif
-
 #endif
diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c
index 965a486b320..2b73d0598c1 100644
--- a/target/arm/kvm-stub.c
+++ b/target/arm/kvm-stub.c
@@ -22,3 +22,80 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
 {
     g_assert_not_reached();
 }
+
+/*
+ * It's safe to call these functions without KVM support.
+ * They should either do nothing or return "not supported".
+ */
+bool kvm_arm_aarch32_supported(void)
+{
+    return false;
+}
+
+bool kvm_arm_pmu_supported(void)
+{
+    return false;
+}
+
+bool kvm_arm_sve_supported(void)
+{
+    return false;
+}
+
+bool kvm_arm_mte_supported(void)
+{
+    return false;
+}
+
+/*
+ * These functions should never actually be called without KVM support.
+ */
+void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
+{
+    g_assert_not_reached();
+}
+
+void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
+{
+    g_assert_not_reached();
+}
+
+int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
+{
+    g_assert_not_reached();
+}
+
+int kvm_arm_vgic_probe(void)
+{
+    g_assert_not_reached();
+}
+
+void kvm_arm_pmu_set_irq(ARMCPU *cpu, int irq)
+{
+    g_assert_not_reached();
+}
+
+void kvm_arm_pmu_init(ARMCPU *cpu)
+{
+    g_assert_not_reached();
+}
+
+void kvm_arm_pvtime_init(ARMCPU *cpu, uint64_t ipa)
+{
+    g_assert_not_reached();
+}
+
+void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp)
+{
+    g_assert_not_reached();
+}
+
+uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
+{
+    g_assert_not_reached();
+}
+
+void kvm_arm_enable_mte(Object *cpuobj, Error **errp)
+{
+    g_assert_not_reached();
+}
-- 
2.47.2

[PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers

[Pierrick Bouvier]

"linux/kvm.h" is not included for code compiled without
COMPILING_PER_TARGET, and headers are different depending architecture
(arm, arm64).
Thus we need to manually expose some definitions that will
be used by target/arm, ensuring they are the same for arm amd aarch64.

As well, we must but prudent to not redefine things if code is already
including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/kvm_arm.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index c8ddf8beb2e..eedd081064c 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -16,6 +16,21 @@
 #define KVM_ARM_VGIC_V2   (1 << 0)
 #define KVM_ARM_VGIC_V3   (1 << 1)
 
+#ifndef COMPILING_PER_TARGET
+
+/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
+ * for both architectures */
+#define KVM_ARM_IRQ_CPU_IRQ 0
+#define KVM_ARM_IRQ_CPU_FIQ 1
+#define KVM_ARM_IRQ_TYPE_CPU 0
+typedef unsigned int __u32;
+struct kvm_vcpu_init {
+    __u32 target;
+    __u32 features[7];
+};
+
+#endif /* COMPILING_PER_TARGET */
+
 /**
  * kvm_arm_register_device:
  * @mr: memory region for this device
-- 
2.47.2

[PATCH 06/13] target/arm/kvm-stub: add missing stubs

[Pierrick Bouvier]

Whem removing CONFIG_KVM from target/arm/cpu.c, we need more stubs.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/kvm-stub.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c
index 2b73d0598c1..2d369489543 100644
--- a/target/arm/kvm-stub.c
+++ b/target/arm/kvm-stub.c
@@ -99,3 +99,13 @@ void kvm_arm_enable_mte(Object *cpuobj, Error **errp)
 {
     g_assert_not_reached();
 }
+
+void kvm_arm_reset_vcpu(ARMCPU *cpu)
+{
+    g_assert_not_reached();
+}
+
+int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level)
+{
+    g_assert_not_reached();
+}
-- 
2.47.2

[PATCH 07/13] target/arm/cpu: remove CONFIG_KVM from arm_cpu_kvm_set_irq

[Pierrick Bouvier]

This function is called only under kvm_enabled(), so this is safe.
Previous commit took care to add kvm_arm_set_irq stub if needed.

We need to keep a CONFIG_KVM_IS_POSSIBLE guard because
this function uses KVM_ARM_IRQ_CPU_IRQ, KVM_ARM_IRQ_CPU_FIQ and
KVM_ARM_IRQ_TYPE_CPU which are only available in this context.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5e951675c60..e7a15ade8b4 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1101,7 +1101,7 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
 
 static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
 {
-#ifdef CONFIG_KVM
+#ifdef CONFIG_KVM_IS_POSSIBLE
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
     CPUState *cs = CPU(cpu);
-- 
2.47.2

[PATCH 08/13] accel/hvf: add hvf_enabled() for common code

[Pierrick Bouvier]

Other accelerators define a CONFIG_{accel}_IS_POSSIBLE when
COMPILING_PER_TARGET is not defined, except hvf.

Without this change, target/arm/cpu.c can't find hvf_enabled.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/system/hvf.h  | 14 +++++++++-----
 accel/hvf/hvf-stub.c  |  3 +++
 accel/hvf/meson.build |  1 +
 3 files changed, 13 insertions(+), 5 deletions(-)
 create mode 100644 accel/hvf/hvf-stub.c

diff --git a/include/system/hvf.h b/include/system/hvf.h
index 356fced63e3..1ee2a4177d9 100644
--- a/include/system/hvf.h
+++ b/include/system/hvf.h
@@ -19,15 +19,19 @@
 
 #ifdef COMPILING_PER_TARGET
 #include "cpu.h"
+# ifdef CONFIG_HVF
+#  define CONFIG_HVF_IS_POSSIBLE
+# endif
+#else
+# define CONFIG_HVF_IS_POSSIBLE
+#endif
 
-#ifdef CONFIG_HVF
+#ifdef CONFIG_HVF_IS_POSSIBLE
 extern bool hvf_allowed;
 #define hvf_enabled() (hvf_allowed)
-#else /* !CONFIG_HVF */
+#else
 #define hvf_enabled() 0
-#endif /* !CONFIG_HVF */
-
-#endif /* COMPILING_PER_TARGET */
+#endif /* CONFIG_HVF_IS_POSSIBLE */
 
 #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
 
diff --git a/accel/hvf/hvf-stub.c b/accel/hvf/hvf-stub.c
new file mode 100644
index 00000000000..22e2f50c01d
--- /dev/null
+++ b/accel/hvf/hvf-stub.c
@@ -0,0 +1,3 @@
+#include "qemu/osdep.h"
+
+bool hvf_allowed;
diff --git a/accel/hvf/meson.build b/accel/hvf/meson.build
index fc52cb78433..7745b94e50f 100644
--- a/accel/hvf/meson.build
+++ b/accel/hvf/meson.build
@@ -5,3 +5,4 @@ hvf_ss.add(files(
 ))
 
 specific_ss.add_all(when: 'CONFIG_HVF', if_true: hvf_ss)
+common_ss.add(when: 'CONFIG_HVF', if_false: files('hvf-stub.c'))
-- 
2.47.2

[PATCH 09/13] target/arm/cpu: get endianness from cpu state

[Pierrick Bouvier]

Remove TARGET_BIG_ENDIAN dependency.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/cpu.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e7a15ade8b4..85e886944f6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -67,6 +67,15 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
     }
 }
 
+static bool arm_cpu_is_big_endian(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    cpu_synchronize_state(cs);
+    return arm_cpu_data_is_big_endian(env);
+}
+
 static vaddr arm_cpu_get_pc(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -1130,15 +1139,6 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
 #endif
 }
 
-static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    cpu_synchronize_state(cs);
-    return arm_cpu_data_is_big_endian(env);
-}
-
 #ifdef CONFIG_TCG
 bool arm_cpu_exec_halt(CPUState *cs)
 {
@@ -1203,7 +1203,7 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 
     info->endian = BFD_ENDIAN_LITTLE;
     if (bswap_code(sctlr_b)) {
-        info->endian = TARGET_BIG_ENDIAN ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
+        info->endian = arm_cpu_is_big_endian(cpu) ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
     }
     info->flags &= ~INSN_ARM_BE32;
 #ifndef CONFIG_USER_ONLY
@@ -2681,7 +2681,7 @@ static const struct SysemuCPUOps arm_sysemu_ops = {
     .asidx_from_attrs = arm_asidx_from_attrs,
     .write_elf32_note = arm_cpu_write_elf32_note,
     .write_elf64_note = arm_cpu_write_elf64_note,
-    .virtio_is_big_endian = arm_cpu_virtio_is_big_endian,
+    .virtio_is_big_endian = arm_cpu_is_big_endian,
     .legacy_vmsd = &vmstate_arm_cpu,
 };
 #endif
-- 
2.47.2

[PATCH 10/13] target/arm/cpu: remove TARGET_AARCH64 around aarch64_cpu_dump_state common

[Pierrick Bouvier]

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/cpu.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 85e886944f6..48ebaf614ee 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1213,8 +1213,6 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 #endif
 }
 
-#ifdef TARGET_AARCH64
-
 static void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -1372,15 +1370,6 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 }
 
-#else
-
-static inline void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
-{
-    g_assert_not_reached();
-}
-
-#endif
-
 static void arm_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 {
     ARMCPU *cpu = ARM_CPU(cs);
-- 
2.47.2

[PATCH 11/13] target/arm/cpu: remove TARGET_AARCH64 in arm_cpu_finalize_features

[Pierrick Bouvier]

Need to stub cpu64 finalize functions.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/cpu.c         |  2 --
 target/arm/cpu32-stubs.c | 24 ++++++++++++++++++++++++
 target/arm/meson.build   | 11 +++++++----
 3 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 target/arm/cpu32-stubs.c

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 48ebaf614ee..79a853609dd 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1908,7 +1908,6 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
 {
     Error *local_err = NULL;
 
-#ifdef TARGET_AARCH64
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
         arm_cpu_sve_finalize(cpu, &local_err);
         if (local_err != NULL) {
@@ -1944,7 +1943,6 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
             return;
         }
     }
-#endif
 
     if (kvm_enabled()) {
         kvm_arm_steal_time_finalize(cpu, &local_err);
diff --git a/target/arm/cpu32-stubs.c b/target/arm/cpu32-stubs.c
new file mode 100644
index 00000000000..fda7ccee4b5
--- /dev/null
+++ b/target/arm/cpu32-stubs.c
@@ -0,0 +1,24 @@
+#include "qemu/osdep.h"
+#include "target/arm/cpu.h"
+#include "target/arm/internals.h"
+#include <glib.h>
+
+void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp)
+{
+    g_assert_not_reached();
+}
+
+void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
+{
+    g_assert_not_reached();
+}
+
+void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
+{
+    g_assert_not_reached();
+}
+
+void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp)
+{
+    g_assert_not_reached();
+}
diff --git a/target/arm/meson.build b/target/arm/meson.build
index 3065081d241..c39ddc4427b 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -11,10 +11,13 @@ arm_ss.add(zlib)
 arm_ss.add(when: 'CONFIG_KVM', if_true: files('hyp_gdbstub.c', 'kvm.c'), if_false: files('kvm-stub.c'))
 arm_ss.add(when: 'CONFIG_HVF', if_true: files('hyp_gdbstub.c'))
 
-arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
-  'cpu64.c',
-  'gdbstub64.c',
-))
+arm_ss.add(when: 'TARGET_AARCH64',
+  if_true: files(
+    'cpu64.c',
+    'gdbstub64.c'),
+  if_false: files(
+    'cpu32-stubs.c'),
+)
 
 arm_system_ss = ss.source_set()
 arm_system_ss.add(files(
-- 
2.47.2

[PATCH 12/13] target/arm/cpu: compile file twice (user, system) only

[Pierrick Bouvier]

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/arm/meson.build b/target/arm/meson.build
index c39ddc4427b..89e305eb56a 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -1,6 +1,6 @@
 arm_ss = ss.source_set()
+arm_common_ss = ss.source_set()
 arm_ss.add(files(
-  'cpu.c',
   'debug_helper.c',
   'gdbstub.c',
   'helper.c',
@@ -20,6 +20,7 @@ arm_ss.add(when: 'TARGET_AARCH64',
 )
 
 arm_system_ss = ss.source_set()
+arm_common_system_ss = ss.source_set()
 arm_system_ss.add(files(
   'arch_dump.c',
   'arm-powerctl.c',
@@ -30,6 +31,9 @@ arm_system_ss.add(files(
 ))
 
 arm_user_ss = ss.source_set()
+arm_user_ss.add(files('cpu.c'))
+
+arm_common_system_ss.add(files('cpu.c'), capstone)
 
 subdir('hvf')
 
@@ -42,3 +46,5 @@ endif
 target_arch += {'arm': arm_ss}
 target_system_arch += {'arm': arm_system_ss}
 target_user_arch += {'arm': arm_user_ss}
+target_common_arch += {'arm': arm_common_ss}
+target_common_system_arch += {'arm': arm_common_system_ss}
-- 
2.47.2

[PATCH 13/13] target/arm/cpu32-stubs.c: compile file twice (user, system)

[Pierrick Bouvier]

It could be squashed with commit introducing it, but I would prefer to
introduce target/arm/cpu.c first.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/meson.build | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/arm/meson.build b/target/arm/meson.build
index 89e305eb56a..de214fe5d56 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -11,13 +11,9 @@ arm_ss.add(zlib)
 arm_ss.add(when: 'CONFIG_KVM', if_true: files('hyp_gdbstub.c', 'kvm.c'), if_false: files('kvm-stub.c'))
 arm_ss.add(when: 'CONFIG_HVF', if_true: files('hyp_gdbstub.c'))
 
-arm_ss.add(when: 'TARGET_AARCH64',
-  if_true: files(
-    'cpu64.c',
-    'gdbstub64.c'),
-  if_false: files(
-    'cpu32-stubs.c'),
-)
+arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
+  'cpu64.c',
+  'gdbstub64.c'))
 
 arm_system_ss = ss.source_set()
 arm_common_system_ss = ss.source_set()
@@ -32,8 +28,12 @@ arm_system_ss.add(files(
 
 arm_user_ss = ss.source_set()
 arm_user_ss.add(files('cpu.c'))
+arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files(
+  'cpu32-stubs.c'))
 
 arm_common_system_ss.add(files('cpu.c'), capstone)
+arm_common_system_ss.add(when: 'TARGET_AARCH64', if_false: files(
+  'cpu32-stubs.c'))
 
 subdir('hvf')
 
-- 
2.47.2

Re: [PATCH 02/13] include/system/hvf: missing vaddr include

[Philippe Mathieu-Daudé]

On 29/4/25 06:59, Pierrick Bouvier wrote:
> On MacOS x86_64:
> In file included from ../target/i386/hvf/x86_task.c:13:
> /Users/runner/work/qemu/qemu/include/system/hvf.h:42:5: error: unknown type name 'vaddr'
>      vaddr pc;
>      ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:43:5: error: unknown type name 'vaddr'
>      vaddr saved_insn;
>      ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:5: error: type name requires a specifier or qualifier
>      QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>      ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:18: error: a parameter list without types is only allowed in a function definition
>      QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>                   ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:36: error: expected ';' at end of declaration list
>      QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/system/hvf.h | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 02/13] include/system/hvf: missing vaddr include

[Philippe Mathieu-Daudé]

Hi Pierrick,

On 29/4/25 06:59, Pierrick Bouvier wrote:
> On MacOS x86_64:
> In file included from ../target/i386/hvf/x86_task.c:13:
> /Users/runner/work/qemu/qemu/include/system/hvf.h:42:5: error: unknown type name 'vaddr'
>      vaddr pc;
>      ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:43:5: error: unknown type name 'vaddr'
>      vaddr saved_insn;
>      ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:5: error: type name requires a specifier or qualifier
>      QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>      ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:18: error: a parameter list without types is only allowed in a function definition
>      QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>                   ^
> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:36: error: expected ';' at end of declaration list
>      QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/system/hvf.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/system/hvf.h b/include/system/hvf.h
> index 730f927f034..356fced63e3 100644
> --- a/include/system/hvf.h
> +++ b/include/system/hvf.h
> @@ -15,6 +15,7 @@
>   
>   #include "qemu/accel.h"
>   #include "qom/object.h"
> +#include "exec/vaddr.h"
>   
>   #ifdef COMPILING_PER_TARGET
>   #include "cpu.h"

What do you think of these changes instead?

https://lore.kernel.org/qemu-devel/20250403235821.9909-27-philmd@linaro.org/

Re: [PATCH 01/13] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> CPUARMState::pc is of type uint64_t.
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> "linux/kvm.h" is not included for code compiled without
> COMPILING_PER_TARGET, and headers are different depending architecture
> (arm, arm64).
> Thus we need to manually expose some definitions that will
> be used by target/arm, ensuring they are the same for arm amd aarch64.
>
> As well, we must but prudent to not redefine things if code is already
> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  target/arm/kvm_arm.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index c8ddf8beb2e..eedd081064c 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -16,6 +16,21 @@
>  #define KVM_ARM_VGIC_V2   (1 << 0)
>  #define KVM_ARM_VGIC_V3   (1 << 1)
>  
> +#ifndef COMPILING_PER_TARGET
> +
> +/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
> + * for both architectures */
> +#define KVM_ARM_IRQ_CPU_IRQ 0
> +#define KVM_ARM_IRQ_CPU_FIQ 1
> +#define KVM_ARM_IRQ_TYPE_CPU 0
> +typedef unsigned int __u32;
> +struct kvm_vcpu_init {
> +    __u32 target;
> +    __u32 features[7];
> +};
> +
> +#endif /* COMPILING_PER_TARGET */
> +

I'm not keen on the duplication. It seems to be the only reason we have
struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
only *external* user passes in a NULL.

If kvm_arm_create_scratch_host_vcpu() is made internal static to
target/arm/kvm.c which will should always include the real linux headers
you just need a QMP helper.

For the IRQ types is this just a sign of target/arm/cpu.c needing
splitting into TCG and KVM bits?


>  /**
>   * kvm_arm_register_device:
>   * @mr: memory region for this device

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 09/13] target/arm/cpu: get endianness from cpu state

[Anton Johansson via]

On 29/04/25, Pierrick Bouvier wrote:
> Remove TARGET_BIG_ENDIAN dependency.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  target/arm/cpu.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index e7a15ade8b4..85e886944f6 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -67,6 +67,15 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
>      }
>  }
>  
> +static bool arm_cpu_is_big_endian(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    cpu_synchronize_state(cs);
> +    return arm_cpu_data_is_big_endian(env);
> +}
> +
>  static vaddr arm_cpu_get_pc(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
> @@ -1130,15 +1139,6 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
>  #endif
>  }
>  
> -static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -
> -    cpu_synchronize_state(cs);
> -    return arm_cpu_data_is_big_endian(env);
> -}
> -
>  #ifdef CONFIG_TCG
>  bool arm_cpu_exec_halt(CPUState *cs)
>  {
> @@ -1203,7 +1203,7 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
>  
>      info->endian = BFD_ENDIAN_LITTLE;
>      if (bswap_code(sctlr_b)) {
> -        info->endian = TARGET_BIG_ENDIAN ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
> +        info->endian = arm_cpu_is_big_endian(cpu) ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
>      }

I'm not the most familiar with arm but as far as I can tell these are
not equivalent. My understanding is that arm_cpu_is_big_endian() models
data endianness, and TARGET_BIG_ENDIAN instruction endianness.

Also, for user mode where this branch is relevant, bswap_code() still
depends on TARGET_BIG_ENDIAN anyway and the above branch would reduce to
(on arm32)

  if (TARGET_BIG_ENDIAN ^ sctlr_b) {
      info->endian = sctlr_b ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
  }

giving the opposite result to the original code.

-- 
Anton Johansson
rev.ng Labs Srl.

Re: [PATCH 03/13] meson: add common libs for target and target_system

[Philippe Mathieu-Daudé]

Hi Pierrick,

On 29/4/25 07:00, Pierrick Bouvier wrote:
> Following what we did for hw/, we need target specific common libraries
> for target. We need 2 different libraries:
> - code common to a base architecture
> - system code common to a base architecture
> 
> For user code, it can stay compiled per target for now.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   meson.build | 78 +++++++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 61 insertions(+), 17 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 68d36ac140f..7b2cf3cd7d1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3684,6 +3684,8 @@ target_arch = {}
>   target_system_arch = {}
>   target_user_arch = {}
>   hw_common_arch = {}
> +target_common_arch = {}
> +target_common_system_arch = {}
>   
>   # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
>   # that is filled in by qapi/.
> @@ -4087,29 +4089,59 @@ common_all = static_library('common',
>   
>   # construct common libraries per base architecture
>   hw_common_arch_libs = {}
> +target_common_arch_libs = {}
> +target_common_system_arch_libs = {}
>   foreach target : target_dirs
>     config_target = config_target_mak[target]
>     target_base_arch = config_target['TARGET_BASE_ARCH']
> +  target_inc = [include_directories('target' / target_base_arch)]
> +  inc = [common_user_inc + target_inc]
>   
> -  # check if already generated
> -  if target_base_arch in hw_common_arch_libs
> -    continue
> -  endif
> +  # prevent common code to access cpu compile time definition,
> +  # but still allow access to cpu.h
> +  target_c_args = ['-DCPU_DEFS_H']
> +  target_system_c_args = target_c_args + ['-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU']
>   
>     if target_base_arch in hw_common_arch
> -    target_inc = [include_directories('target' / target_base_arch)]
> -    src = hw_common_arch[target_base_arch]
> -    lib = static_library(
> -      'hw_' + target_base_arch,
> -      build_by_default: false,
> -      sources: src.all_sources() + genh,
> -      include_directories: common_user_inc + target_inc,
> -      implicit_include_directories: false,
> -      # prevent common code to access cpu compile time
> -      # definition, but still allow access to cpu.h
> -      c_args: ['-DCPU_DEFS_H', '-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU'],
> -      dependencies: src.all_dependencies())
> -    hw_common_arch_libs += {target_base_arch: lib}
> +    if target_base_arch not in hw_common_arch_libs
> +      src = hw_common_arch[target_base_arch]
> +      lib = static_library(
> +        'hw_' + target_base_arch,
> +        build_by_default: false,
> +        sources: src.all_sources() + genh,
> +        include_directories: inc,
> +        c_args: target_system_c_args,
> +        dependencies: src.all_dependencies())
> +      hw_common_arch_libs += {target_base_arch: lib}
> +    endif
> +  endif
> +
> +  if target_base_arch in target_common_arch
> +    if target_base_arch not in target_common_arch_libs
> +      src = target_common_arch[target_base_arch]
> +      lib = static_library(
> +        'target_' + target_base_arch,
> +        build_by_default: false,
> +        sources: src.all_sources() + genh,
> +        include_directories: inc,
> +        c_args: target_c_args,
> +        dependencies: src.all_dependencies())
> +      target_common_arch_libs += {target_base_arch: lib}
> +    endif
> +  endif
> +
> +  if target_base_arch in target_common_system_arch
> +    if target_base_arch not in target_common_system_arch_libs
> +      src = target_common_system_arch[target_base_arch]
> +      lib = static_library(
> +        'target_system_' + target_base_arch,
> +        build_by_default: false,
> +        sources: src.all_sources() + genh,
> +        include_directories: inc,
> +        c_args: target_system_c_args,
> +        dependencies: src.all_dependencies())
> +      target_common_system_arch_libs += {target_base_arch: lib}
> +    endif
>     endif
>   endforeach
>   
> @@ -4282,12 +4314,24 @@ foreach target : target_dirs
>     target_common = common_ss.apply(config_target, strict: false)
>     objects = [common_all.extract_objects(target_common.sources())]
>     arch_deps += target_common.dependencies()
> +  if target_base_arch in target_common_arch_libs
> +    src = target_common_arch[target_base_arch].apply(config_target, strict: false)
> +    lib = target_common_arch_libs[target_base_arch]
> +    objects += lib.extract_objects(src.sources())
> +    arch_deps += src.dependencies()
> +  endif
>     if target_type == 'system' and target_base_arch in hw_common_arch_libs
>       src = hw_common_arch[target_base_arch].apply(config_target, strict: false)
>       lib = hw_common_arch_libs[target_base_arch]
>       objects += lib.extract_objects(src.sources())
>       arch_deps += src.dependencies()
>     endif
> +  if target_type == 'system' and target_base_arch in target_common_system_arch_libs
> +    src = target_common_system_arch[target_base_arch].apply(config_target, strict: false)
> +    lib = target_common_system_arch_libs[target_base_arch]
> +    objects += lib.extract_objects(src.sources())
> +    arch_deps += src.dependencies()
> +  endif
>   
>     target_specific = specific_ss.apply(config_target, strict: false)
>     arch_srcs += target_specific.sources()

Somehow related to this patch, when converting from target_system_arch
to target_common_system_arch, emptying it, I get:

../../meson.build:4237:27: ERROR: Key microblaze is not in the dictionary.

4235   if target.endswith('-softmmu')
4236     target_type='system'
4237     t = target_system_arch[target_base_arch].apply(config_target, 
strict: false)

Re: [PATCH 09/13] target/arm/cpu: get endianness from cpu state

[Pierrick Bouvier]

On 4/29/25 5:26 AM, Anton Johansson wrote:
> On 29/04/25, Pierrick Bouvier wrote:
>> Remove TARGET_BIG_ENDIAN dependency.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   target/arm/cpu.c | 22 +++++++++++-----------
>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index e7a15ade8b4..85e886944f6 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -67,6 +67,15 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
>>       }
>>   }
>>   
>> +static bool arm_cpu_is_big_endian(CPUState *cs)
>> +{
>> +    ARMCPU *cpu = ARM_CPU(cs);
>> +    CPUARMState *env = &cpu->env;
>> +
>> +    cpu_synchronize_state(cs);
>> +    return arm_cpu_data_is_big_endian(env);
>> +}
>> +
>>   static vaddr arm_cpu_get_pc(CPUState *cs)
>>   {
>>       ARMCPU *cpu = ARM_CPU(cs);
>> @@ -1130,15 +1139,6 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
>>   #endif
>>   }
>>   
>> -static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
>> -{
>> -    ARMCPU *cpu = ARM_CPU(cs);
>> -    CPUARMState *env = &cpu->env;
>> -
>> -    cpu_synchronize_state(cs);
>> -    return arm_cpu_data_is_big_endian(env);
>> -}
>> -
>>   #ifdef CONFIG_TCG
>>   bool arm_cpu_exec_halt(CPUState *cs)
>>   {
>> @@ -1203,7 +1203,7 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
>>   
>>       info->endian = BFD_ENDIAN_LITTLE;
>>       if (bswap_code(sctlr_b)) {
>> -        info->endian = TARGET_BIG_ENDIAN ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
>> +        info->endian = arm_cpu_is_big_endian(cpu) ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
>>       }
> 
> I'm not the most familiar with arm but as far as I can tell these are
> not equivalent. My understanding is that arm_cpu_is_big_endian() models
> data endianness, and TARGET_BIG_ENDIAN instruction endianness.
> 
> Also, for user mode where this branch is relevant, bswap_code() still
> depends on TARGET_BIG_ENDIAN anyway and the above branch would reduce to
> (on arm32)
> 
>    if (TARGET_BIG_ENDIAN ^ sctlr_b) {
>        info->endian = sctlr_b ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
>    }
> 
> giving the opposite result to the original code.
> 

Ooops, that's a good point, I missed it was calling 
arm_cpu_data_is_big_endian under the hoods.

I'll stick to target_big_endian().

Thanks,
Pierrick

Re: [PATCH 02/13] include/system/hvf: missing vaddr include

[Pierrick Bouvier]

On 4/29/25 12:13 AM, Philippe Mathieu-Daudé wrote:
> Hi Pierrick,
> 
> On 29/4/25 06:59, Pierrick Bouvier wrote:
>> On MacOS x86_64:
>> In file included from ../target/i386/hvf/x86_task.c:13:
>> /Users/runner/work/qemu/qemu/include/system/hvf.h:42:5: error: unknown type name 'vaddr'
>>       vaddr pc;
>>       ^
>> /Users/runner/work/qemu/qemu/include/system/hvf.h:43:5: error: unknown type name 'vaddr'
>>       vaddr saved_insn;
>>       ^
>> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:5: error: type name requires a specifier or qualifier
>>       QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>>       ^
>> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:18: error: a parameter list without types is only allowed in a function definition
>>       QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>>                    ^
>> /Users/runner/work/qemu/qemu/include/system/hvf.h:45:36: error: expected ';' at end of declaration list
>>       QTAILQ_ENTRY(hvf_sw_breakpoint) entry;
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    include/system/hvf.h | 1 +
>>    1 file changed, 1 insertion(+)
>>
>> diff --git a/include/system/hvf.h b/include/system/hvf.h
>> index 730f927f034..356fced63e3 100644
>> --- a/include/system/hvf.h
>> +++ b/include/system/hvf.h
>> @@ -15,6 +15,7 @@
>>    
>>    #include "qemu/accel.h"
>>    #include "qom/object.h"
>> +#include "exec/vaddr.h"
>>    
>>    #ifdef COMPILING_PER_TARGET
>>    #include "cpu.h"
> 
> What do you think of these changes instead?
> 
> https://lore.kernel.org/qemu-devel/20250403235821.9909-27-philmd@linaro.org/

Sounds good to me, it's the right include set.
I tried to remove cpu.h, and noticed the error, so readded it, without 
investigating too much.

Feel free to merge the current patch on your side (or the version you 
wrote, it's ok for me).

Re: [PATCH 03/13] meson: add common libs for target and target_system

[Pierrick Bouvier]

On 4/29/25 11:01 AM, Philippe Mathieu-Daudé wrote:
> Hi Pierrick,
> 
> On 29/4/25 07:00, Pierrick Bouvier wrote:
>> Following what we did for hw/, we need target specific common libraries
>> for target. We need 2 different libraries:
>> - code common to a base architecture
>> - system code common to a base architecture
>>
>> For user code, it can stay compiled per target for now.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    meson.build | 78 +++++++++++++++++++++++++++++++++++++++++------------
>>    1 file changed, 61 insertions(+), 17 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 68d36ac140f..7b2cf3cd7d1 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -3684,6 +3684,8 @@ target_arch = {}
>>    target_system_arch = {}
>>    target_user_arch = {}
>>    hw_common_arch = {}
>> +target_common_arch = {}
>> +target_common_system_arch = {}
>>    
>>    # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
>>    # that is filled in by qapi/.
>> @@ -4087,29 +4089,59 @@ common_all = static_library('common',
>>    
>>    # construct common libraries per base architecture
>>    hw_common_arch_libs = {}
>> +target_common_arch_libs = {}
>> +target_common_system_arch_libs = {}
>>    foreach target : target_dirs
>>      config_target = config_target_mak[target]
>>      target_base_arch = config_target['TARGET_BASE_ARCH']
>> +  target_inc = [include_directories('target' / target_base_arch)]
>> +  inc = [common_user_inc + target_inc]
>>    
>> -  # check if already generated
>> -  if target_base_arch in hw_common_arch_libs
>> -    continue
>> -  endif
>> +  # prevent common code to access cpu compile time definition,
>> +  # but still allow access to cpu.h
>> +  target_c_args = ['-DCPU_DEFS_H']
>> +  target_system_c_args = target_c_args + ['-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU']
>>    
>>      if target_base_arch in hw_common_arch
>> -    target_inc = [include_directories('target' / target_base_arch)]
>> -    src = hw_common_arch[target_base_arch]
>> -    lib = static_library(
>> -      'hw_' + target_base_arch,
>> -      build_by_default: false,
>> -      sources: src.all_sources() + genh,
>> -      include_directories: common_user_inc + target_inc,
>> -      implicit_include_directories: false,
>> -      # prevent common code to access cpu compile time
>> -      # definition, but still allow access to cpu.h
>> -      c_args: ['-DCPU_DEFS_H', '-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU'],
>> -      dependencies: src.all_dependencies())
>> -    hw_common_arch_libs += {target_base_arch: lib}
>> +    if target_base_arch not in hw_common_arch_libs
>> +      src = hw_common_arch[target_base_arch]
>> +      lib = static_library(
>> +        'hw_' + target_base_arch,
>> +        build_by_default: false,
>> +        sources: src.all_sources() + genh,
>> +        include_directories: inc,
>> +        c_args: target_system_c_args,
>> +        dependencies: src.all_dependencies())
>> +      hw_common_arch_libs += {target_base_arch: lib}
>> +    endif
>> +  endif
>> +
>> +  if target_base_arch in target_common_arch
>> +    if target_base_arch not in target_common_arch_libs
>> +      src = target_common_arch[target_base_arch]
>> +      lib = static_library(
>> +        'target_' + target_base_arch,
>> +        build_by_default: false,
>> +        sources: src.all_sources() + genh,
>> +        include_directories: inc,
>> +        c_args: target_c_args,
>> +        dependencies: src.all_dependencies())
>> +      target_common_arch_libs += {target_base_arch: lib}
>> +    endif
>> +  endif
>> +
>> +  if target_base_arch in target_common_system_arch
>> +    if target_base_arch not in target_common_system_arch_libs
>> +      src = target_common_system_arch[target_base_arch]
>> +      lib = static_library(
>> +        'target_system_' + target_base_arch,
>> +        build_by_default: false,
>> +        sources: src.all_sources() + genh,
>> +        include_directories: inc,
>> +        c_args: target_system_c_args,
>> +        dependencies: src.all_dependencies())
>> +      target_common_system_arch_libs += {target_base_arch: lib}
>> +    endif
>>      endif
>>    endforeach
>>    
>> @@ -4282,12 +4314,24 @@ foreach target : target_dirs
>>      target_common = common_ss.apply(config_target, strict: false)
>>      objects = [common_all.extract_objects(target_common.sources())]
>>      arch_deps += target_common.dependencies()
>> +  if target_base_arch in target_common_arch_libs
>> +    src = target_common_arch[target_base_arch].apply(config_target, strict: false)
>> +    lib = target_common_arch_libs[target_base_arch]
>> +    objects += lib.extract_objects(src.sources())
>> +    arch_deps += src.dependencies()
>> +  endif
>>      if target_type == 'system' and target_base_arch in hw_common_arch_libs
>>        src = hw_common_arch[target_base_arch].apply(config_target, strict: false)
>>        lib = hw_common_arch_libs[target_base_arch]
>>        objects += lib.extract_objects(src.sources())
>>        arch_deps += src.dependencies()
>>      endif
>> +  if target_type == 'system' and target_base_arch in target_common_system_arch_libs
>> +    src = target_common_system_arch[target_base_arch].apply(config_target, strict: false)
>> +    lib = target_common_system_arch_libs[target_base_arch]
>> +    objects += lib.extract_objects(src.sources())
>> +    arch_deps += src.dependencies()
>> +  endif
>>    
>>      target_specific = specific_ss.apply(config_target, strict: false)
>>      arch_srcs += target_specific.sources()
> 
> Somehow related to this patch, when converting from target_system_arch
> to target_common_system_arch, emptying it, I get:
> 
> ../../meson.build:4237:27: ERROR: Key microblaze is not in the dictionary.
> 
> 4235   if target.endswith('-softmmu')
> 4236     target_type='system'
> 4237     t = target_system_arch[target_base_arch].apply(config_target,
> strict: false)
>

Patch 12 introduces an empty arm_common_ss and it does not seem to be a 
problem.
Feel free to share your meson.build if there is a problem.

Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers

[Pierrick Bouvier]

On 4/29/25 3:28 AM, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> "linux/kvm.h" is not included for code compiled without
>> COMPILING_PER_TARGET, and headers are different depending architecture
>> (arm, arm64).
>> Thus we need to manually expose some definitions that will
>> be used by target/arm, ensuring they are the same for arm amd aarch64.
>>
>> As well, we must but prudent to not redefine things if code is already
>> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   target/arm/kvm_arm.h | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index c8ddf8beb2e..eedd081064c 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -16,6 +16,21 @@
>>   #define KVM_ARM_VGIC_V2   (1 << 0)
>>   #define KVM_ARM_VGIC_V3   (1 << 1)
>>   
>> +#ifndef COMPILING_PER_TARGET
>> +
>> +/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
>> + * for both architectures */
>> +#define KVM_ARM_IRQ_CPU_IRQ 0
>> +#define KVM_ARM_IRQ_CPU_FIQ 1
>> +#define KVM_ARM_IRQ_TYPE_CPU 0
>> +typedef unsigned int __u32;
>> +struct kvm_vcpu_init {
>> +    __u32 target;
>> +    __u32 features[7];
>> +};
>> +
>> +#endif /* COMPILING_PER_TARGET */
>> +
> 
> I'm not keen on the duplication. It seems to be the only reason we have
> struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
> only *external* user passes in a NULL.
>

I'm not keen about it either, so thanks for pointing it.

> If kvm_arm_create_scratch_host_vcpu() is made internal static to
> target/arm/kvm.c which will should always include the real linux headers
> you just need a QMP helper.
>

Yes, sounds like the good approach! Thanks.

> For the IRQ types is this just a sign of target/arm/cpu.c needing
> splitting into TCG and KVM bits?
> 

I'll move relevant functions to target/arm/kvm.c, so cpu.c can be 
isolated from this.

> 
>>   /**
>>    * kvm_arm_register_device:
>>    * @mr: memory region for this device
> 

Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers

[Pierrick Bouvier]

On 4/29/25 2:14 PM, Pierrick Bouvier wrote:
> On 4/29/25 3:28 AM, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> "linux/kvm.h" is not included for code compiled without
>>> COMPILING_PER_TARGET, and headers are different depending architecture
>>> (arm, arm64).
>>> Thus we need to manually expose some definitions that will
>>> be used by target/arm, ensuring they are the same for arm amd aarch64.
>>>
>>> As well, we must but prudent to not redefine things if code is already
>>> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    target/arm/kvm_arm.h | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>>> index c8ddf8beb2e..eedd081064c 100644
>>> --- a/target/arm/kvm_arm.h
>>> +++ b/target/arm/kvm_arm.h
>>> @@ -16,6 +16,21 @@
>>>    #define KVM_ARM_VGIC_V2   (1 << 0)
>>>    #define KVM_ARM_VGIC_V3   (1 << 1)
>>>    
>>> +#ifndef COMPILING_PER_TARGET
>>> +
>>> +/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
>>> + * for both architectures */
>>> +#define KVM_ARM_IRQ_CPU_IRQ 0
>>> +#define KVM_ARM_IRQ_CPU_FIQ 1
>>> +#define KVM_ARM_IRQ_TYPE_CPU 0
>>> +typedef unsigned int __u32;
>>> +struct kvm_vcpu_init {
>>> +    __u32 target;
>>> +    __u32 features[7];
>>> +};
>>> +
>>> +#endif /* COMPILING_PER_TARGET */
>>> +
>>
>> I'm not keen on the duplication. It seems to be the only reason we have
>> struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
>> only *external* user passes in a NULL.
>>
> 
> I'm not keen about it either, so thanks for pointing it.
> 
>> If kvm_arm_create_scratch_host_vcpu() is made internal static to
>> target/arm/kvm.c which will should always include the real linux headers
>> you just need a QMP helper.
>>
> 
> Yes, sounds like the good approach! Thanks.
>

Alas this function is used in target/arm/arm-qmp-cmds.c, and if we move 
the code using it, it pulls QAPI, which is target dependent at this time.

Since struct kvm_vcpu_init is only used by pointer, I could workaround 
this by doing a simple forward declaration in kvm_arm.h.

>> For the IRQ types is this just a sign of target/arm/cpu.c needing
>> splitting into TCG and KVM bits?
>>
> 
> I'll move relevant functions to target/arm/kvm.c, so cpu.c can be
> isolated from this.
> 
>>
>>>    /**
>>>     * kvm_arm_register_device:
>>>     * @mr: memory region for this device
>>
> 

Re: [PATCH 03/13] meson: add common libs for target and target_system

[Philippe Mathieu-Daudé]

On 29/4/25 23:11, Pierrick Bouvier wrote:
> On 4/29/25 11:01 AM, Philippe Mathieu-Daudé wrote:
>> Hi Pierrick,
>>
>> On 29/4/25 07:00, Pierrick Bouvier wrote:
>>> Following what we did for hw/, we need target specific common libraries
>>> for target. We need 2 different libraries:
>>> - code common to a base architecture
>>> - system code common to a base architecture
>>>
>>> For user code, it can stay compiled per target for now.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    meson.build | 78 ++++++++++++++++++++++++++++++++++++++++ 
>>> +------------
>>>    1 file changed, 61 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 68d36ac140f..7b2cf3cd7d1 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -3684,6 +3684,8 @@ target_arch = {}
>>>    target_system_arch = {}
>>>    target_user_arch = {}
>>>    hw_common_arch = {}
>>> +target_common_arch = {}
>>> +target_common_system_arch = {}
>>>    # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
>>>    # that is filled in by qapi/.
>>> @@ -4087,29 +4089,59 @@ common_all = static_library('common',
>>>    # construct common libraries per base architecture
>>>    hw_common_arch_libs = {}
>>> +target_common_arch_libs = {}
>>> +target_common_system_arch_libs = {}
>>>    foreach target : target_dirs
>>>      config_target = config_target_mak[target]
>>>      target_base_arch = config_target['TARGET_BASE_ARCH']
>>> +  target_inc = [include_directories('target' / target_base_arch)]
>>> +  inc = [common_user_inc + target_inc]
>>> -  # check if already generated
>>> -  if target_base_arch in hw_common_arch_libs
>>> -    continue
>>> -  endif
>>> +  # prevent common code to access cpu compile time definition,
>>> +  # but still allow access to cpu.h
>>> +  target_c_args = ['-DCPU_DEFS_H']
>>> +  target_system_c_args = target_c_args + ['- 
>>> DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU']
>>>      if target_base_arch in hw_common_arch
>>> -    target_inc = [include_directories('target' / target_base_arch)]
>>> -    src = hw_common_arch[target_base_arch]
>>> -    lib = static_library(
>>> -      'hw_' + target_base_arch,
>>> -      build_by_default: false,
>>> -      sources: src.all_sources() + genh,
>>> -      include_directories: common_user_inc + target_inc,
>>> -      implicit_include_directories: false,
>>> -      # prevent common code to access cpu compile time
>>> -      # definition, but still allow access to cpu.h
>>> -      c_args: ['-DCPU_DEFS_H', '-DCOMPILING_SYSTEM_VS_USER', '- 
>>> DCONFIG_SOFTMMU'],
>>> -      dependencies: src.all_dependencies())
>>> -    hw_common_arch_libs += {target_base_arch: lib}
>>> +    if target_base_arch not in hw_common_arch_libs
>>> +      src = hw_common_arch[target_base_arch]
>>> +      lib = static_library(
>>> +        'hw_' + target_base_arch,
>>> +        build_by_default: false,
>>> +        sources: src.all_sources() + genh,
>>> +        include_directories: inc,
>>> +        c_args: target_system_c_args,
>>> +        dependencies: src.all_dependencies())
>>> +      hw_common_arch_libs += {target_base_arch: lib}
>>> +    endif
>>> +  endif
>>> +
>>> +  if target_base_arch in target_common_arch
>>> +    if target_base_arch not in target_common_arch_libs
>>> +      src = target_common_arch[target_base_arch]
>>> +      lib = static_library(
>>> +        'target_' + target_base_arch,
>>> +        build_by_default: false,
>>> +        sources: src.all_sources() + genh,
>>> +        include_directories: inc,
>>> +        c_args: target_c_args,
>>> +        dependencies: src.all_dependencies())
>>> +      target_common_arch_libs += {target_base_arch: lib}
>>> +    endif
>>> +  endif
>>> +
>>> +  if target_base_arch in target_common_system_arch
>>> +    if target_base_arch not in target_common_system_arch_libs
>>> +      src = target_common_system_arch[target_base_arch]
>>> +      lib = static_library(
>>> +        'target_system_' + target_base_arch,
>>> +        build_by_default: false,
>>> +        sources: src.all_sources() + genh,
>>> +        include_directories: inc,
>>> +        c_args: target_system_c_args,
>>> +        dependencies: src.all_dependencies())
>>> +      target_common_system_arch_libs += {target_base_arch: lib}
>>> +    endif
>>>      endif
>>>    endforeach
>>> @@ -4282,12 +4314,24 @@ foreach target : target_dirs
>>>      target_common = common_ss.apply(config_target, strict: false)
>>>      objects = [common_all.extract_objects(target_common.sources())]
>>>      arch_deps += target_common.dependencies()
>>> +  if target_base_arch in target_common_arch_libs
>>> +    src = target_common_arch[target_base_arch].apply(config_target, 
>>> strict: false)
>>> +    lib = target_common_arch_libs[target_base_arch]
>>> +    objects += lib.extract_objects(src.sources())
>>> +    arch_deps += src.dependencies()
>>> +  endif
>>>      if target_type == 'system' and target_base_arch in 
>>> hw_common_arch_libs
>>>        src = hw_common_arch[target_base_arch].apply(config_target, 
>>> strict: false)
>>>        lib = hw_common_arch_libs[target_base_arch]
>>>        objects += lib.extract_objects(src.sources())
>>>        arch_deps += src.dependencies()
>>>      endif
>>> +  if target_type == 'system' and target_base_arch in 
>>> target_common_system_arch_libs
>>> +    src = 
>>> target_common_system_arch[target_base_arch].apply(config_target, 
>>> strict: false)
>>> +    lib = target_common_system_arch_libs[target_base_arch]
>>> +    objects += lib.extract_objects(src.sources())
>>> +    arch_deps += src.dependencies()
>>> +  endif
>>>      target_specific = specific_ss.apply(config_target, strict: false)
>>>      arch_srcs += target_specific.sources()
>>
>> Somehow related to this patch, when converting from target_system_arch

"Somehow related to" ~-> "pre-existing issue exposed by"

>> to target_common_system_arch, emptying it, I get:
>>
>> ../../meson.build:4237:27: ERROR: Key microblaze is not in the 
>> dictionary.
>>
>> 4235   if target.endswith('-softmmu')
>> 4236     target_type='system'
>> 4237     t = target_system_arch[target_base_arch].apply(config_target,
>> strict: false)
>>
> 
> Patch 12 introduces an empty arm_common_ss and it does not seem to be a 
> problem.
> Feel free to share your meson.build if there is a problem.

Empty arm_common_ss[] isn't a problem. What I'm saying is
when I move all files from target_system_arch[ARCH] to 
target_common_system_arch[ARCH] I get an error because
target_system_arch[ARCH] isn't expected to be empty.
I suppose due to:

   target_system_arch[target_base_arch].apply()

Yes, I can keep/add an empty source set but it makes meson
files review more cumbersome (unused source set, but if you
remove it then the build fails).

Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers

[Philippe Mathieu-Daudé]

On 30/4/25 00:02, Pierrick Bouvier wrote:
> On 4/29/25 2:14 PM, Pierrick Bouvier wrote:
>> On 4/29/25 3:28 AM, Alex Bennée wrote:
>>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>>
>>>> "linux/kvm.h" is not included for code compiled without
>>>> COMPILING_PER_TARGET, and headers are different depending architecture
>>>> (arm, arm64).
>>>> Thus we need to manually expose some definitions that will
>>>> be used by target/arm, ensuring they are the same for arm amd aarch64.
>>>>
>>>> As well, we must but prudent to not redefine things if code is already
>>>> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>    target/arm/kvm_arm.h | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>>>> index c8ddf8beb2e..eedd081064c 100644
>>>> --- a/target/arm/kvm_arm.h
>>>> +++ b/target/arm/kvm_arm.h
>>>> @@ -16,6 +16,21 @@
>>>>    #define KVM_ARM_VGIC_V2   (1 << 0)
>>>>    #define KVM_ARM_VGIC_V3   (1 << 1)
>>>> +#ifndef COMPILING_PER_TARGET
>>>> +
>>>> +/* we copy those definitions from asm-arm and asm-aarch64, as they 
>>>> are the same
>>>> + * for both architectures */
>>>> +#define KVM_ARM_IRQ_CPU_IRQ 0
>>>> +#define KVM_ARM_IRQ_CPU_FIQ 1
>>>> +#define KVM_ARM_IRQ_TYPE_CPU 0
>>>> +typedef unsigned int __u32;
>>>> +struct kvm_vcpu_init {
>>>> +    __u32 target;
>>>> +    __u32 features[7];
>>>> +};
>>>> +
>>>> +#endif /* COMPILING_PER_TARGET */
>>>> +
>>>
>>> I'm not keen on the duplication. It seems to be the only reason we have
>>> struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
>>> only *external* user passes in a NULL.
>>>
>>
>> I'm not keen about it either, so thanks for pointing it.
>>
>>> If kvm_arm_create_scratch_host_vcpu() is made internal static to
>>> target/arm/kvm.c which will should always include the real linux headers
>>> you just need a QMP helper.
>>>
>>
>> Yes, sounds like the good approach! Thanks.
>>
> 
> Alas this function is used in target/arm/arm-qmp-cmds.c, and if we move 
> the code using it, it pulls QAPI, which is target dependent at this time.
> 
> Since struct kvm_vcpu_init is only used by pointer, I could workaround 
> this by doing a simple forward declaration in kvm_arm.h.

Correct, great!

Re: [PATCH 03/13] meson: add common libs for target and target_system

[Pierrick Bouvier]

On 4/29/25 11:06 PM, Philippe Mathieu-Daudé wrote:
> On 29/4/25 23:11, Pierrick Bouvier wrote:
>> On 4/29/25 11:01 AM, Philippe Mathieu-Daudé wrote:
>>> Hi Pierrick,
>>>
>>> On 29/4/25 07:00, Pierrick Bouvier wrote:
>>>> Following what we did for hw/, we need target specific common libraries
>>>> for target. We need 2 different libraries:
>>>> - code common to a base architecture
>>>> - system code common to a base architecture
>>>>
>>>> For user code, it can stay compiled per target for now.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>     meson.build | 78 ++++++++++++++++++++++++++++++++++++++++
>>>> +------------
>>>>     1 file changed, 61 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index 68d36ac140f..7b2cf3cd7d1 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -3684,6 +3684,8 @@ target_arch = {}
>>>>     target_system_arch = {}
>>>>     target_user_arch = {}
>>>>     hw_common_arch = {}
>>>> +target_common_arch = {}
>>>> +target_common_system_arch = {}
>>>>     # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
>>>>     # that is filled in by qapi/.
>>>> @@ -4087,29 +4089,59 @@ common_all = static_library('common',
>>>>     # construct common libraries per base architecture
>>>>     hw_common_arch_libs = {}
>>>> +target_common_arch_libs = {}
>>>> +target_common_system_arch_libs = {}
>>>>     foreach target : target_dirs
>>>>       config_target = config_target_mak[target]
>>>>       target_base_arch = config_target['TARGET_BASE_ARCH']
>>>> +  target_inc = [include_directories('target' / target_base_arch)]
>>>> +  inc = [common_user_inc + target_inc]
>>>> -  # check if already generated
>>>> -  if target_base_arch in hw_common_arch_libs
>>>> -    continue
>>>> -  endif
>>>> +  # prevent common code to access cpu compile time definition,
>>>> +  # but still allow access to cpu.h
>>>> +  target_c_args = ['-DCPU_DEFS_H']
>>>> +  target_system_c_args = target_c_args + ['-
>>>> DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU']
>>>>       if target_base_arch in hw_common_arch
>>>> -    target_inc = [include_directories('target' / target_base_arch)]
>>>> -    src = hw_common_arch[target_base_arch]
>>>> -    lib = static_library(
>>>> -      'hw_' + target_base_arch,
>>>> -      build_by_default: false,
>>>> -      sources: src.all_sources() + genh,
>>>> -      include_directories: common_user_inc + target_inc,
>>>> -      implicit_include_directories: false,
>>>> -      # prevent common code to access cpu compile time
>>>> -      # definition, but still allow access to cpu.h
>>>> -      c_args: ['-DCPU_DEFS_H', '-DCOMPILING_SYSTEM_VS_USER', '-
>>>> DCONFIG_SOFTMMU'],
>>>> -      dependencies: src.all_dependencies())
>>>> -    hw_common_arch_libs += {target_base_arch: lib}
>>>> +    if target_base_arch not in hw_common_arch_libs
>>>> +      src = hw_common_arch[target_base_arch]
>>>> +      lib = static_library(
>>>> +        'hw_' + target_base_arch,
>>>> +        build_by_default: false,
>>>> +        sources: src.all_sources() + genh,
>>>> +        include_directories: inc,
>>>> +        c_args: target_system_c_args,
>>>> +        dependencies: src.all_dependencies())
>>>> +      hw_common_arch_libs += {target_base_arch: lib}
>>>> +    endif
>>>> +  endif
>>>> +
>>>> +  if target_base_arch in target_common_arch
>>>> +    if target_base_arch not in target_common_arch_libs
>>>> +      src = target_common_arch[target_base_arch]
>>>> +      lib = static_library(
>>>> +        'target_' + target_base_arch,
>>>> +        build_by_default: false,
>>>> +        sources: src.all_sources() + genh,
>>>> +        include_directories: inc,
>>>> +        c_args: target_c_args,
>>>> +        dependencies: src.all_dependencies())
>>>> +      target_common_arch_libs += {target_base_arch: lib}
>>>> +    endif
>>>> +  endif
>>>> +
>>>> +  if target_base_arch in target_common_system_arch
>>>> +    if target_base_arch not in target_common_system_arch_libs
>>>> +      src = target_common_system_arch[target_base_arch]
>>>> +      lib = static_library(
>>>> +        'target_system_' + target_base_arch,
>>>> +        build_by_default: false,
>>>> +        sources: src.all_sources() + genh,
>>>> +        include_directories: inc,
>>>> +        c_args: target_system_c_args,
>>>> +        dependencies: src.all_dependencies())
>>>> +      target_common_system_arch_libs += {target_base_arch: lib}
>>>> +    endif
>>>>       endif
>>>>     endforeach
>>>> @@ -4282,12 +4314,24 @@ foreach target : target_dirs
>>>>       target_common = common_ss.apply(config_target, strict: false)
>>>>       objects = [common_all.extract_objects(target_common.sources())]
>>>>       arch_deps += target_common.dependencies()
>>>> +  if target_base_arch in target_common_arch_libs
>>>> +    src = target_common_arch[target_base_arch].apply(config_target,
>>>> strict: false)
>>>> +    lib = target_common_arch_libs[target_base_arch]
>>>> +    objects += lib.extract_objects(src.sources())
>>>> +    arch_deps += src.dependencies()
>>>> +  endif
>>>>       if target_type == 'system' and target_base_arch in
>>>> hw_common_arch_libs
>>>>         src = hw_common_arch[target_base_arch].apply(config_target,
>>>> strict: false)
>>>>         lib = hw_common_arch_libs[target_base_arch]
>>>>         objects += lib.extract_objects(src.sources())
>>>>         arch_deps += src.dependencies()
>>>>       endif
>>>> +  if target_type == 'system' and target_base_arch in
>>>> target_common_system_arch_libs
>>>> +    src =
>>>> target_common_system_arch[target_base_arch].apply(config_target,
>>>> strict: false)
>>>> +    lib = target_common_system_arch_libs[target_base_arch]
>>>> +    objects += lib.extract_objects(src.sources())
>>>> +    arch_deps += src.dependencies()
>>>> +  endif
>>>>       target_specific = specific_ss.apply(config_target, strict: false)
>>>>       arch_srcs += target_specific.sources()
>>>
>>> Somehow related to this patch, when converting from target_system_arch
> 
> "Somehow related to" ~-> "pre-existing issue exposed by"
> 
>>> to target_common_system_arch, emptying it, I get:
>>>
>>> ../../meson.build:4237:27: ERROR: Key microblaze is not in the
>>> dictionary.
>>>
>>> 4235   if target.endswith('-softmmu')
>>> 4236     target_type='system'
>>> 4237     t = target_system_arch[target_base_arch].apply(config_target,
>>> strict: false)
>>>
>>
>> Patch 12 introduces an empty arm_common_ss and it does not seem to be a
>> problem.
>> Feel free to share your meson.build if there is a problem.
> 
> Empty arm_common_ss[] isn't a problem. What I'm saying is
> when I move all files from target_system_arch[ARCH] to
> target_common_system_arch[ARCH] I get an error because
> target_system_arch[ARCH] isn't expected to be empty.
> I suppose due to:
> 
>     target_system_arch[target_base_arch].apply()
> 
> Yes, I can keep/add an empty source set but it makes meson
> files review more cumbersome (unused source set, but if you
> remove it then the build fails).

Oh, I see.
Then, you just need to add a conditional
"if target_base_arch in target_system_arch" around this spot, and remove 
the dictionary entry set in target/microblaze/meson.build.

- target_arch += {'microblaze': microblaze_ss}

This target was much quicker than arm, it's nice :)

Re: [PATCH 12/13] target/arm/cpu: compile file twice (user, system) only

[Philippe Mathieu-Daudé]

On 29/4/25 07:00, Pierrick Bouvier wrote:
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   target/arm/meson.build | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/meson.build b/target/arm/meson.build
> index c39ddc4427b..89e305eb56a 100644
> --- a/target/arm/meson.build
> +++ b/target/arm/meson.build
> @@ -1,6 +1,6 @@
>   arm_ss = ss.source_set()
> +arm_common_ss = ss.source_set()

Unused AFAICT.

>   arm_ss.add(files(
> -  'cpu.c',
>     'debug_helper.c',
>     'gdbstub.c',
>     'helper.c',
> @@ -20,6 +20,7 @@ arm_ss.add(when: 'TARGET_AARCH64',
>   )
>   
>   arm_system_ss = ss.source_set()
> +arm_common_system_ss = ss.source_set()
>   arm_system_ss.add(files(
>     'arch_dump.c',
>     'arm-powerctl.c',
> @@ -30,6 +31,9 @@ arm_system_ss.add(files(
>   ))
>   
>   arm_user_ss = ss.source_set()
> +arm_user_ss.add(files('cpu.c'))
> +
> +arm_common_system_ss.add(files('cpu.c'), capstone)
>   
>   subdir('hvf')
>   
> @@ -42,3 +46,5 @@ endif
>   target_arch += {'arm': arm_ss}
>   target_system_arch += {'arm': arm_system_ss}
>   target_user_arch += {'arm': arm_user_ss}
> +target_common_arch += {'arm': arm_common_ss}
> +target_common_system_arch += {'arm': arm_common_system_ss}

Re: [PATCH 12/13] target/arm/cpu: compile file twice (user, system) only

[Pierrick Bouvier]

On 4/30/25 1:32 AM, Philippe Mathieu-Daudé wrote:
> On 29/4/25 07:00, Pierrick Bouvier wrote:
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/meson.build | 8 +++++++-
>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/meson.build b/target/arm/meson.build
>> index c39ddc4427b..89e305eb56a 100644
>> --- a/target/arm/meson.build
>> +++ b/target/arm/meson.build
>> @@ -1,6 +1,6 @@
>>    arm_ss = ss.source_set()
>> +arm_common_ss = ss.source_set()
> 
> Unused AFAICT.
>

Yes, I was expecting some files to eventually be really common, but so 
far I didn't find some in target/arm.
Same comment goes for the patch 3 with associated target_common libraries.

I'm not sure if it's worth saving the lines (especially in main 
meson.build), compared to the "pain" for someone to have to write them 
later.

I don't mind removing this though, if you think it's too bad to leave 
this unused.

>>    arm_ss.add(files(
>> -  'cpu.c',
>>      'debug_helper.c',
>>      'gdbstub.c',
>>      'helper.c',
>> @@ -20,6 +20,7 @@ arm_ss.add(when: 'TARGET_AARCH64',
>>    )
>>    
>>    arm_system_ss = ss.source_set()
>> +arm_common_system_ss = ss.source_set()
>>    arm_system_ss.add(files(
>>      'arch_dump.c',
>>      'arm-powerctl.c',
>> @@ -30,6 +31,9 @@ arm_system_ss.add(files(
>>    ))
>>    
>>    arm_user_ss = ss.source_set()
>> +arm_user_ss.add(files('cpu.c'))
>> +
>> +arm_common_system_ss.add(files('cpu.c'), capstone)
>>    
>>    subdir('hvf')
>>    
>> @@ -42,3 +46,5 @@ endif
>>    target_arch += {'arm': arm_ss}
>>    target_system_arch += {'arm': arm_system_ss}
>>    target_user_arch += {'arm': arm_user_ss}
>> +target_common_arch += {'arm': arm_common_ss}
>> +target_common_system_arch += {'arm': arm_common_system_ss}
>