Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/5/21 9:07 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> Introduce the 'query-accels' QMP command which returns a list
>>> of built-in accelerator names.
>>>
>>> - Accelerator is a QAPI enum of all existing accelerators,
>>>
>>> - AcceleratorInfo is a QAPI structure providing accelerator
>>>   specific information. Currently the common structure base
>>>   provides the name of the accelerator, while the specific
>>>   part is empty, but each accelerator can expand it.
>>>
>>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>>
>>> For example on a KVM-only build we get:
>>>
>>>     { "execute": "query-accels" }
>>>     {
>>>         "return": [
>>>             {
>>>                 "name": "qtest"
>>>             },
>>>             {
>>>                 "name": "kvm"
>>>             }
>>>         ]
>>>     }
>>>
>>> Note that we can't make the enum values or union branches conditional
>>> because of target-specific poisoning of accelerator definitions.
>> 
>> Let me try to complete this argument;
>> 
>>   If we did, enum Accelerator could only be used in target-specific
>>   code.  But we want to use it in generic code, too.
>> 
>> Which generic code exactly?
>
>
> cpu.c:133:#ifdef CONFIG_TCG
> hmp-commands-info.hx:271:#if defined(CONFIG_TCG)
> monitor/misc.c:324:#ifdef CONFIG_TCG
> softmmu/physmem.c:28:#ifdef CONFIG_TCG
>
> and more importantly:
>
> include/exec/cpu-all.h:430:#ifdef CONFIG_TCG
> include/exec/cpu-defs.h:77:#if !defined(CONFIG_USER_ONLY) &&
> defined(CONFIG_TCG)
> include/exec/exec-all.h:25:#ifdef CONFIG_TCG
> include/sysemu/cpu-timers.h:27:#ifdef CONFIG_TCG
>
> Thomas might provide more cases, IIRC he audited this recently.

No need, I'm interested in examples, not a complete list.

Please amend the commit message:

    Note that we can't make the enum values or union branches conditional
    because of target-specific poisoning of accelerator definitions.
    If we did, enum Accelerator could only be used in target-specific
    code.  But we want to also use it in generic code, such as ...

with ... replaced by one or more references to relevant code.

If we expect readers of the code to later wonder why we're not using
QAPI conditionals, then we should add a comment to the QAPI schema,
too.  Not a demand, since I'm not sure what to expect.