Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Philippe Mathieu-Daudé]

On 5/25/21 5:20 AM, Richard Henderson wrote:
> On 5/24/21 7:58 PM, Swetha Joshi wrote:
>> Signed-off-by: Swetha Joshi <swethajoshi139@gmail.com>
>> ---
>>   target/arm/kvm64.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> You're still missing the commit message.
> 
>>
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index dff85f6db9..47a4d9d831 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -1403,7 +1403,10 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int
>> code, void *addr)
>>       hwaddr paddr;
>>       Object *obj = qdev_get_machine();
>>       VirtMachineState *vms = VIRT_MACHINE(obj);
>> -    bool acpi_enabled = virt_is_acpi_enabled(vms);
>> +    bool acpi_enabled = false;

Oh, IIUC this is an user request to build QEMU *with* KVM but
*without* the Virt machine? Presumably sbsa-ref or Xilinx
Versal/ZynqMP?

Interesting, I never tested that config. Swetha, do you mind
providing more information on the use case?

Thanks,

Phil.

>> +#ifdef CONFIG_ARM_VIRT
>> +    acpi_enabled = virt_is_acpi_enabled(vms);
>> +#endif /* CONFIG_ARM_VIRT */
>>         assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
>>   @@ -1426,12 +1429,13 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c,
>> int code, void *addr)
>>                */
>>               if (code == BUS_MCEERR_AR) {
>>                   kvm_cpu_synchronize_state(c);
>> -                if (!acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA,
>> paddr)) {
>> -                    kvm_inject_arm_sea(c);
>> -                } else {
>> +#ifdef CONFIG_ACPI_APEI
>> +                if (acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA,
>> paddr)) {
>>                       error_report("failed to record the error");
>>                       abort();
>>                   }
>> +#endif /* CONFIG_ACPI_APEI */
>> +                kvm_inject_arm_sea(c);
>>               }
> 
> Otherwise the actual patch looks ok.
> 
> I'm a bit surprised that you need the second hunk.  I would have
> expected the entire block to be optimized away with the 'acpi_enabled =
> false' being propagated into the outermost IF.
> 
> 
> r~
>