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)
+{
+ GArray *addresses = g_array_new(false, false, sizeof(paddr));
+ 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");
+ abort();
+}
+
void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
{
ram_addr_t ram_addr;
hwaddr paddr;
AcpiGhesState *ags;
- GArray *addresses;
assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
@@ -2443,7 +2465,6 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
ram_addr = qemu_ram_addr_from_host(addr);
if (ram_addr != RAM_ADDR_INVALID &&
kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
- addresses = g_array_new(false, false, sizeof(paddr));
kvm_hwpoison_page_add(ram_addr);
/*
* If this is a BUS_MCEERR_AR, we know we have been called
@@ -2456,19 +2477,10 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
* later from the main thread, so doing the injection of
* the error would be more complicated.
*/
- g_array_append_vals(addresses, &paddr, 1);
if (code == BUS_MCEERR_AR) {
- kvm_cpu_synchronize_state(c);
- if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
- addresses)) {
- kvm_inject_arm_sea(c);
- } else {
- error_report("failed to record the error");
- abort();
- }
+ push_ghes_memory_errors(c, ags, paddr);
}
- g_array_free(addresses, true);
return;
}
if (code == BUS_MCEERR_AO) {
--
2.51.0On 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)
> +{
> + GArray *addresses = g_array_new(false, false, sizeof(paddr));
> + 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;
perhaps use &error_abort and use it right here instead of goto
or even more cleaner: pass &error_abort as arg to acpi_ghes_memory_errors()
and drop not needed 'if's & return value
> + }
> +
> + kvm_inject_arm_sea(c);
> +
> + g_array_free(addresses, true);
> +
> + return;
> +error:
> + error_report("failed to record the error");
> + abort();
> +}
> +
> void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> {
> ram_addr_t ram_addr;
> hwaddr paddr;
> AcpiGhesState *ags;
> - GArray *addresses;
>
> assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
>
> @@ -2443,7 +2465,6 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> ram_addr = qemu_ram_addr_from_host(addr);
> if (ram_addr != RAM_ADDR_INVALID &&
> kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
> - addresses = g_array_new(false, false, sizeof(paddr));
> kvm_hwpoison_page_add(ram_addr);
> /*
> * If this is a BUS_MCEERR_AR, we know we have been called
> @@ -2456,19 +2477,10 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> * later from the main thread, so doing the injection of
> * the error would be more complicated.
> */
> - g_array_append_vals(addresses, &paddr, 1);
> if (code == BUS_MCEERR_AR) {
> - kvm_cpu_synchronize_state(c);
> - if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
> - addresses)) {
> - kvm_inject_arm_sea(c);
> - } else {
> - error_report("failed to record the error");
> - abort();
> - }
> + push_ghes_memory_errors(c, ags, paddr);
> }
>
> - g_array_free(addresses, true);
> return;
> }
> if (code == BUS_MCEERR_AO) {
On 10/31/25 11:25 PM, Igor Mammedov 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)
>> +{
>> + GArray *addresses = g_array_new(false, false, sizeof(paddr));
>> + 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;
>
> perhaps use &error_abort and use it right here instead of goto
> or even more cleaner: pass &error_abort as arg to acpi_ghes_memory_errors()
> and drop not needed 'if's & return value
>
Right, the second way looks cleaner with the assumption that abort is expected
on any errors in this call path. I will clean it up in next respin.
Note the error handling in acpi_ghes_memory_errors() seems already problematic.
acpi_ghes_memory_errors // Declare errp
ghes_record_cper_errors
ghes_record_cper_errors // error is initialized by error_setg
error_setg // !read_ack_register
>> + }
>> +
>> + kvm_inject_arm_sea(c);
>> +
>> + g_array_free(addresses, true);
>> +
>> + return;
>> +error:
>> + error_report("failed to record the error");
>> + abort();
>> +}
>> +
>> void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
>> {
>> ram_addr_t ram_addr;
>> hwaddr paddr;
>> AcpiGhesState *ags;
>> - GArray *addresses;
>>
>> assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
>>
>> @@ -2443,7 +2465,6 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
>> ram_addr = qemu_ram_addr_from_host(addr);
>> if (ram_addr != RAM_ADDR_INVALID &&
>> kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
>> - addresses = g_array_new(false, false, sizeof(paddr));
>> kvm_hwpoison_page_add(ram_addr);
>> /*
>> * If this is a BUS_MCEERR_AR, we know we have been called
>> @@ -2456,19 +2477,10 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
>> * later from the main thread, so doing the injection of
>> * the error would be more complicated.
>> */
>> - g_array_append_vals(addresses, &paddr, 1);
>> if (code == BUS_MCEERR_AR) {
>> - kvm_cpu_synchronize_state(c);
>> - if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
>> - addresses)) {
>> - kvm_inject_arm_sea(c);
>> - } else {
>> - error_report("failed to record the error");
>> - abort();
>> - }
>> + push_ghes_memory_errors(c, ags, paddr);
>> }
>>
>> - g_array_free(addresses, true);
>> return;
>> }
>> if (code == BUS_MCEERR_AO) {
Thanks,
Gavin
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?
> +{
> + 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.
> + 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.
> + abort();
If you do the above with the message, just duplicate this in the
two error paths (by end of next patch).
> +}
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
On Mon, 3 Nov 2025 09:39:50 +1000
Gavin Shan <gshan@redhat.com> wrote:
> 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.
ack to that, it's uint64_t in spec so I'd stick to that.
> >> +{
> >> + 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
>
© 2016 - 2025 Red Hat, Inc.