Re: [RFC PATCH v2 02/11] qemu: Convert target_name() to TargetInfo API
On 18/4/25 18:23, Pierrick Bouvier wrote:
On 4/18/25 07:02, Philippe Mathieu-Daudé wrote:
On 18/4/25 05:01, Pierrick Bouvier wrote:
On 4/17/25 17:50, Philippe Mathieu-Daudé wrote:
Have target_name() be a target-agnostic method, dispatching
to a per-target TargetInfo singleton structure.
By default a stub singleton is used. No logical change
expected.
Inspired-by: Pierrick Bouvier
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Pierrick Bouvier
---
meson.build | 3 +++
include/hw/core/cpu.h | 2 --
include/qemu/target_info-impl.h | 23 +++
include/qemu/target_info.h | 19 +++
cpu-target.c | 5 -
hw/core/machine-qmp-cmds.c | 1 +
plugins/loader.c | 2 +-
system/vl.c | 2 +-
target_info-stub.c | 19 +++
target_info.c | 16
10 files changed, 83 insertions(+), 9 deletions(-)
create mode 100644 include/qemu/target_info-impl.h
create mode 100644 include/qemu/target_info.h
create mode 100644 target_info-stub.c
create mode 100644 target_info.c
diff --git a/target_info-stub.c b/target_info-stub.c
new file mode 100644
index 000..1e44bb6f6fb
--- /dev/null
+++ b/target_info-stub.c
@@ -0,0 +1,19 @@
+/*
+ * QEMU target info stubs
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+
+static const TargetInfo target_info_stub = {
+ .name = TARGET_NAME,
+};
+
+const TargetInfo *target_info(void)
+{
+ return &target_info_stub;
+}
diff --git a/target_info.c b/target_info.c
new file mode 100644
index 000..877a6a15014
--- /dev/null
+++ b/target_info.c
@@ -0,0 +1,16 @@
+/*
+ * QEMU binary/target helpers
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+#include "qemu/target_info.h"
+
+const char *target_name(void)
+{
+ return target_info()->name;
+}
What is the benefit to have two different files (common and specific)?
target_name() can be inline in the same header, returning the matching
field in existing target_info, which does not need any specialization
per target.
common interface exposed target-agnostic, dispatching to target-specific
implementation (providing a stub we'll remove once all targets
converted).
What would you suggest?
To remove target_info.c and target_info-impl.h, and implement
target_name() as a static inline in target_info.h.
This way, target_info.h can still be included from common code, the
structure is directly accessed, and we have a single header where we can
add new sugar functions and associated fields later.
OK. As I'm about to post a good-enough v3, I'll not implement this
now, but will consider for v4.
Re: [RFC PATCH v2 02/11] qemu: Convert target_name() to TargetInfo API
On 4/18/25 07:02, Philippe Mathieu-Daudé wrote:
On 18/4/25 05:01, Pierrick Bouvier wrote:
On 4/17/25 17:50, Philippe Mathieu-Daudé wrote:
Have target_name() be a target-agnostic method, dispatching
to a per-target TargetInfo singleton structure.
By default a stub singleton is used. No logical change
expected.
Inspired-by: Pierrick Bouvier
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Pierrick Bouvier
---
meson.build | 3 +++
include/hw/core/cpu.h | 2 --
include/qemu/target_info-impl.h | 23 +++
include/qemu/target_info.h | 19 +++
cpu-target.c | 5 -
hw/core/machine-qmp-cmds.c | 1 +
plugins/loader.c | 2 +-
system/vl.c | 2 +-
target_info-stub.c | 19 +++
target_info.c | 16
10 files changed, 83 insertions(+), 9 deletions(-)
create mode 100644 include/qemu/target_info-impl.h
create mode 100644 include/qemu/target_info.h
create mode 100644 target_info-stub.c
create mode 100644 target_info.c
diff --git a/target_info-stub.c b/target_info-stub.c
new file mode 100644
index 000..1e44bb6f6fb
--- /dev/null
+++ b/target_info-stub.c
@@ -0,0 +1,19 @@
+/*
+ * QEMU target info stubs
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+
+static const TargetInfo target_info_stub = {
+ .name = TARGET_NAME,
+};
+
+const TargetInfo *target_info(void)
+{
+ return &target_info_stub;
+}
diff --git a/target_info.c b/target_info.c
new file mode 100644
index 000..877a6a15014
--- /dev/null
+++ b/target_info.c
@@ -0,0 +1,16 @@
+/*
+ * QEMU binary/target helpers
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+#include "qemu/target_info.h"
+
+const char *target_name(void)
+{
+ return target_info()->name;
+}
What is the benefit to have two different files (common and specific)?
target_name() can be inline in the same header, returning the matching
field in existing target_info, which does not need any specialization
per target.
common interface exposed target-agnostic, dispatching to target-specific
implementation (providing a stub we'll remove once all targets converted).
What would you suggest?
To remove target_info.c and target_info-impl.h, and implement
target_name() as a static inline in target_info.h.
This way, target_info.h can still be included from common code, the
structure is directly accessed, and we have a single header where we can
add new sugar functions and associated fields later.
Re: [RFC PATCH v2 02/11] qemu: Convert target_name() to TargetInfo API
On 18/4/25 05:01, Pierrick Bouvier wrote:
On 4/17/25 17:50, Philippe Mathieu-Daudé wrote:
Have target_name() be a target-agnostic method, dispatching
to a per-target TargetInfo singleton structure.
By default a stub singleton is used. No logical change
expected.
Inspired-by: Pierrick Bouvier
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Pierrick Bouvier
---
meson.build | 3 +++
include/hw/core/cpu.h | 2 --
include/qemu/target_info-impl.h | 23 +++
include/qemu/target_info.h | 19 +++
cpu-target.c | 5 -
hw/core/machine-qmp-cmds.c | 1 +
plugins/loader.c | 2 +-
system/vl.c | 2 +-
target_info-stub.c | 19 +++
target_info.c | 16
10 files changed, 83 insertions(+), 9 deletions(-)
create mode 100644 include/qemu/target_info-impl.h
create mode 100644 include/qemu/target_info.h
create mode 100644 target_info-stub.c
create mode 100644 target_info.c
diff --git a/target_info-stub.c b/target_info-stub.c
new file mode 100644
index 000..1e44bb6f6fb
--- /dev/null
+++ b/target_info-stub.c
@@ -0,0 +1,19 @@
+/*
+ * QEMU target info stubs
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+
+static const TargetInfo target_info_stub = {
+ .name = TARGET_NAME,
+};
+
+const TargetInfo *target_info(void)
+{
+ return &target_info_stub;
+}
diff --git a/target_info.c b/target_info.c
new file mode 100644
index 000..877a6a15014
--- /dev/null
+++ b/target_info.c
@@ -0,0 +1,16 @@
+/*
+ * QEMU binary/target helpers
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+#include "qemu/target_info.h"
+
+const char *target_name(void)
+{
+ return target_info()->name;
+}
What is the benefit to have two different files (common and specific)?
target_name() can be inline in the same header, returning the matching
field in existing target_info, which does not need any specialization
per target.
common interface exposed target-agnostic, dispatching to target-specific
implementation (providing a stub we'll remove once all targets converted).
What would you suggest?
Re: [RFC PATCH v2 02/11] qemu: Convert target_name() to TargetInfo API
On 4/17/25 17:50, Philippe Mathieu-Daudé wrote:
Have target_name() be a target-agnostic method, dispatching
to a per-target TargetInfo singleton structure.
By default a stub singleton is used. No logical change
expected.
Inspired-by: Pierrick Bouvier
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Pierrick Bouvier
---
meson.build | 3 +++
include/hw/core/cpu.h | 2 --
include/qemu/target_info-impl.h | 23 +++
include/qemu/target_info.h | 19 +++
cpu-target.c| 5 -
hw/core/machine-qmp-cmds.c | 1 +
plugins/loader.c| 2 +-
system/vl.c | 2 +-
target_info-stub.c | 19 +++
target_info.c | 16
10 files changed, 83 insertions(+), 9 deletions(-)
create mode 100644 include/qemu/target_info-impl.h
create mode 100644 include/qemu/target_info.h
create mode 100644 target_info-stub.c
create mode 100644 target_info.c
diff --git a/meson.build b/meson.build
index bcb9d39a387..49a050a28a3 100644
--- a/meson.build
+++ b/meson.build
@@ -3807,6 +3807,9 @@ endif
common_ss.add(pagevary)
specific_ss.add(files('page-target.c', 'page-vary-target.c'))
+common_ss.add(files('target_info.c'))
+specific_ss.add(files('target_info-stub.c'))
+
subdir('backends')
subdir('disas')
subdir('migration')
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5b645df59f5..9d9448341d1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1115,8 +1115,6 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp);
void cpu_exec_unrealizefn(CPUState *cpu);
void cpu_exec_reset_hold(CPUState *cpu);
-const char *target_name(void);
-
#ifdef COMPILING_PER_TARGET
extern const VMStateDescription vmstate_cpu_common;
diff --git a/include/qemu/target_info-impl.h b/include/qemu/target_info-impl.h
new file mode 100644
index 000..d5c94ed5296
--- /dev/null
+++ b/include/qemu/target_info-impl.h
@@ -0,0 +1,23 @@
+/*
+ * QEMU binary/target API ...
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_TARGET_INFO_IMPL_H
+#define QEMU_TARGET_INFO_IMPL_H
+
+#include "qemu/target_info.h"
+
+typedef struct TargetInfo {
+
+/* runtime equivalent of TARGET_NAME definition */
+const char *const name;
+
+} TargetInfo;
+
+const TargetInfo *target_info(void);
+
+#endif
diff --git a/include/qemu/target_info.h b/include/qemu/target_info.h
new file mode 100644
index 000..3f6cbbbd300
--- /dev/null
+++ b/include/qemu/target_info.h
@@ -0,0 +1,19 @@
+/*
+ * QEMU binary/target API
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_TARGET_INFO_H
+#define QEMU_TARGET_INFO_H
+
+/**
+ * target_name:
+ *
+ * Returns: Canonical target name (i.e. "i386").
+ */
+const char *target_name(void);
+
+#endif
diff --git a/cpu-target.c b/cpu-target.c
index c99d208a7c4..3f82d3ea444 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -165,8 +165,3 @@ bool target_words_bigendian(void)
{
return TARGET_BIG_ENDIAN;
}
-
-const char *target_name(void)
-{
-return TARGET_NAME;
-}
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 408994b67d7..b317aec234f 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -19,6 +19,7 @@
#include "qapi/qobject-input-visitor.h"
#include "qapi/type-helpers.h"
#include "qemu/uuid.h"
+#include "qemu/target_info.h"
#include "qom/qom-qobject.h"
#include "system/hostmem.h"
#include "system/hw_accel.h"
diff --git a/plugins/loader.c b/plugins/loader.c
index 7523d554f03..36a4e88d4db 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -29,7 +29,7 @@
#include "qemu/xxhash.h"
#include "qemu/plugin.h"
#include "qemu/memalign.h"
-#include "hw/core/cpu.h"
+#include "qemu/target_info.h"
#include "exec/tb-flush.h"
#include "plugin.h"
diff --git a/system/vl.c b/system/vl.c
index c17945c4939..d8a0fe713c9 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -40,6 +40,7 @@
#include "qemu/help_option.h"
#include "qemu/hw-version.h"
#include "qemu/uuid.h"
+#include "qemu/target_info.h"
#include "system/reset.h"
#include "system/runstate.h"
#include "system/runstate-action.h"
@@ -79,7 +80,6 @@
#include "hw/block/block.h"
#include "hw/i386/x86.h"
#include "hw/i386/pc.h"
-#include "hw/core/cpu.h"
#include "migration/cpr.h"
#include "migration/misc.h"
#include "migration/snapshot.h"
diff --git a/target_info-stub.c b/target_info-stub.c
new file mode 100644
index 000..1e44bb6f6fb
--- /dev/null
+++ b/target_info-stub.c
@@ -0,0 +1,19 @@
+/*
+ * QEMU target info stubs
+ *
+ * Copyright (c) Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/target_info-impl.h"
+
+static const TargetInfo target_info_stub = {
+.name = TARGET_NAME,
+};
+
+const
