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

[Yonggang Luo <luoyonggang@xxxxxxxxx>]

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.

>
> Regards,
> BALATON Zoltan
>
> >
> > And the
> >    .instance_size = sizeof(MachineState),
> > is indeed not needed, they sharing TYPE_MACHINE's sizeof(MachineState)
> > is enough.
> > Even though DEFINE_MACHINE expands to .instance_size =
> > sizeof(MachineState), and it's redundant.
> > So I think convert to DEFINE_TYPES would be a good alternative to this
> > issue, less boilerplate(not none)
> > And dealt with the issue we faced.
> >
> > What do you think about this idea, BALATON
> >
> >>
> >> Regards,
> >> BALATON Zoltan
> >
> >
> >
> >



-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo