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

[Pierrick Bouvier <pierrick.bouvier@xxxxxxxxxxxxxxxx>]

On 11/3/2026 5:55 AM, Daniel P. Berrangé wrote:
> On Tue, Nov 03, 2026 at 09:48:44PM +0800, Yonggang Luo wrote:
>> On Tue, Nov 3, 2026 at 9:23 PM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
>>>
>>> On Tue, 3 Nov 2026, Yonggang Luo wrote:
>>>> On Tue, Nov 3, 2026 at 4:16 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> 
>>>> wrote:
>>>>>
>>>>> On Fri, Oct 30, 2026 at 05:51:44PM -0800, 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.
>>>>>
>>>>> For each machine, it is only written once, but we add machines
>>>>> over time, and every time a new machine is added the code style
>>>>> is slightly different. We introduced all the variuos DEFINE_*
>>>>> macros for QOM to better ensure consistent style and usage as
>>>>> new code is added over time.
>>>>
>>>> I understand that the DEFINE_* macros are bloating because of this.
>>>> Indeed currently the only two macros used are "DEFINE_MACHINE" and
>>>> "DEFINE_MACHINE_EXTENDED"
>>>
>>> There are a few machines using some others but these are the most used.
>>> That suggests maybe we should change the convenience wrappers to match
>>> what machines need but DEFINE_MACHINE_EXTENDED is meant to be the one that
>>> allows all parameters which should be rarely needed.
>>>
>>>> ; each time people want to add a new attribute, the macros is twiced.
>>>
>>> Not necessarily. The SECURE parameter did that to preserve previous
>>> behaviour but if you add new parameter changing all callers (which is the
>>> same as expanding the macro that would also change all callers) then it's
>>> just a new parameter added not changing the number of DEFINE_MACHINE*
>>> wrappers. Rearranging the wrappers to make DEFINE_MACHINE_EXTENDED used
>>> less is an independent change.
>>>
>>>> And the usage of DEFINE_MACHINE_EXTENDED is rare and can be replaced
>>>> with DEFINE_TYPES or TypeInfo
>>>> and be more explicit. When we used as macro parameter, we need think
>>>
>>> But that defies the purpose of hiding the QOM boiler plate as much as
>>> possible. New people are confused by QOM and it's easier for them to look
>>> up documentation on the DEFINE_MACHINE* macros to see what parameter is
>>> what and not care about how it's stored in QOM TypeInfo or what is that
>>> at all.
>>>
>>>> what's that parameter means, and for
>>>> TypeInfo, we can directly know what that member is about(because we
>>>> are assigned with the member name).
>>>
>>> The parameter also has a name and documentation so it's not simpler to
>>> look up TypeInfo than look at the macro definition or docs. In fact it's
>>> harder to guess from TypeInfo as then you also need to get into QOM.
>>>
>>>> That's why I suggest keeping a single DEFINE_MACHINE, just like
>>>> DEFINE_TYPES; as they are the most
>>>> used macros(and simple enough), and won't suffer from changes much.
>>>
>>> That means exposing all the boiler plate for all machines other than the
>>> simplest case. I think it would be better to keep DEFINE_MACHINE_EXTENDED
>>> but maybe rethink the existing wrappers and drop unused ones and maybe add
>>> new ones that reduce the usage of DEFINE_MACHINE_EXTENDED but still keep
>>> the boiler plate hidden.
>>
>> Thanks, I'll go this way, remove unused macros, and keep 
>> DEFINE_MACHINE_EXTENDED
>> and extend both DEFINE_MACHINE_EXTENDED DEFINE_MACHINE supports
>> "targets" or "arch_mask" parameter.
> 
> Given that we don't need to do general per-arch type filtering in
> QOM, we could just move 'is_available' out of TypeInfo, and into the
> MachineClass, so it is a regular class level property, which can be
> set in the class_init methods, and thus avoid any changes to the
> DEFINE_MACHINE macros.
>

Some cpus need it also, and it's used for filtering purpose at this time
(to avoid duplication at registration). We already depend on this behavior.

> With regards,
> Daniel

Regards,
Pierrick