Re: [RFC PATCH v5 08/21] hw/arm: Add DEFINE_MACHINE_[ARM_]AARCH64() macros

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 2774 bytes --]

On Fri, 25 Apr 2025, Philippe Mathieu-Daudé wrote:
> A machine defined with the DEFINE_MACHINE_ARM_AARCH64() macro
> will be available on qemu-system-arm and qemu-system-aarch64
> binaries.
>
> One defined with DEFINE_MACHINE_AARCH64() will only be available
> in the qemu-system-aarch64 binary.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/arm/machines-qom.h | 13 +++++++++++++
> target/arm/machine.c          | 12 ++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/include/hw/arm/machines-qom.h b/include/hw/arm/machines-qom.h
> index a17225f5f92..6277ee986d9 100644
> --- a/include/hw/arm/machines-qom.h
> +++ b/include/hw/arm/machines-qom.h
> @@ -9,10 +9,23 @@
> #ifndef HW_ARM_MACHINES_QOM_H
> #define HW_ARM_MACHINES_QOM_H
>
> +#include "hw/boards.h"
> +
> #define TYPE_TARGET_ARM_MACHINE \
>         "target-info-arm-machine"
>
> #define TYPE_TARGET_AARCH64_MACHINE \
>         "target-info-aarch64-machine"
>
> +extern InterfaceInfo arm_aarch64_machine_interfaces[];
> +extern InterfaceInfo aarch64_machine_interfaces[];
> +
> +#define DEFINE_MACHINE_ARM_AARCH64(namestr, machine_initfn) \
> +        DEFINE_MACHINE_WITH_INTERFACES(namestr, machine_initfn, \
> +                                       arm_aarch64_machine_interfaces)
> +
> +#define DEFINE_MACHINE_AARCH64(namestr, machine_initfn) \
> +        DEFINE_MACHINE_WITH_INTERFACES(namestr, machine_initfn, \
> +                                       aarch64_machine_interfaces)
> +
> #endif
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index 978249fb71b..193c7a9cff0 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -8,6 +8,7 @@
> #include "cpu-features.h"
> #include "migration/cpu.h"
> #include "target/arm/gtimer.h"
> +#include "hw/arm/machines-qom.h"
>
> static bool vfp_needed(void *opaque)
> {
> @@ -1111,3 +1112,14 @@ const VMStateDescription vmstate_arm_cpu = {
>         NULL
>     }
> };
> +
> +InterfaceInfo arm_aarch64_machine_interfaces[] = {
> +    { TYPE_TARGET_ARM_MACHINE },
> +    { TYPE_TARGET_AARCH64_MACHINE },
> +    { }
> +};
> +
> +InterfaceInfo aarch64_machine_interfaces[] = {
> +    { TYPE_TARGET_AARCH64_MACHINE },
> +    { }
> +};

Why do you need these? If you define DEFINE_MACHINE_WITH_INTERFACES as 
OBJECT_DEFINE_TYPE_WITH_INTERFACES then you can write:

DEFINE_MACHINE_WITH_INTERFACES(name, initfn, { TYPE_TARGET_ARM_MACHINE },
     { TYPE_TARGET_AARCH64_MACHINE }, { })

and no more macros needed. Ideally those places that are now blown up 
should use DEFINE_MACHINE too. Maybe they don't yet because the parent
type  is hardcoded so we should really have

DEFINE_MACHINE_EXTENDED(name, parent, initfn, interfaces...)

and remove more bolier plate that way?

Regards,
BALATON Zoltan