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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 18/4/25 18:30, Pierrick Bouvier wrote:
> 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.

Correct.

> 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.

Which file? target_info-qom.c is already in system_ss[].