On 4/4/25 11:08, Pierrick Bouvier wrote:
On 4/4/25 11:01, Philippe Mathieu-Daudé wrote:
Hi Pierrick,

On 4/4/25 19:10, Pierrick Bouvier wrote:
On 4/3/25 16:49, Philippe Mathieu-Daudé wrote:
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
    system/vl.c | 24 ++++++++++++++++++++++++
    1 file changed, 24 insertions(+)

diff --git a/system/vl.c b/system/vl.c
index d8a0fe713c9..554f5f2a467 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -27,6 +27,8 @@
    #include "qemu/datadir.h"
    #include "qemu/units.h"
    #include "qemu/module.h"
+#include "qemu/target_info.h"
+#include "qemu/target_info-qom.h"
    #include "exec/cpu-common.h"
    #include "exec/page-vary.h"
    #include "hw/qdev-properties.h"
@@ -833,14 +835,29 @@ static bool usb_parse(const char *cmdline, Error
**errp)
    /***********************************************************/
    /* machine registration */
+static char *machine_binary_filter(void)
+{
+    if (target_info_is_stub()) {
+        return NULL;
+    }
+    return g_strconcat(TYPE_LEGACY_BINARY_PREFIX,
+                       "qemu-system-", target_name(), NULL);

No, we should not have such things.
We can make it work with proper QOM types, defined by target, instead of
relying on string construction/compare like this.

I am not understanding you, do you mind sharing code snippets of what
you have in mind?


Instead of the current and previous patch,

we define TYPE_TARGET_MACHINE_PREFIX.

For each target, we define a specific TYPE_TARGET_MACHINE variant, like:
- TYPE_TARGET_MACHINE_ARM
- TYPE_TARGET_MACHINE_AARCH64
...

In TargetInfo, we add a new function target_machine_type(), that returns
this type, specialized for each architecture.
As a first step, the stub implementation can return TYPE_MACHINE, and we
can enable this architecture per architecture later.

For the first architecture implementation, arm, we will define
TYPE_TARGET_MACHINE_ARM, and TYPE_TARGET_MACHINE_AARCH64, which will
allow concerned files to be common, while still maintaining a specific
set of machines per target.


Note: Those TYPE_TARGET_MACHINE_* types are QOM interfaces, that every concerned machine implements.

Once things are done this way, the only required change is:
- machines = object_class_get_list(TYPE_MACHINE, false);
+ machines = object_class_get_list(target_machine_type(), false);

As a further step, it will be very easy to support having multiple targets enabled at the same time (build a list of machine types instead of a single one), but we can do this *later* when tackling heterogeneous emulation.

Is that more clear?


+}
+
    static MachineClass *find_machine(const char *name, GSList *machines)
    {
        GSList *el;
+    g_autofree char *binary_filter = machine_binary_filter();
        for (el = machines; el; el = el->next) {
            MachineClass *mc = el->data;
            if (!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) {
+            if (binary_filter && !object_class_dynamic_cast(el->data,
+
binary_filter)) {
+                /* Machine is not for this binary: fail */
+                return NULL;
+            }
                return mc;
            }
        }
@@ -1563,6 +1580,7 @@ static void machine_help_func(const QDict *qdict)
        g_autoptr(GSList) machines = NULL;
        GSList *el;
        const char *type = qdict_get_try_str(qdict, "type");
+    g_autofree char *binary_filter = machine_binary_filter();
        machines = object_class_get_list(TYPE_MACHINE, false);

If we define a proper TYPE_TARGET_MACHINE per target, and we add this to
TargetInfo, this can become:

machines = object_class_get_list(target_machine_type(), false);

And we don't need any other string hack to detect what is the correct type.

        if (type) {
@@ -1577,6 +1595,12 @@ static void machine_help_func(const QDict *qdict)
        machines = g_slist_sort(machines, machine_class_cmp);
        for (el = machines; el; el = el->next) {
            MachineClass *mc = el->data;
+
+        if (binary_filter && !object_class_dynamic_cast(el->data,
+
binary_filter)) {
+            /* Machine is not for this binary: skip */
+            continue;
+        }

With the approach above, this is not needed anymore.

            if (mc->alias) {
                printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc,
mc->name);
            }

I think we are missing a commit here, defining a proper
TYPE_TARGET_MACHINE_PREFIX, that is target dependent, instead of the
TYPE_LEGACY_BINARY_PREFIX.

And we should include in this type in TargetInfo, the same way it was
done for cpus.



Reply via email to