Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/18/25 07:07, Philippe Mathieu-Daudé wrote:
> On 18/4/25 05:07, Pierrick Bouvier wrote:
>> On 4/17/25 17:50, Philippe Mathieu-Daudé wrote:
>>> Define the TYPE_TARGET_ARM_MACHINE and TYPE_TARGET_AARCH64_MACHINE
>>> QOM interface names to allow machines to implement them.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    meson.build                    |  1 +
>>>    include/hw/boards.h            |  1 +
>>>    include/qemu/target_info-qom.h | 18 ++++++++++++++++++
>>>    target_info-qom.c              | 24 ++++++++++++++++++++++++
>>>    4 files changed, 44 insertions(+)
>>>    create mode 100644 include/qemu/target_info-qom.h
>>>    create mode 100644 target_info-qom.c
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 49a050a28a3..168b07b5887 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -3808,6 +3808,7 @@ common_ss.add(pagevary)
>>>    specific_ss.add(files('page-target.c', 'page-vary-target.c'))
>>>    common_ss.add(files('target_info.c'))
>>> +system_ss.add(files('target_info-qom.c'))
>>>    specific_ss.add(files('target_info-stub.c'))
>>>    subdir('backends')
>>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>>> index 02f43ac5d4d..b1bbf3c34d4 100644
>>> --- a/include/hw/boards.h
>>> +++ b/include/hw/boards.h
>>> @@ -7,6 +7,7 @@
>>>    #include "system/hostmem.h"
>>>    #include "system/blockdev.h"
>>>    #include "qapi/qapi-types-machine.h"
>>> +#include "qemu/target_info-qom.h"
>>>    #include "qemu/module.h"
>>>    #include "qom/object.h"
>>>    #include "hw/core/cpu.h"
>>> diff --git a/include/qemu/target_info-qom.h b/include/qemu/
>>> target_info-qom.h
>>> new file mode 100644
>>> index 00000000000..7eb9b6bd254
>>> --- /dev/null
>>> +++ b/include/qemu/target_info-qom.h
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + * QEMU binary/target API (QOM types)
>>> + *
>>> + *  Copyright (c) Linaro
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#ifndef QEMU_TARGET_INFO_QOM_H
>>> +#define QEMU_TARGET_INFO_QOM_H
>>> +
>>> +#define TYPE_TARGET_ARM_MACHINE \
>>> +        "target-info-arm-machine"
>>> +
>>> +#define TYPE_TARGET_AARCH64_MACHINE \
>>> +        "target-info-aarch64-machine"
>>> +
>>> +#endif
>>> diff --git a/target_info-qom.c b/target_info-qom.c
>>> new file mode 100644
>>> index 00000000000..d3fee57361b
>>> --- /dev/null
>>> +++ b/target_info-qom.c
>>> @@ -0,0 +1,24 @@
>>> +/*
>>> + * QEMU binary/target API (QOM types)
>>> + *
>>> + *  Copyright (c) Linaro
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qemu/target_info-qom.h"
>>> +#include "qom/object.h"
>>> +
>>> +static const TypeInfo target_info_types[] = {
>>> +    {
>>> +        .name           = TYPE_TARGET_ARM_MACHINE,
>>> +        .parent         = TYPE_INTERFACE,
>>> +    },
>>> +    {
>>> +        .name           = TYPE_TARGET_AARCH64_MACHINE,
>>> +        .parent         = TYPE_INTERFACE,
>>> +    },
>>> +};
>>> +
>>> +DEFINE_TYPES(target_info_types)
>>
>> Ideally, this should be in target/arm, as this type will only be used by
>> boards in hw/arm, and by the target_info specialization.
> 
> Not the way QOM works, interfaces must be registered, which is
> why I use this centralized file. Otherwise we get:
> 
> $ qemu-system-sh4 -M help
> qemu-system-sh4: -M help: missing interface 'target-info-arm-machine'
> for object 'machine'
> 

If I'm correct, types can be registered anywhere, since they rely on 
static initializer, but in qemu-system-sh4, hw/arm or target/arm is not 
linked, so it fails.
I guess this is the null board that is creating this situation, since 
it's included by all binaries.

I see two solutions while still moving those types in target/arm:
- include this type file in libcommon, so it's always linked.
- introduce a new TYPE_TARGET_ALL_MACHINE, and always include it in list 
of machines. But I think it's not so good as it would require to deal 
with a list of types when we want to access a machine.

So I would just move the file and link it inconditonnally in all binaries.

How about that?