Re: [PATCH RESEND v2 2/3] kvm/arm/kvm: Introduce helper push_ghes_memory_errors()

[Gavin Shan <gshan@redhat.com>]

On 10/31/25 8:09 PM, Jonathan Cameron wrote:
> On Tue,  7 Oct 2025 16:08:09 +1000
> Gavin Shan <gshan@redhat.com> wrote:
> 
>> Introduce helper push_ghes_memory_errors(), which sends ACPI GHES memory
>> errors, injects SEA exception or aborts on errors. This function will
>> be extended to support multiple ACPI GHES memory errors in the next
>> path.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   target/arm/kvm.c | 36 ++++++++++++++++++++++++------------
>>   1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index 9a47ac9e3a..c5d5b3b16e 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -2429,12 +2429,34 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
>>       return ret;
>>   }
>>   
>> +static void push_ghes_memory_errors(CPUState *c, AcpiGhesState *ags,
>> +                                    uint64_t paddr)
> 
> Why not hwaddr paddr?
> 

Because acpi_ghes_memory_errors() accepts it as uint64_t.

>> +{
>> +    GArray *addresses = g_array_new(false, false, sizeof(paddr));
> 
> As in previous I'd just have
> 	hwaddr paddrs[16];
> 
> rather than bothering with a g_array.
> 

Ok.

>> +    int ret;
>> +
>> +    kvm_cpu_synchronize_state(c);
>> +    g_array_append_vals(addresses, &paddr, 1);
>> +    ret = acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC, addresses);
>> +    if (ret) {
>> +        goto error;
>> +    }
>> +
>> +    kvm_inject_arm_sea(c);
>> +
>> +    g_array_free(addresses, true);
>> +
>> +    return;
>> +error:
>> +    error_report("failed to record the error");
> 
> I'd just do this inline at the error case. In the next
> patch you add a more specific report of why to another path
> that would then be followed by this.
> 

Ok.

>> +    abort();
> 
> If you do the above with the message, just duplicate this in the
> two error paths (by end of next patch).
> 

Will correct this in next revision if we still take current design. Igor suggested
to have individual error source per vCPU in another thread.

>> +}
> 

Thanks,
Gavin