On 3/16/21 1:48 PM, Thomas Huth wrote:
> On 16/03/2021 13.41, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <phi...@redhat.com> writes:
>>
>>> 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;
>>
>> CONFIG_KVM, CONFIG_TCG, ... are defined in ${target}-config-target.h,
>> and may only be used in target-specific code.
>>
>> If the enum ends up in libqemuutil.a, there are uses outside
>> target-specific code.
>>
>> exec/poison.h lacks CONFIG_KVM, CONFIG_TCG, ... Should they be added?
>
> CONFIG_KVM is already in poison.h, and CONFIG_TCG cannot be added there
> since it is also defined in config-host.h. But the other accelerator
> switches should be marked as poisoned, too.
Maybe we hit "exec/poison.h" limit. Maybe it is too wide / generic,
and need split, or multiple ones.
Should we redefine on which code area we want these definitions
poisoned?
AFAIK accel/ is not target specific but host specific.
My list of area / useful to poison:
- user-mode
. non-tcg accel
. hardware emulation
- generic hardware emulation
. target specific
. accel specific
For this one it is already too late:
- target acceleration
. hardware emulation
Thoughts?
