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

[Yonggang Luo <luoyonggang@xxxxxxxxx>]

On Mon, Nov 2, 2026 at 10:56 PM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
>
> 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?
Or sorry, I did not explain well.

I means reshape DEFINE_MACHINE like

https://lore.kernel.org/qemu-devel/20261102194826.1298-1-luoyonggang@xxxxxxxxx/T/#u

basically every call to DEFINE_MACHINE needs the targets parameter.
So we can always passing targets parameter for DEFINE_MACHINE.

And I think we can remove DEFINE_MACHINE_EXTENDED, as the call to it is verbose



>
> Regards,
> BALATON Zoltan



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