Hi Igor,
On 11/12/25 10:49 PM, Igor Mammedov wrote:
On Tue, 11 Nov 2025 13:58:46 +1000
Gavin Shan <[email protected]> wrote:
On 11/11/25 12:54 AM, Philippe Mathieu-Daudé wrote:
On 5/11/25 12:44, Gavin Shan wrote:
Use error_abort in acpi_ghes_memory_errors() so that the caller needn't
explicitly call abort() on errors. With this change, its return value
isn't needed any more.
Suggested-by: Igor Mammedov <[email protected]>
Signed-off-by: Gavin Shan <[email protected]>
---
hw/acpi/ghes-stub.c | 6 +++---
hw/acpi/ghes.c | 15 ++++-----------
include/hw/acpi/ghes.h | 5 +++--
target/arm/kvm.c | 10 +++-------
4 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index f73908985d..35c7bbbb01 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -98,8 +98,9 @@ void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
const char *oem_id, const char *oem_table_id);
void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
GArray *hardware_errors);
-int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
- uint64_t *addresses, uint32_t num_of_addresses);
+void acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
+ uint64_t *addresses, uint32_t num_of_addresses,
+ Error **errp);
This is an anti-pattern w.r.t. commit e3fe3988d78 ("error: Document
Error API usage rules"), because it can be called with an errp distinct
of &error_abort.
If you really want to abort(), remove the errp argument, directly call
abort() and rename as acpi_ghes_memory_abort_on_errors().
Thanks for pointing it out. I will improve this like below in next revision.
- Drop 'errp' argument from acpi_ghes_memory_errors(), but I prefer to keep
the function name.
- In acpi_ghes_memory_errors(), a local variable 'Error *err' is added and
pass it to ghes_record_cper_errors(), which is also called by QMP handler
qmp_inject_ghes_v2_error().
Please let me know if there are any more improvements needed.
void ghes_record_cper_errors(AcpiGhesState *ags, const void *cper, size_t
len,
uint16_t source_id, Error **errp);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 459ca4a9b0..a889315606 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -2458,13 +2458,9 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void
*addr)
addresses[0] = paddr;
if (code == BUS_MCEERR_AR) {
kvm_cpu_synchronize_state(c);
- if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
- addresses, 1)) {
- kvm_inject_arm_sea(c);
- } else {
- error_report("failed to record the error");
- abort();
- }
+ acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
+ addresses, 1, &error_abort);
I guess that was my request,
with calling abort() directly you have to print error separately,
while using 'error_abort' is much more cleaner (as this hunk demonstrates)
error_abort has been replaced by error_fatal as Markus suggested in another
reply.
+ kvm_inject_arm_sea(c);
}
return;
}
Thanks,
Gavin
