[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command |
Date: |
Wed, 5 May 2021 17:28:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 5/5/21 5:01 PM, Markus Armbruster wrote:
> 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.
OK, will do in v8.
[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