Philippe Mathieu-Daudé <phi...@linaro.org> writes:
> On 7/8/24 10:18, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <phi...@linaro.org> writes:
>>
>>> When configuring QEMU with --without-default-devices and
>>> not including machines using a GIC, the GIC model is not
>>> built in but the 'query-gic-capabilities' command still
>>> returns false hopes about GIC:
>>>
>>> {"execute": "query-gic-capabilities"}
>>> {"return": [{"emulated": true, "version": 3, "kernel": false},
>>> {"emulated": true, "version": 2, "kernel": false}]}
>>>
>>> Restrict the command to when the GIC is available. If it
>>> isn't we'll get:
>>>
>>> { "execute": "query-gic-capabilities" }
>>> {"error": {"class": "CommandNotFound", "desc": "The command
>>> query-gic-capabilities has not been found"}}
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2484
>>> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>>> ---
>>> qapi/misc-target.json | 4 ++--
>>> hw/intc/arm_gic_qmp.c | 2 ++
>>> hw/intc/meson.build | 2 +-
>>> 3 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>>> index 8d70bd24d8..b857e44c2e 100644
>>> --- a/qapi/misc-target.json
>>> +++ b/qapi/misc-target.json
>>> @@ -316,7 +316,7 @@
>>> 'data': { 'version': 'int',
>>> 'emulated': 'bool',
>>> 'kernel': 'bool' },
>>> - 'if': 'TARGET_ARM' }
>>> + 'if': 'CONFIG_ARM_GIC' }
>>>
>>> ##
>>> # @query-gic-capabilities:
>>> @@ -335,7 +335,7 @@
>>> # { "version": 3, "emulated": false, "kernel": true }
>>> ] }
>>> ##
>>> { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
>>> - 'if': 'TARGET_ARM' }
>>> + 'if': 'CONFIG_ARM_GIC' }
>>>
>>> ##
>>> # @SGXEPCSection:
>>> diff --git a/hw/intc/arm_gic_qmp.c b/hw/intc/arm_gic_qmp.c
>>> index 71056a0c10..1fc79c775b 100644
>>> --- a/hw/intc/arm_gic_qmp.c
>>> +++ b/hw/intc/arm_gic_qmp.c
>>> @@ -6,6 +6,8 @@
>>>
>>> #include "qemu/osdep.h"
>>> #include "qapi/util.h"
>>> +
>>> +#include CONFIG_DEVICES
>>
>> Uh, why do we need this now?
>
> Now qapi-commands-misc-target.h is generated guarded with
> '#ifdef CONFIG_ARM_GIC', and CONFIG_ARM_GIC is defined per
> target in CONFIG_DEVICES.
>
> I'll update the patch description, but does this makes
> sense to you? QAPI headers don't include headers defining
> guards, we have to include them manually where we use QAPI
> headers.
Hmm. Then the generated headers aren't self-contained anymore.
Having to manually include a configuration header like CONFIG_DEVICES
wherever you use configuration symbols strikes me as unadvisable when
uses include checking for definedness, such as #ifdef: silent miscompile
when you forget to include.
This is why Autoconf wants you to include config.h first in any .c: it
makes #ifdef & friends safe.
qemu/osdep.h does include some configuration headers:
#include "config-host.h"
#ifdef COMPILING_PER_TARGET
#include CONFIG_TARGET
#else
#include "exec/poison.h"
#endif
Why not CONFIG_DEVICES?
[...]
