Signed-off-by: Philippe Mathieu-Daudé <philmd@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);
+}
+
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 (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;
+ }
if (mc->alias) {
printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
}
--
2.47.1
On 4/3/25 16:49, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@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.
> +}
> +
> 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.
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é <philmd@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?
>
>> +}
>> +
>> 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.
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é <philmd@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.
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.
>
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é <philmd@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.
>>
>
© 2016 - 2025 Red Hat, Inc.