Re: [PATCH 8/8] qom: pass TargetInfo to is_available for later use

[Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>]

On 10/7/2026 5:00 PM, Pierrick Bouvier wrote:
> On 10/7/2026 2:42 PM, Yonggang Luo wrote:
>> On Thu, Oct 8, 2026 at 4:44 AM Pierrick Bouvier
>> <pierrick.bouvier@oss.qualcomm.com> wrote:
>>>
>>> On 10/7/2026 4:48 AM, Yonggang Luo wrote:
>>>> Change TypeInfo.is_available to the new TypeIsAvailable type, which
>>>> takes a const TargetInfo *. Registration passes target_info(), so
>>>> behavior is unchanged. object.h only forward-declares TargetInfo.
>>>>
>>>> Switch the existing users (raspi, raspi4b, ast27x0 evb and fc) and the
>>>> generated machine parents to the matching target_is_<arch> helpers.
>>>>
>>>> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
>>>> ---
>>>>  hw/arm/aspeed_ast27x0-fc.c  |  2 +-
>>>>  hw/arm/aspeed_ast27x0_evb.c |  4 ++--
>>>>  hw/arm/raspi.c              |  4 ++--
>>>>  hw/arm/raspi4b.c            |  2 +-
>>>>  hw/arm/virt.c               |  4 ++--
>>>>  hw/m68k/virt.c              |  4 ++--
>>>>  hw/or1k/virt.c              |  2 +-
>>>>  include/qom/object.h        | 12 +++++++++++-
>>>>  qom/object.c                |  3 ++-
>>>>  9 files changed, 24 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
>>>> index 73362b60d2a..8a7c6a116ac 100644
>>>> --- a/hw/arm/aspeed_ast27x0-fc.c
>>>> +++ b/hw/arm/aspeed_ast27x0-fc.c
>>>> @@ -234,7 +234,7 @@ static const TypeInfo ast2700fc_types[] = {
>>>>          .parent         = TYPE_MACHINE,
>>>>          .class_init     = ast2700fc_class_init,
>>>>          .instance_size  = sizeof(Ast2700FCState),
>>>> -        .is_available   = target_aarch64,
>>>> +        .is_available   = target_is_aarch64,
>>>>      },
>>>>  };
>>>>
>>>> diff --git a/hw/arm/aspeed_ast27x0_evb.c b/hw/arm/aspeed_ast27x0_evb.c
>>>> index 460c9ff0d2e..02a8cff1d9d 100644
>>>> --- a/hw/arm/aspeed_ast27x0_evb.c
>>>> +++ b/hw/arm/aspeed_ast27x0_evb.c
>>>> @@ -75,13 +75,13 @@ static const TypeInfo aspeed_ast27x0_evb_types[] = {
>>>>          .name          = MACHINE_TYPE_NAME("ast2700a1-evb"),
>>>>          .parent        = TYPE_ASPEED_MACHINE,
>>>>          .class_init    = aspeed_machine_ast2700a1_evb_class_init,
>>>> -        .is_available  = target_aarch64,
>>>> +        .is_available  = target_is_aarch64,
>>>>      },
>>>>      {
>>>>          .name          = MACHINE_TYPE_NAME("ast2700a2-evb"),
>>>>          .parent        = TYPE_ASPEED_MACHINE,
>>>>          .class_init    = aspeed_machine_ast2700a2_evb_class_init,
>>>> -        .is_available  = target_aarch64,
>>>> +        .is_available  = target_is_aarch64,
>>>>      }
>>>>  };
>>>>
>>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>>> index fadbacb2534..22b06c6ca22 100644
>>>> --- a/hw/arm/raspi.c
>>>> +++ b/hw/arm/raspi.c
>>>> @@ -403,12 +403,12 @@ static const TypeInfo raspi_machine_types[] = {
>>>>          .name           = MACHINE_TYPE_NAME("raspi3ap"),
>>>>          .parent         = TYPE_RASPI_MACHINE,
>>>>          .class_init     = raspi3ap_machine_class_init,
>>>> -        .is_available   = target_aarch64,
>>>> +        .is_available   = target_is_aarch64,
>>>>      }, {
>>>>          .name           = MACHINE_TYPE_NAME("raspi3b"),
>>>>          .parent         = TYPE_RASPI_MACHINE,
>>>>          .class_init     = raspi3b_machine_class_init,
>>>> -        .is_available   = target_aarch64,
>>>> +        .is_available   = target_is_aarch64,
>>>>      }, {
>>>>          .name           = TYPE_RASPI_MACHINE,
>>>>          .parent         = TYPE_RASPI_BASE_MACHINE,
>>>> diff --git a/hw/arm/raspi4b.c b/hw/arm/raspi4b.c
>>>> index 566d64572db..a1dc477f25e 100644
>>>> --- a/hw/arm/raspi4b.c
>>>> +++ b/hw/arm/raspi4b.c
>>>> @@ -121,7 +121,7 @@ static const TypeInfo raspi4b_machine_type = {
>>>>      .parent         = TYPE_RASPI_BASE_MACHINE,
>>>>      .instance_size  = sizeof(Raspi4bMachineState),
>>>>      .class_init     = raspi4b_machine_class_init,
>>>> -    .is_available   = target_aarch64,
>>>> +    .is_available   = target_is_aarch64,
>>>>  };
>>>>
>>>>  static void raspi4b_machine_register_type(void)
>>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>>> index 9d192515d85..18160f7fad3 100644
>>>> --- a/hw/arm/virt.c
>>>> +++ b/hw/arm/virt.c
>>>> @@ -132,7 +132,7 @@ static void arm_virt_compat_default_set(MachineClass *mc)
>>>>          .name = MACHINE_VER_TYPE_NAME("virt", __VA_ARGS__), \
>>>>          .parent = TYPE_VIRT_MACHINE, \
>>>>          .class_init = MACHINE_VER_SYM(class_init, virt, __VA_ARGS__), \
>>>> -        .is_available = target_base_arm, \
>>>> +        .is_available = target_is_base_arm, \
>>>>      }; \
>>>>      static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \
>>>>      { \
>>>> @@ -4430,7 +4430,7 @@ static const TypeInfo virt_machine_info = {
>>>>           { TYPE_HOTPLUG_HANDLER },
>>>>           { }
>>>>      },
>>>> -    .is_available = target_base_arm,
>>>> +    .is_available = target_is_base_arm,
>>>>  };
>>>>
>>>>  static void machvirt_machine_init(void)
>>>> diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
>>>> index e52546e4fab..ad2a43ef3fb 100644
>>>> --- a/hw/m68k/virt.c
>>>> +++ b/hw/m68k/virt.c
>>>> @@ -327,7 +327,7 @@ static const TypeInfo virt_machine_info = {
>>>>      .parent       = TYPE_MACHINE,
>>>>      .abstract     = true,
>>>>      .class_init   = virt_machine_class_init,
>>>> -    .is_available = target_m68k,
>>>> +    .is_available = target_is_m68k,
>>>>  };
>>>>
>>>>  static void virt_machine_register_types(void)
>>>> @@ -355,7 +355,7 @@ type_init(virt_machine_register_types)
>>>>          .name = MACHINE_VER_TYPE_NAME("virt", __VA_ARGS__), \
>>>>          .parent = MACHINE_TYPE_NAME("virt"), \
>>>>          .class_init = MACHINE_VER_SYM(class_init, virt, __VA_ARGS__), \
>>>> -        .is_available = target_m68k, \
>>>> +        .is_available = target_is_m68k, \
>>>>      }; \
>>>>      static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \
>>>>      { \
>>>> diff --git a/hw/or1k/virt.c b/hw/or1k/virt.c
>>>> index a3ff903839b..89fb786641f 100644
>>>> --- a/hw/or1k/virt.c
>>>> +++ b/hw/or1k/virt.c
>>>> @@ -559,7 +559,7 @@ static const TypeInfo or1ksim_machine_typeinfo = {
>>>>      .parent     = TYPE_MACHINE,
>>>>      .class_init = openrisc_virt_machine_init,
>>>>      .instance_size = sizeof(OR1KVirtState),
>>>> -    .is_available = target_or1k,
>>>> +    .is_available = target_is_or1k,
>>>>  };
>>>>
>>>>  static void or1ksim_machine_init_register_types(void)
>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>> index 4c0645817a5..7d52a15f6b8 100644
>>>> --- a/include/qom/object.h
>>>> +++ b/include/qom/object.h
>>>> @@ -17,6 +17,7 @@
>>>>  #include "qapi/qapi-builtin-types.h"
>>>>  #include "qemu/module.h"
>>>>
>>>> +typedef struct TargetInfo TargetInfo;
>>>>  struct TypeImpl;
>>>>  typedef struct TypeImpl *Type;
>>>>
>>>> @@ -117,6 +118,15 @@ typedef void (ObjectUnparent)(Object *obj);
>>>>   */
>>>>  typedef void (ObjectFree)(void *obj);
>>>>
>>>> +/**
>>>> + * typedef TypeIsAvailable:
>>>> + * @ti: TargetInfo to test.
>>>> + *
>>>> + * Returns whether a type is available for @ti. Typical implementations
>>>> + * are target_is_*().
>>>> + */
>>>> +typedef bool (TypeIsAvailable)(const TargetInfo *ti);
>>>> +
>>>>  #define OBJECT_CLASS_CAST_CACHE 4
>>>>
>>>>  /**
>>>> @@ -498,7 +508,7 @@ struct TypeInfo
>>>>      void (*class_base_init)(ObjectClass *klass, const void *data);
>>>>      const void *class_data;
>>>>
>>>> -    bool (*is_available)(void);
>>>> +    TypeIsAvailable *is_available;
>>>>      const InterfaceInfo *interfaces;
>>>>  };
>>>>
>>>> diff --git a/qom/object.c b/qom/object.c
>>>> index 96498f3d001..253b9562b41 100644
>>>> --- a/qom/object.c
>>>> +++ b/qom/object.c
>>>> @@ -15,6 +15,7 @@
>>>>  #include "qom/compat-properties.h"
>>>>  #include "qom/object.h"
>>>>  #include "qom/object_interfaces.h"
>>>> +#include "qemu/target-info.h"
>>>>  #include "qemu/cutils.h"
>>>>  #include "qemu/memalign.h"
>>>>  #include "qapi/visitor.h"
>>>> @@ -167,7 +168,7 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
>>>>          abort();
>>>>      }
>>>>
>>>> -    if (info->is_available && !info->is_available()) {
>>>> +    if (info->is_available && !info->is_available(target_info())) {
>>>>          return NULL;
>>>>      }
>>>>
>>>
>>> ```
>>> info->is_available(target_info())
>>> ```
>>>
>>> or
>>>
>>> ```
>>> is_available() {
>>>   TargetInfo ti * = target_info();
>>>   ...
>>> }
>>> ```
>>>
>>> Are doing the exact same thing today.
>>>
>>> So what is the future goal requiring to introduce this?
>>
>> From Daniel P. Berrangé's
>> requesthttps://lore.kernel.org/qemu-devel/apAszS1ewxM0Y1XU@redhat.com/
>>         I forgot to mention that this is also pretty important for making
>>         introspection work, as we need to be able to say
>>
>>           'qemu-system --machine none -qmp ...'
>>
>>         and connect to QMP to introspect the binary. Having to add a
>>         '--target <name>' into that mix is undesirable, as it creates
>>         a chicken & egg problem. You don't know what targets are built
>>         into qemu-system without introspecting it and you can't
>>         introspect it without selecting a target to introspect.
>>
>> From Pierrick's review
>> https://lore.kernel.org/qemu-devel/557262df-b8e7-49b5-a245-ae46b8885b30@oss.qualcomm.com/So,
>> for existing binary: -M virt works as before.
>>
>>         For new multi binary:
>>         $ qemu-system -M virt:
>>         Multiple machines were found, select one among:
>>         - aarch64-virt
>>         - riscv64-virt
>>
>>         Does it matches what you would expect?
>>
>> From Daniel's review
>> https://lore.kernel.org/qemu-devel/aro93NuZBXYGbD33@redhat.com/
>>         We already have  'QemuTargetInfo' struct which is used
>>         with 'query-target', that should also be used with the
>>         new 'query-targets' command.
>>
>>         We need to add the 'endianness' field to QemuTargetInfo.
>>
>>         I don't think that we need to report machines though.
>>         We already have 'query-machines' which gives much broader
>>         information on the machines. "MachineInfo" needs to
>>         gain
>>
>>         "targets": ["SysEmuTarget"]
>>
>>         to list which target(s) a machine can be used with.
>>
>>
>> To achieve these, we needs add TypeInfo parameter to it so that we can do
>>
>> static void report_unsupported_machine_type(const char *name, Error **errp)
>> {
>>     g_autoptr(GSList) machines = machine_class_get_list(true);
>>     g_autoptr(GString) targets = g_string_new(NULL);
>>     TargetInfoNode *node;
>>     GSList *el;
>>
>>     error_setg(errp, "unsupported machine type: \"%s\" for target \"%s\"",
>>                name, target_name());
>>     QLIST_FOREACH(node, target_info_list(), next) {
>>         for (el = machines; el; el = el->next) {
>>             MachineClass *mc = el->data;
>>
>>             if ((!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) &&
>>                 object_class_get_is_available(OBJECT_CLASS(mc))(node->info)) {
>>                 g_string_append_printf(targets, "  %s\n",
>>                                        node->info->target_name);
>>                 break;
>>             }
>>         }
>>     }
>>     if (targets->len) {
>>         error_append_hint(errp, "Available for targets:\n%s", targets->str);
>>     }
>> }
>>
>> That is we can iterate on all target-list, and check if it's can be fitlered.
>>
> 
> Ok, I finally see where this is going.
> 
> So, based on all the previous conversations, we agreed to have a
> qemu-system binary with all the QOM types available (for all targets).
> 
> I personally think that if we go this way, we should not have any
> -target option, and even less a -target none option.
> 
> The current is_available() approach was made with filtering per target
> in mind, which predates where things are now. So, it's a good time to
> reevaluate how to do it.
> 
> This leaves us with (at least) two options:
> - keep the callback and add TargetInfo parameter (what is done here)
> - remove is_available, and adopt a more declarative approach
> 
> How about we (re)-explore the second one?
> 
> It's what we did in the past
> (https://gitlab.com/qemu-project/qemu/-/commit/5de9216064d32eaac125dac4de1878486c16d73e),
> and that I removed recently, in favor of is_available.
> My rationale was that it was incomplete, since we needed to filter types
> per config also, and not only target.
> Now that direction is to not filter anything, filter per config is
> unneeded, and switching back to declarative instead of callback makes
> total sense. Using interfaces like we did has its own issues, and I
> would not like to see this coming back.
> 
> I'll do one very concrete proposal:
> 
> ```
> How about letting machines (and machines only first), have a new field
> 'targets', being a uint32_t mask with an array of EMU_TARGET?
> machine->targets = EMU_TARGET_X | EMU_TARGET_Y;
> ```
>

Machine class more exactly: mc->targets
See mc->desc for a concrete example of where it should be set.

> @Daniel, IIUC, this is exactly the approach you proposed a few weeks
> ago, just before I got frustrated and lost my patience against you. I
> think it's more clear for me where you wanted to go. Sorry it took such
> a large detour.
> 
> Yonggang, does that sounds good for you?
> Daniel, does that sounds good for you also?
> 
> Regards,
> Pierrick