Re: [PATCH 01/27] include/qemu/target-info-qom.h: declare TYPE_TARGET_SPECIFIC interface
Pierrick Bouvier <[email protected]>
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
On 8/12/2026 8:52 AM, Pierrick Bouvier wrote:
> On 8/12/2026 7:26 AM, Daniel P. Berrangé wrote:
>> On Wed, Aug 12, 2026 at 07:09:29AM -0700, Pierrick Bouvier wrote:
>>> On 8/12/2026 6:03 AM, Daniel P. Berrangé wrote:
>>>> On Wed, Aug 12, 2026 at 05:54:49AM -0700, Pierrick Bouvier wrote:
>>>>> On 8/12/2026 3:52 AM, Daniel P. Berrangé wrote:
>>>>>> On Mon, Aug 10, 2026 at 03:26:26PM -0700, Pierrick Bouvier wrote:
>>>>>>> I have been experimenting today with static filtering of types, as you
>>>>>>> suggested, and came up with two possibilities. Before implementing this
>>>>>>> for all types, I would like to get some feedback on which way is the
>>>>>>> best for you.
>>>>>>>
>>>>>>> 1. Add a new callback is_available to TypeInfo, similar to existing
>>>>>>> interface.
>>>>>>> Pros: local information is present in type definition, making obvious
>>>>>>> why a type is available or not. Very easy to implement
>>>>>>> since only type_register_static has to be modified to check it.
>>>>>>> Cons: Add a new field to each existing type. Not the end of the world
>>>>>>> in terms of memory consumption, but worth mentioning.
>>>>>>> 2. Add conditional around each type_register_static, or add an
>>>>>>> alternative type_register_static_cond function.
>>>>>>> Pros: I don't see any
>>>>>>> Cons: Decouples type information from type definition, makes it harder
>>>>>>> to follow why a type is available or not. Much more verbose since
>>>>>>> we need to modify all type_register_static buried in macros.
>>>>>>>
>>>>>>> Based on this, I'm much more in favor or 1. The fact availability
>>>>>>> information is in the same location than type definition is the biggest
>>>>>>> advantage for me.
>>>>>>>
>>>>>>> On example given above, this would give something like this:
>>>>>>> static const TypeInfo emulated_card_info = {
>>>>>>> .name = TYPE_EMULATED_CCID,
>>>>>>> .parent = TYPE_CCID_CARD,
>>>>>>> .instance_size = sizeof(EmulatedState),
>>>>>>> .class_init = emulated_class_initfn,
>>>>>>> .is_available = target_config_X,
>>>>>>> };
>>>>>>>
>>>>>>> Would that work for you?
>>>>>>> Do you have a 3rd way to offer to solve the problem we have?
>>>>>>
>>>>>> In the case of user creatable objects, we represented a classes'
>>>>>> conditional availablity in QAPI schema. For example:
>>>>>>
>>>>>> { 'struct': 'InputLinuxProperties',
>>>>>> 'data': { 'evdev': 'str',
>>>>>> '*grab_all': 'bool',
>>>>>> '*repeat': 'bool',
>>>>>> '*grab-toggle': 'GrabToggleKeys' },
>>>>>> 'if': 'CONFIG_LINUX' }
>>>>>>
>>>>>> Now this isn't quite a match for what we want, as QAPI schema is
>>>>>> common to all targets and used to control the code generator to
>>>>>> turn off output. Thus the permitted CONFIG_xxx are only those in
>>>>>> config-host.mak, not any from $TARGET-softmmu-config-devices.mak
>>>>>>
>>>>>> The other scenario in QAPI is the concept of features:
>>>>>>
>>>>>> { 'struct': 'SevCommonProperties',
>>>>>> 'data': { '*sev-device': 'str',
>>>>>> '*cbitpos': 'uint32',
>>>>>> 'reduced-phys-bits': 'uint32',
>>>>>> '*kernel-hashes': 'bool' },
>>>>>> 'features': ['confidential-guest-reset']}
>>>>>>
>>>>>> where again we just have a list of named strings. Features are fully
>>>>>> dynamic, and crucially they are introspectable so applications can
>>>>>> query at runtime what is available.
>>>>>>
>>>>>> The introspection concept is something that is highly likely to be
>>>>>> relevant to our use cases here. If we have a single binary with all
>>>>>> types, it would be very valuable to be able to query it once to
>>>>>> discover everything in one go, instead of havnig to query it over
>>>>>> and over again for each target. That implies the application doing
>>>>>> the query needs to be told about per-target usage restrictions on
>>>>>> types it is querying.
>>>>>>
>>>>>> We don't have QAPI schema for QDev devices currently, but it is likely
>>>>>> we will do so in the future. We can't wait for that though as there is
>>>>>> no clear ETA.
>>>>>>
>>>>>> What this all says to me though, is that instead of an 'is_available'
>>>>>> method, we should instead just list the conditions as data in the
>>>>>> TypeInfo struct directly. IOW a list of const strings like this:
>>>>>>
>>>>>> static const TypeInfo emulated_card_info = {
>>>>>> .name = TYPE_EMULATED_CCID,
>>>>>> .parent = TYPE_CCID_CARD,
>>>>>> .instance_size = sizeof(EmulatedState),
>>>>>> .class_init = emulated_class_initfn,
>>>>>> .available = (const char *[]){ TARGET_AARCH64, CONFIG_something .... },
>>>>>> };
>>>>>>
>>>>>> This will make it easy to add in introspection at a later date.
>>>>>>
>>>>>> The limitation is that this can only express an "AND" condition.
>>>>>> You can not do complex AND+OR+grouping conditions that you could
>>>>>> express in code, but IMHO that is not likely to be a problem.
>>>>>> In the worst case you can define a higher level "CONFIG_BLAH"
>>>>>> that encapsulates the complex condition.
>>>>>>
>>>>>
>>>>> Those strings names reuse existing target config identifiers, which
>>>>> can't be done because of config poisoning.
>>>>> The string compare will definitively be visible at boot time, so we
>>>>> should rely on enum instead if we go in this direction.
>>>>>
>>>>> I'm a bit worried about verbosity of the result:
>>>>> .available = (const char *[]){ ENUM_NAME_TARGET_AARCH64, {} }
>>>>> versus:
>>>>> is_available = target_aarch64
>>>>
>>>> The verbosity could be hidden behind a macro that takes a tar
>>>>
>>>> #define QOM_AVAIL_TGT(name) \
>>>> .available = (const char *[]) ENUM_TARGET # name, NULL }
>>>>
>>>> So usage becomes
>>>>
>>>> static const TypeInfo emulated_card_info = {
>>>> .name = TYPE_EMULATED_CCID,
>>>> .parent = TYPE_CCID_CARD,
>>>> .instance_size = sizeof(EmulatedState),
>>>> .class_init = emulated_class_initfn,
>>>> QOM_AVAIL_TGT(AARCH64),
>>>> }
>>>>
>>>> A separate macro could be defined for the more complex case
>>>> wanting multiple config options.
>>>>
>>>
>>> Then we'll have different macros for:
>>> one target, a collection of target, a mix of target and config, config
>>> only. Plus, we hide what was clear in the first place with:
>>> .is_available = target_aarch64.
>>> I'm not super fond of the result to be honest.
>>
>> That's a different interpration of "clear".
>>
>> What I'm proposing exposes the rules as data, and that ensures
>> the rules are clear to machines.
>>
>> What you're proposing exposes the rules as code and that is only
>> clear to human reviewers.
>>
>
> Agree, I will always prefer what is clear for human to read/understand.
> That said, your requirement is fair also, and I see the value in
> describing rules instead of a function pointer.
> We both agree to add a field in TypeInfo, that's a good start.
>
> I'll do my best to add something that satisfies both in next version.
>
>>> >> 99% of the types will be filtered by target only, for which we have
>>>>> functions in target-info API. Only a few of them will need custom
>>>>> functions. It would be worth having a nice/short way to write this
>>>>> without having to declare all combination in a file.
>>>>> One of your point was to not include more macros, and seems like we'll
>>>>> end up this way.
>>>>
>>>> In this case the macros expand to data that is machine consumable
>>>> at runtime, so I thjnk that's a good tradeoff.
>>>>
>>>>> The introspection argument is good, but will we really need that? The
>>>>> whole point of conversation was to register only types available,
>>>>> ignoring the rest. If you want introspection, it means registering all
>>>>> types and selectively filter them.
>>>>
>>>> Yes, introspection is critical to mgmt apps and places where we
>>>> forget/miss it cause endless pain.
>>>>
>>>
>>> We can easily make a mechanical replace on all '^\s*\.is_available'
>>> lines when it will be needed. Feels like we are anticipating a bit too
>>> much, and there would be no cost to change that later.
>>>
>>> I would kindly ask again to consider if you would be ok to delay this to
>>> when it's effectively introduced on QAPI side.
>>>
>>> Would you be open to it?
>>
>> Introspection has always been a critical problem in QEMU and rectifying
>> it after the fact is very costly. IMHO we need to do this as data from
>> day one rather than knowingly taking a path that makes introspection
>> harder.
>>
>>
>> With regards,
>> Daniel
>
> Thanks for the feedback, and glad we found a solution that works for
> both of us.
>
> Regards,
> Pierrick
Sent v2, and waiting for Daniel's feedback before going further:
https://lore.kernel.org/qemu-devel/[email protected]/T/#
Regards,
Pierrick