On 3/16/21 11:26 AM, Philippe Mathieu-Daudé wrote:
> On 3/16/21 10:02 AM, Philippe Mathieu-Daudé wrote:
>> On 3/16/21 7:51 AM, Markus Armbruster wrote:
>>> Eric Blake <ebl...@redhat.com> writes:
>>>
>>>> On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
>>> [...]
>>>>> diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
>>>>> new file mode 100644
>>>>> index 00000000000..f16e49b8956
>>>>> --- /dev/null
>>>>> +++ b/accel/accel-qmp.c
>>>>> @@ -0,0 +1,47 @@
>>>>> +/*
>>>>> + * QEMU accelerators, QMP commands
>>>>> + *
>>>>> + * Copyright (c) 2021 Red Hat Inc.
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>>> + */
>>>>> +
>>>>> +#include "qemu/osdep.h"
>>>>> +#include "qapi/qapi-commands-machine.h"
>>>>> +
>>>>> +static const Accelerator accel_list[] = {
>>>>> + ACCELERATOR_QTEST,
>>>>> +#ifdef CONFIG_TCG
>>>>> + ACCELERATOR_TCG,
>>>>> +#endif
>>>>> +#ifdef CONFIG_KVM
>>>>> + ACCELERATOR_KVM,
>>>>> +#endif
>>>>
>>>> ...would it be worth compiling the enum to only list enum values that
>>>> were actually compiled in? That would change it to:
>>>>
>>>> { 'enum': 'Accelerator',
>>>> 'data': [ 'qtest',
>>>> { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
>>>> ...
>>
>> These accelerator definitions are supposed to be poisoned in generic
>> code... But I like the simplicity of your suggestion, so I'll give it
>> a try and see what happens with removing the poisoned definitions.
>
> This is actually quite interesting :) Accelerator definitions are
> declared in config-target.h, but acceleration is host specific...
Thomas, I guess I hit Claudio's reported bug again...
1/ generic libqemuutil.a is built without any CONFIG_accel definition.
So this qapi-generated enum ... :
typedef enum Accelerator {
ACCELERATOR_QTEST,
#if defined(CONFIG_TCG)
ACCELERATOR_TCG,
#endif /* defined(CONFIG_TCG) */
#if defined(CONFIG_KVM)
ACCELERATOR_KVM,
#endif /* defined(CONFIG_KVM) */
#if defined(CONFIG_HAX)
ACCELERATOR_HAX,
#endif /* defined(CONFIG_HAX) */
#if defined(CONFIG_HVF)
ACCELERATOR_HVF,
#endif /* defined(CONFIG_HVF) */
#if defined(CONFIG_WHPX)
ACCELERATOR_WHPX,
#endif /* defined(CONFIG_WHPX) */
#if defined(CONFIG_XEN_BACKEND)
ACCELERATOR_XEN,
#endif /* defined(CONFIG_XEN_BACKEND) */
ACCELERATOR__MAX,
} Accelerator;
... is expanded to:
typedef enum Accelerator {
ACCELERATOR_QTEST,
ACCELERATOR__MAX,
} Accelerator;
2/ softmmu code and qtest do get the definition, the enum
is different:
typedef enum Accelerator {
ACCELERATOR_QTEST,
ACCELERATOR_KVM,
ACCELERATOR__MAX,
} Accelerator;
qmp_query_accels() fills AcceleratorInfoList with 2 entries
3/ trying to understand what's happening, query-qmp-schema
returns:
{
"name": "206",
"tag": "name",
"variants": [
{
"case": "qtest",
"type": "0"
},
{
"case": "kvm",
"type": "0"
}
],
"members": [
{
"name": "name",
"type": "403"
}
],
"meta-type": "object"
},
{
"name": "403",
"meta-type": "enum",
"values": [
"qtest",
"kvm"
]
},
So accelerators are listed, but with the same enum index?
4/ Running 'query-accels' aborts in qapi_enum_lookup():
The first entry 'qtest' is visited correctly.
When the second entry 'kvm' is visited, this assertion fires:
assert(val >= 0 && val < lookup->size);
because lookup->size = 1 (remember from 1/ Accelerator_lookup
has been compiled without definitions, in libqemuutil.a).
I'll keep the current patch, as it works, and address the rest
of this series comments.
Thanks,
Phil.
