Re: [RFC PATCH-for-9.1? 2/2] hw/intc/arm_gic: Only provide query-gic-capabilities when GIC built-in

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 7/8/24 10:18, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@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é <philmd@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.

> 
>>   #include "qapi/qapi-commands-misc-target.h"
>>   #include "kvm_arm.h"
>>   
>> diff --git a/hw/intc/meson.build b/hw/intc/meson.build
>> index 45d3503d49..b9550967e2 100644
>> --- a/hw/intc/meson.build
>> +++ b/hw/intc/meson.build
>> @@ -39,7 +39,7 @@ if config_all_devices.has_key('CONFIG_APIC') or \
>>   endif
>>   
>>   specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c'))
>> -specific_ss.add(when: 'CONFIG_ARM', if_true: files('arm_gic_qmp.c'))
>> +specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gic_qmp.c'))
>>   specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif_common.c'))
>>   specific_ss.add(when: 'CONFIG_ARM_GICV3_TCG', if_true: files('arm_gicv3_cpuif.c'))
>>   specific_ss.add(when: 'CONFIG_ARM_GIC_KVM', if_true: files('arm_gic_kvm.c'))
>