[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command |
Date: |
Wed, 05 May 2021 17:01:41 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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.
[PATCH v5 03/10] tests/qtest: Add qtest_has_accel() method, Philippe Mathieu-Daudé, 2021/05/01
[PATCH v5 04/10] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for KVM, Philippe Mathieu-Daudé, 2021/05/01
[PATCH v5 05/10] qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM, Philippe Mathieu-Daudé, 2021/05/01
[PATCH v5 06/10] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests, Philippe Mathieu-Daudé, 2021/05/01
[PATCH v5 07/10] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for TCG, Philippe Mathieu-Daudé, 2021/05/01
[PATCH v5 08/10] qtest/bios-tables-test: Make test build-independent from accelerator, Philippe Mathieu-Daudé, 2021/05/01