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

Philippe Mathieu-Daudé posted 11 patches 9 months, 3 weeks ago
There is a newer version of this series
[RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago
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)
-- 
2.47.1


Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Pierrick Bouvier 9 months, 3 weeks ago
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.
Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago
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'


Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Pierrick Bouvier 9 months, 3 weeks ago
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?
Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago
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[].

Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Pierrick Bouvier 9 months, 3 weeks ago
On 4/18/25 10:04, Philippe Mathieu-Daudé wrote:
> 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[].

Move existing target_info-qom in in target/arm/target_info_types.c, and 
add it to system_ss, instead of arm_ss or arm_common_ss, so it's always 
linked.
Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago
On 18/4/25 19:07, Pierrick Bouvier wrote:
> On 4/18/25 10:04, Philippe Mathieu-Daudé wrote:
>> 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[].
> 
> Move existing target_info-qom in in target/arm/target_info_types.c, and 
> add it to system_ss, instead of arm_ss or arm_common_ss, so it's always 
> linked.

That should work, but what is the benefit over having a single file
where all definitions are concentrated?


Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Pierrick Bouvier 9 months, 3 weeks ago
On 4/18/25 10:10, Philippe Mathieu-Daudé wrote:
> On 18/4/25 19:07, Pierrick Bouvier wrote:
>> On 4/18/25 10:04, Philippe Mathieu-Daudé wrote:
>>> 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[].
>>
>> Move existing target_info-qom in in target/arm/target_info_types.c, and
>> add it to system_ss, instead of arm_ss or arm_common_ss, so it's always
>> linked.
> 
> That should work, but what is the benefit over having a single file
> where all definitions are concentrated?
> 

Limit which types are visible from every hw/X, so it's explicit which 
architecture every board/cpu/device will implement in the end.
I don't mind if you want to keep it there for now, it doesn't really matter.
Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago
On 18/4/25 19:23, Pierrick Bouvier wrote:
> On 4/18/25 10:10, Philippe Mathieu-Daudé wrote:
>> On 18/4/25 19:07, Pierrick Bouvier wrote:
>>> On 4/18/25 10:04, Philippe Mathieu-Daudé wrote:
>>>> 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[].
>>>
>>> Move existing target_info-qom in in target/arm/target_info_types.c, and
>>> add it to system_ss, instead of arm_ss or arm_common_ss, so it's always
>>> linked.
>>
>> That should work, but what is the benefit over having a single file
>> where all definitions are concentrated?
>>
> 
> Limit which types are visible from every hw/X, so it's explicit which 
> architecture every board/cpu/device will implement in the end.
> I don't mind if you want to keep it there for now, it doesn't really 
> matter.

Oh, then I think I understood you and we are good:

Instead of having a single include/qemu/target_info-qom.h with
all target definitions, I added the ARM* ones to
include/hw/arm/machines-qom.h. But I kept target_info_types[]
in a single common source file.

Re: [RFC PATCH v2 04/11] hw/arm: Register TYPE_TARGET_ARM/AARCH64_CPU QOM interfaces
Posted by Pierrick Bouvier 9 months, 3 weeks ago
On 4/18/25 10:32, Philippe Mathieu-Daudé wrote:
> On 18/4/25 19:23, Pierrick Bouvier wrote:
>> On 4/18/25 10:10, Philippe Mathieu-Daudé wrote:
>>> On 18/4/25 19:07, Pierrick Bouvier wrote:
>>>> On 4/18/25 10:04, Philippe Mathieu-Daudé wrote:
>>>>> 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[].
>>>>
>>>> Move existing target_info-qom in in target/arm/target_info_types.c, and
>>>> add it to system_ss, instead of arm_ss or arm_common_ss, so it's always
>>>> linked.
>>>
>>> That should work, but what is the benefit over having a single file
>>> where all definitions are concentrated?
>>>
>>
>> Limit which types are visible from every hw/X, so it's explicit which
>> architecture every board/cpu/device will implement in the end.
>> I don't mind if you want to keep it there for now, it doesn't really
>> matter.
> 
> Oh, then I think I understood you and we are good:
> 
> Instead of having a single include/qemu/target_info-qom.h with
> all target definitions, I added the ARM* ones to
> include/hw/arm/machines-qom.h. But I kept target_info_types[]
> in a single common source file.

All good.