* [PATCH v6 04/19] acpi/ghes: Cleanup the code which gets ghes ged state
2025-02-27 16:00 [PATCH v6 00/19] Change ghes to use HEST-based offsets and add support for error inject Mauro Carvalho Chehab
@ 2025-02-27 16:00 ` Mauro Carvalho Chehab
2025-02-28 9:19 ` Jonathan Cameron
2025-02-27 16:43 ` [PATCH v6 00/19] Change ghes to use HEST-based offsets and add support for error inject Igor Mammedov
1 sibling, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2025-02-27 16:00 UTC (permalink / raw)
To: Igor Mammedov, Michael S . Tsirkin
Cc: Jonathan Cameron, Shiju Jose, qemu-arm, qemu-devel,
Mauro Carvalho Chehab, Ani Sinha, Dongjiu Geng, Paolo Bonzini,
Peter Maydell, kvm, linux-kernel
Move the check logic into a common function and simplify the
code which checks if GHES is enabled and was properly setup.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/ghes-stub.c | 7 ++++---
hw/acpi/ghes.c | 38 +++++++++++---------------------------
include/hw/acpi/ghes.h | 14 +++++++-------
target/arm/kvm.c | 7 +++++--
4 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/hw/acpi/ghes-stub.c b/hw/acpi/ghes-stub.c
index 7cec1812dad9..40f660c246fe 100644
--- a/hw/acpi/ghes-stub.c
+++ b/hw/acpi/ghes-stub.c
@@ -11,12 +11,13 @@
#include "qemu/osdep.h"
#include "hw/acpi/ghes.h"
-int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)
+int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
+ uint64_t physical_address)
{
return -1;
}
-bool acpi_ghes_present(void)
+AcpiGhesState *acpi_ghes_get_state(void)
{
- return false;
+ return NULL;
}
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index b709c177cdea..84b891fd3dcf 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -360,18 +360,12 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
/* Create a read-write fw_cfg file for Address */
fw_cfg_add_file_callback(s, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, NULL, NULL,
NULL, &(ags->hw_error_le), sizeof(ags->hw_error_le), false);
-
- ags->present = true;
}
static void get_hw_error_offsets(uint64_t ghes_addr,
uint64_t *cper_addr,
uint64_t *read_ack_register_addr)
{
- if (!ghes_addr) {
- return;
- }
-
/*
* non-HEST version supports only one source, so no need to change
* the start offset based on the source ID. Also, we can't validate
@@ -390,35 +384,20 @@ static void get_hw_error_offsets(uint64_t ghes_addr,
*read_ack_register_addr = ghes_addr + sizeof(uint64_t);
}
-void ghes_record_cper_errors(const void *cper, size_t len,
+void ghes_record_cper_errors(AcpiGhesState *ags, const void *cper, size_t len,
uint16_t source_id, Error **errp)
{
uint64_t cper_addr = 0, read_ack_register_addr = 0, read_ack_register;
- AcpiGedState *acpi_ged_state;
- AcpiGhesState *ags;
if (len > ACPI_GHES_MAX_RAW_DATA_LENGTH) {
error_setg(errp, "GHES CPER record is too big: %zd", len);
return;
}
- acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
- NULL));
- if (!acpi_ged_state) {
- error_setg(errp, "Can't find ACPI_GED object");
- return;
- }
- ags = &acpi_ged_state->ghes_state;
-
assert(ACPI_GHES_ERROR_SOURCE_COUNT == 1);
get_hw_error_offsets(le64_to_cpu(ags->hw_error_le),
&cper_addr, &read_ack_register_addr);
- if (!cper_addr) {
- error_setg(errp, "can not find Generic Error Status Block");
- return;
- }
-
cpu_physical_memory_read(read_ack_register_addr,
&read_ack_register, sizeof(read_ack_register));
@@ -444,7 +423,8 @@ void ghes_record_cper_errors(const void *cper, size_t len,
return;
}
-int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)
+int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
+ uint64_t physical_address)
{
/* Memory Error Section Type */
const uint8_t guid[] =
@@ -470,7 +450,7 @@ int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)
acpi_ghes_build_append_mem_cper(block, physical_address);
/* Report the error */
- ghes_record_cper_errors(block->data, block->len, source_id, &errp);
+ ghes_record_cper_errors(ags, block->data, block->len, source_id, &errp);
g_array_free(block, true);
@@ -482,7 +462,7 @@ int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)
return 0;
}
-bool acpi_ghes_present(void)
+AcpiGhesState *acpi_ghes_get_state(void)
{
AcpiGedState *acpi_ged_state;
AcpiGhesState *ags;
@@ -491,8 +471,12 @@ bool acpi_ghes_present(void)
NULL));
if (!acpi_ged_state) {
- return false;
+ return NULL;
}
ags = &acpi_ged_state->ghes_state;
- return ags->present;
+
+ if (!ags->hw_error_le) {
+ return NULL;
+ }
+ return ags;
}
diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index 39619a2457cb..f96ac3e85ca2 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -66,7 +66,6 @@ enum {
typedef struct AcpiGhesState {
uint64_t hw_error_le;
- bool present; /* True if GHES is present at all on this board */
} AcpiGhesState;
void acpi_build_hest(GArray *table_data, GArray *hardware_errors,
@@ -74,15 +73,16 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors,
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(uint16_t source_id, uint64_t error_physical_addr);
-void ghes_record_cper_errors(const void *cper, size_t len,
+int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
+ uint64_t error_physical_addr);
+void ghes_record_cper_errors(AcpiGhesState *ags, const void *cper, size_t len,
uint16_t source_id, Error **errp);
/**
- * acpi_ghes_present: Report whether ACPI GHES table is present
+ * acpi_ghes_get_state: Get a pointer for ACPI ghes state
*
- * Returns: true if the system has an ACPI GHES table and it is
- * safe to call acpi_ghes_memory_errors() to record a memory error.
+ * Returns: a pointer to ghes state if the system has an ACPI GHES table,
+ * NULL, otherwise.
*/
-bool acpi_ghes_present(void);
+AcpiGhesState *acpi_ghes_get_state(void);
#endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index da30bdbb2349..80ca7779797b 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -2366,10 +2366,12 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
{
ram_addr_t ram_addr;
hwaddr paddr;
+ AcpiGhesState *ags;
assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
- if (acpi_ghes_present() && addr) {
+ ags = acpi_ghes_get_state();
+ if (ags && 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)) {
@@ -2387,7 +2389,8 @@ 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_memory_errors(ACPI_HEST_SRC_ID_SEA, paddr)) {
+ if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SEA,
+ paddr)) {
kvm_inject_arm_sea(c);
} else {
error_report("failed to record the error");
--
2.48.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v6 00/19] Change ghes to use HEST-based offsets and add support for error inject
2025-02-27 16:00 [PATCH v6 00/19] Change ghes to use HEST-based offsets and add support for error inject Mauro Carvalho Chehab
2025-02-27 16:00 ` [PATCH v6 04/19] acpi/ghes: Cleanup the code which gets ghes ged state Mauro Carvalho Chehab
@ 2025-02-27 16:43 ` Igor Mammedov
1 sibling, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2025-02-27 16:43 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Michael S . Tsirkin, Jonathan Cameron, Shiju Jose, qemu-arm,
qemu-devel, Philippe Mathieu-Daudé, Ani Sinha, Cleber Rosa,
Dongjiu Geng, Eduardo Habkost, Eric Blake, John Snow,
Marcel Apfelbaum, Markus Armbruster, Michael Roth, Paolo Bonzini,
Peter Maydell, Shannon Zhao, Yanan Wang, Zhao Liu, kvm,
linux-kernel
On Thu, 27 Feb 2025 17:00:38 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> Now that the ghes preparation patches were merged, let's add support
> for error injection.
>
> On this version, HEST table got added to ACPI tables testing for aarch64 virt.
>
> There are also some patch reorder to help reviewers to check the changes.
>
> The code itself is almost identical to v4, with just a few minor nits addressed.
checkpatch on my machine still complains
0007-acpi-ghes-Use-HEST-table-offsets-when-preparing-GHES.patch has no obvious style problems and is ready for submission.
Checking 0008-acpi-ghes-don-t-hard-code-the-number-of-sources-for-.patch...
WARNING: line over 80 characters
#170: FILE: hw/acpi/ghes.c:390:
+ build_ghes_v2_entry(table_data, linker, ¬if_source[i], i, num_sources);
total: 0 errors, 1 warnings, 159 lines checked
0008-acpi-ghes-don-t-hard-code-the-number-of-sources-for-.patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking 0009-acpi-ghes-add-a-notifier-to-notify-when-error-data-i.patch...
total: 0 errors, 0 warnings, 26 lines checked
0009-acpi-ghes-add-a-notifier-to-notify-when-error-data-i.patch has no obvious style problems and is ready for submission.
Checking 0010-acpi-generic_event_device-Update-GHES-migration-to-c.patch...
total: 0 errors, 0 warnings, 41 lines checked
0010-acpi-generic_event_device-Update-GHES-migration-to-c.patch has no obvious style problems and is ready for submission.
Checking 0011-acpi-generic_event_device-add-logic-to-detect-if-HES.patch...
total: 0 errors, 0 warnings, 59 lines checked
0011-acpi-generic_event_device-add-logic-to-detect-if-HES.patch has no obvious style problems and is ready for submission.
Checking 0012-acpi-generic_event_device-add-an-APEI-error-device.patch...
total: 0 errors, 0 warnings, 72 lines checked
0012-acpi-generic_event_device-add-an-APEI-error-device.patch has no obvious style problems and is ready for submission.
Checking 0013-tests-acpi-virt-allow-acpi-table-changes-at-DSDT-and.patch...
total: 0 errors, 0 warnings, 7 lines checked
0013-tests-acpi-virt-allow-acpi-table-changes-at-DSDT-and.patch has no obvious style problems and is ready for submission.
Checking 0014-arm-virt-Wire-up-a-GED-error-device-for-ACPI-GHES.patch...
WARNING: line over 80 characters
#68: FILE: hw/arm/virt.c:1015:
+ VirtMachineState *s = container_of(n, VirtMachineState, generic_error_notifier);
total: 0 errors, 1 warnings, 44 lines checked
0014-arm-virt-Wire-up-a-GED-error-device-for-ACPI-GHES.patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking 0015-qapi-acpi-hest-add-an-interface-to-do-generic-CPER-e.patch...
total: 0 errors, 0 warnings, 178 lines checked
> ---
> v6:
> - some minor nits addressed:
> - use GPA instead of offset;
> - merged two patches;
> - fixed a couple of long line coding style issues;
> - the HEST/DSDT diff inside a patch was changed to avoid troubles
> applying it.
>
> v5:
> - make checkpatch happier;
> - HEST table is now tested;
> - some changes at HEST spec documentation to align with code changes;
> - extra care was taken with regards to git bisectability.
>
> v4:
> - added an extra comment for AcpiGhesState structure;
> - patches reordered;
> - no functional changes, just code shift between the patches in this series.
>
> v3:
> - addressed more nits;
> - hest_add_le now points to the beginning of HEST table;
> - removed HEST from tests/data/acpi;
> - added an extra patch to not use fw_cfg with virt-10.0 for hw_error_le
>
> v2:
> - address some nits;
> - improved ags cleanup patch and removed ags.present field;
> - added some missing le*_to_cpu() calls;
> - update date at copyright for new files to 2024-2025;
> - qmp command changed to: inject-ghes-v2-error ans since updated to 10.0;
> - added HEST and DSDT tables after the changes to make check target happy.
> (two patches: first one whitelisting such tables; second one removing from
> whitelist and updating/adding such tables to tests/data/acpi)
>
>
>
> Mauro Carvalho Chehab (19):
> tests/acpi: virt: add an empty HEST file
> tests/qtest/bios-tables-test: extend to also check HEST table
> tests/acpi: virt: update HEST file with its current data
> acpi/ghes: Cleanup the code which gets ghes ged state
> acpi/ghes: prepare to change the way HEST offsets are calculated
> acpi/ghes: add a firmware file with HEST address
> acpi/ghes: Use HEST table offsets when preparing GHES records
> acpi/ghes: don't hard-code the number of sources for HEST table
> acpi/ghes: add a notifier to notify when error data is ready
> acpi/generic_event_device: Update GHES migration to cover hest addr
> acpi/generic_event_device: add logic to detect if HEST addr is
> available
> acpi/generic_event_device: add an APEI error device
> tests/acpi: virt: allow acpi table changes at DSDT and HEST tables
> arm/virt: Wire up a GED error device for ACPI / GHES
> qapi/acpi-hest: add an interface to do generic CPER error injection
> acpi/generic_event_device.c: enable use_hest_addr for QEMU 10.x
> tests/acpi: virt: update HEST and DSDT tables
> docs: hest: add new "etc/acpi_table_hest_addr" and update workflow
> scripts/ghes_inject: add a script to generate GHES error inject
>
> MAINTAINERS | 10 +
> docs/specs/acpi_hest_ghes.rst | 28 +-
> hw/acpi/Kconfig | 5 +
> hw/acpi/aml-build.c | 10 +
> hw/acpi/generic_event_device.c | 44 ++
> hw/acpi/ghes-stub.c | 7 +-
> hw/acpi/ghes.c | 231 ++++--
> hw/acpi/ghes_cper.c | 38 +
> hw/acpi/ghes_cper_stub.c | 19 +
> hw/acpi/meson.build | 2 +
> hw/arm/virt-acpi-build.c | 35 +-
> hw/arm/virt.c | 19 +-
> hw/core/machine.c | 2 +
> include/hw/acpi/acpi_dev_interface.h | 1 +
> include/hw/acpi/aml-build.h | 2 +
> include/hw/acpi/generic_event_device.h | 1 +
> include/hw/acpi/ghes.h | 51 +-
> include/hw/arm/virt.h | 2 +
> qapi/acpi-hest.json | 35 +
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> scripts/arm_processor_error.py | 476 ++++++++++++
> scripts/ghes_inject.py | 51 ++
> scripts/qmp_helper.py | 703 ++++++++++++++++++
> target/arm/kvm.c | 7 +-
> tests/data/acpi/aarch64/virt/DSDT | Bin 5196 -> 5240 bytes
> .../data/acpi/aarch64/virt/DSDT.acpihmatvirt | Bin 5282 -> 5326 bytes
> tests/data/acpi/aarch64/virt/DSDT.memhp | Bin 6557 -> 6601 bytes
> tests/data/acpi/aarch64/virt/DSDT.pxb | Bin 7679 -> 7723 bytes
> tests/data/acpi/aarch64/virt/DSDT.topology | Bin 5398 -> 5442 bytes
> tests/data/acpi/aarch64/virt/HEST | Bin 0 -> 224 bytes
> tests/qtest/bios-tables-test.c | 2 +-
> 32 files changed, 1692 insertions(+), 91 deletions(-)
> create mode 100644 hw/acpi/ghes_cper.c
> create mode 100644 hw/acpi/ghes_cper_stub.c
> create mode 100644 qapi/acpi-hest.json
> create mode 100644 scripts/arm_processor_error.py
> create mode 100755 scripts/ghes_inject.py
> create mode 100755 scripts/qmp_helper.py
> create mode 100644 tests/data/acpi/aarch64/virt/HEST
>
^ permalink raw reply [flat|nested] 4+ messages in thread