Re: [PATCH 00/15] hw: expand DEFINE_MACHINE to explicit TypeInfo registration

[BALATON Zoltan <balaton@xxxxxxxxxx>]

On Mon, 2 Nov 2026, Yonggang Luo wrote:
> On Mon, Nov 2, 2026 at 4:57 AM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
> > On Sun, 1 Nov 2026, Yonggang Luo wrote:
> > > On Sun, Nov 1, 2026 at 7:29 AM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
> > > > On Sun, 1 Nov 2026, Yonggang Luo wrote:
> > > > > On Sun, Nov 1, 2026 at 1:06 AM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
> > > > > > On Sat, 31 Oct 2026, Yonggang Luo wrote:
> > > > > > > On Sat, Oct 31, 2026 at 8:42 PM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
> > > > > > > > On Fri, 30 Oct 2026, Pierrick Bouvier wrote:
> > > > > > > > > On 10/30/2026 4:09 PM, BALATON Zoltan wrote:
> > > > > > > > > > On Fri, 30 Oct 2026, Yonggang Luo wrote:
> > > > > > > > > > > DEFINE_MACHINE and DEFINE_MACHINE_EXTENDED hide the TypeInfo and the
> > > > > > > > > > > type_init
> > > > > > > > > > > function that calls type_register_static(). A later change needs to
> > > > > > > > > > > pass a
> > > > > > > > > > That's the point of the macro to hide the boiler plate code.
> > > > > > > > > >
> > > > > > > > > > > target mask into that registration, and the macros have no parameter
> > > > > > > > > > > for it.
> > > > > > > > > > Couldn't you just change the macros to take that parameter instead of
> > > > > > > > > > expanding it everywhere?
> > > > > > > > > Problem of macros is that they don't have default parameters, so you
> > > > > > > > > need variants for everything:
> > > > > > > > >
> > > > > > > > > DEFINE_MACHINE
> > > > > > > > > DEFINE_MACHINE_EXTENDED
> > > > > > > > > DEFINE_MACHINE_WITH_INTERFACES
> > > > > > > > > DEFINE_MACHINE_WITH_INTERFACE_ARRAY
> > > > > > > > > and now would need a new variant, with variant for other existing ones.
> > > > > > > > >
> > > > > > > > > In the end, it's just simpler to remove this and add the boilerplate.
> > > > > > > > > It's written once and never modified, so using a macro just save typing
> > > > > > > > > those lines once.
> > > > > > > > >
> > > > > > > > > > Regards,
> > > > > > > > > > BALATON Zoltan
> > > > > > > > > >
> > > > > > > > > > > This series expands every caller first, with no behavior change, so the
> > > > > > > > > > > follow-up only replaces the type_register_static() call.
> > > > > > > > > > >
> > > > > > > > > > > Each machine file gains a class_init function, a TypeInfo, and a
> > > > > > > > > > > type_init
> > > > > > > > > > > function that calls type_register_static(). The last patch removes
> > > > > > > > > > > DEFINE_MACHINE* and DEFINE_SECURE_MACHINE* from include/hw/core/boards.h.
> > > > > > > > Macros help reading the code not saving typing. If the actual
> > > > > > > > functionality is buried in a lot of boiler plate it's harder to understand
> > > > > > > > and spot bugs so putting the unintresting boiler plate behind a macro is
> > > > > > > > better even if you need a separate version for different cases. Then the
> > > > > > > > code that uses the macro will be much more obvious and not just a lot of
> > > > > > > I think mainly because DEFINE_MACHINE* and DEFINE_SECURE_MACHINE* are
> > > > > > > not defined properly
> > > > > > > that causes them to not be extended easily.
> > > > > > >
> > > > > > > DEFINE_TYPES is another example, use them can extend easily, how about
> > > > > > > always use DEFINE_TYPES?
> > > > > > > Or add a DEFINE_TYPE to replace DEFINE_MACHINE* and DEFINE_SECURE_MACHINE*?
> > > > > > Maybe I'm missing the problem but DEFINE_MACHINE macros are just
> > > > > > convenience wrappers around DEFINE_MACHINE_EXTENDED. So why can't you add
> > > > > > the needed parameter to DEFINE_MACHINE_EXTENDED and use that for machines
> > > > > > that need it, or if it's needed by all machines then make the new
> > > > > > parameter required and add it to all convenience wrappers too so you can
> > > > > > keep using DEFINE_MACHINE* and just need to add one parameter to it in
> > > > > > machines?
> > > > > That's the issue, add parameter to DEFINE_MACHINE_EXTENDED means all
> > > > > calls to it also need
> > > > > to be changed, that means touch many $target_arch files. currently
> > > > > it's 13. But after these
> > > > That's not different from expanding the macro which would touch the same
> > > > files.
> > > >
> > > > > changes, there will be much more usage on it, and if we want to add
> > > > > more parameters or attributes
> > > > > to it, it will be a new tree-wide change.
> > > > Do you expect to need more parameters? Can't you add them at once then?
> > > >
> > > > > And DEFINE_TYPES do not suffer from such issues. As the TypeInfo[] is
> > > > > explicit defined for
> > > > > DEFINE_TYPES
> > > > My point is that keeping the boiler plate as little as possible makes the
> > > > code more readable. DEFINE_TYPES hides some of the boiler plate,
> > > > DEFINE_MACHINE hides even more so converting to DEFINE_TYPES would expose
> > > > some even if it's better than no macro at all.
> > > >
> > > > What exactly are the parameters missing from DEFINE_MACHINE_EXTENDED that
> > > > need to be added?
> > > Hi BALATON
> > > the things we faced is we first is to add TypeInfo::is_available parameter to
> > > DEFINE_MACHINE_EXTENDED, and have more usage of it.
> > > Then we will add targets:uint64_t parameter to DEFINE_MACHINE_EXTENDED, this is
> > > a prepare to remove TypeInfo::is_available parameter from
> > > DEFINE_MACHINE_EXTENDED,
> > >
> > > And finally, once all usage of TypeInfo::is_available can be replaced with
> > > targets:uint64_t, then we finally need to remove
> > > TypeInfo::is_available parameter from DEFINE_MACHINE_EXTENDED,
> > Is there a way to avoid the temporary is_available churn and introduce the
> > targets:uint64_t right away? (I'm not following this so I don't really
> > know what you're talking about just looks like there's some churn here
> > which might be good to awoid anyway independent of these macros.)
> >
> > > That's what I am facing now. And the explicit
> > > static const TypeInfo xxx_machine_typeinfo = {
> > >    .name       = MACHINE_TYPE_NAME("xxx"),
> > >    .parent     = TYPE_MACHINE,
> > >    .class_init = xxx_machine_class_init,
> > > };
> > >
> > > Will help me avoid the issue. And I think this is a little boilerplate
> > > and very explicit.
> > That's not all DEFINE_MACHINE hides though. It's longer with .abstract,
> > .secure and .iterfaces plus the machine initfn that would also need to be
> > expanded if you used DEFINE_TYPES.
> >
> > Is this conversion needed for all machines using all DEFINE_MACHINE
> At the end, only "none" machine do not have targets parameter or
> (QEMU_ARCH_ALL), as that's the sole
> machine that supports all targets.
>
> > variants or only some of them? If the latter why not convert those to
>
>
>
> > DEFINE_MACHINE_EXTENDED and then only that macro needs to be changed with
> > other wrappers passing a default value without needing a new parameter.
> > That does not work if all machines need to do this but even then, keeping
> > the macro and adding a new required parameter is better in my opinion as
> > it keeps boiler plate hidden.
> Another option is keep DEFINE_MACHINE and introduce DEFINE_MACHINE_TARGETS
> and remove DEFINE_MACHINE_EXTENDED, and we only use DEFINE_MACHINE for
> simple case,
> that is parent is TYPE_MACHINE and have no custom type and no more
> attributes assigned.
> DEFINE_MACHINE_EXTENDED is too verbose, when calling it, it less clear
> than calling to DEFINE_MACHINE.
> And if we want to set more attributes to TypeInfo, just expand it to
> use type_init or DEFINE_TYPES, how about this way.
>
> This will be something like:
>
> #define DEFINE_MACHINE_TARGETS(namestr, machine_initfn, targets) \
>    static void machine_initfn##_class_init(ObjectClass *oc, const void *data) \
>    { \
>        MachineClass *mc = MACHINE_CLASS(oc); \
>        machine_initfn(mc); \
>    } \
>    static const TypeInfo machine_initfn##_typeinfo = { \
>        .name       = MACHINE_TYPE_NAME(namestr), \
>        .parent     = TYPE_MACHINE, \
>        .class_init = machine_initfn##_class_init, \
>    }; \
>    static void machine_initfn##_register_types(void) \
>    { \
>        type_register_targets(&machine_initfn##_typeinfo, targets); \
>    } \
>    type_init(machine_initfn##_register_types)
>
> /* Implicitly insecure */
> #define DEFINE_MACHINE(namestr, machine_initfn) \
>    DEFINE_MACHINE_TARGETS(namestr, machine_initfn, QEMU_ARCH_ALL) \
>
> As the many call is to DEFINE_MACHINE.
>
> And after all conversion, all existing usage of DEFINE_MACHINE will be
> converted to
> DEFINE_MACHINE_TARGETS.
> DEFINE_MACHINE will be called for ('none') machine or future heterogeneous
> machines
>
>
> Either way, these patches will be dropped. We will introduce
> DEFINE_MACHINE_TARGETS first.
I think I'm lost and cannot follow what you want. As far as I understood 
you want to add a new field "target" to machine TypeInfo but instead of 
adding that straight away you proposed to first add is_available then 
later replace that with target. I asked why do you need is_available and 
not add target right away?
Then if this target parameter will be needed for all machines why can't 
you add that to all DEFINE_MACHINE macros as a required parameter and set 
it for all machines passing it through to DEFINE_MACHINE_EXTENDED which 
sets in the TypeInfo. That way no machines need changing other than adding 
the target parameter to the existing DEFINE_MACHINE calls.
What am I missing?

Regards,
BALATON Zoltan