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 <swethajoshi...@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~
>
