On Thu, 27 Feb 2025 17:00:43 +0100
Mauro Carvalho Chehab <mchehab+hua...@kernel.org> wrote:
> Add a new ags flag to change the way HEST offsets are calculated.
> Currently, offsets needed to store ACPI HEST offsets and read ack
> are calculated based on a previous knowledge from the logic
> which creates the HEST table.
>
> Such logic is not generic, not allowing to easily add more HEST
> entries nor replicates what OSPM does.
>
> As the next patches will be adding a more generic logic, add a
> new use_hest_addr, set to false, in preparation for such changes.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/acpi/ghes.c | 39 ++++++++++++++++++++++++---------------
> hw/arm/virt-acpi-build.c | 13 ++++++++++---
> include/hw/acpi/ghes.h | 12 +++++++++++-
> 3 files changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index 84b891fd3dcf..9243b5ad4acb 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -206,7 +206,8 @@ ghes_gen_err_data_uncorrectable_recoverable(GArray *block,
> * Initialize "etc/hardware_errors" and "etc/hardware_errors_addr" fw_cfg
> blobs.
> * See docs/specs/acpi_hest_ghes.rst for blobs format.
> */
> -static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker
> *linker)
> +static void build_ghes_error_table(AcpiGhesState *ags, GArray
> *hardware_errors,
> + BIOSLinker *linker)
> {
> int i, error_status_block_offset;
>
> @@ -251,13 +252,15 @@ static void build_ghes_error_table(GArray
> *hardware_errors, BIOSLinker *linker)
> i * ACPI_GHES_MAX_RAW_DATA_LENGTH);
> }
>
> - /*
> - * tell firmware to write hardware_errors GPA into
> - * hardware_errors_addr fw_cfg, once the former has been initialized.
> - */
> - bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE,
> 0,
> - sizeof(uint64_t),
> - ACPI_HW_ERROR_FW_CFG_FILE, 0);
> + if (!ags->use_hest_addr) {
> + /*
> + * Tell firmware to write hardware_errors GPA into
> + * hardware_errors_addr fw_cfg, once the former has been initialized.
> + */
> + bios_linker_loader_write_pointer(linker,
> ACPI_HW_ERROR_ADDR_FW_CFG_FILE,
> + 0, sizeof(uint64_t),
> + ACPI_HW_ERROR_FW_CFG_FILE, 0);
> + }
> }
>
> /* Build Generic Hardware Error Source version 2 (GHESv2) */
> @@ -331,14 +334,15 @@ static void build_ghes_v2(GArray *table_data,
> }
>
> /* Build Hardware Error Source Table */
> -void acpi_build_hest(GArray *table_data, GArray *hardware_errors,
> +void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
> + GArray *hardware_errors,
> BIOSLinker *linker,
> const char *oem_id, const char *oem_table_id)
> {
> AcpiTable table = { .sig = "HEST", .rev = 1,
> .oem_id = oem_id, .oem_table_id = oem_table_id };
>
> - build_ghes_error_table(hardware_errors, linker);
> + build_ghes_error_table(ags, hardware_errors, linker);
>
> acpi_table_begin(&table, table_data);
>
> @@ -357,9 +361,11 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState
> *s,
> fw_cfg_add_file(s, ACPI_HW_ERROR_FW_CFG_FILE, hardware_error->data,
> hardware_error->len);
>
> - /* 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);
> + if (!ags->use_hest_addr) {
> + /* 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);
> + }
> }
>
> static void get_hw_error_offsets(uint64_t ghes_addr,
> @@ -395,8 +401,11 @@ void ghes_record_cper_errors(AcpiGhesState *ags, const
> void *cper, size_t len,
> }
>
> 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 (!ags->use_hest_addr) {
> + get_hw_error_offsets(le64_to_cpu(ags->hw_error_le),
> + &cper_addr, &read_ack_register_addr);
> + }
>
> cpu_physical_memory_read(read_ack_register_addr,
> &read_ack_register, sizeof(read_ack_register));
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 3ac8f8e17861..040d875d4e83 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -946,9 +946,16 @@ void virt_acpi_build(VirtMachineState *vms,
> AcpiBuildTables *tables)
> build_dbg2(tables_blob, tables->linker, vms);
>
> if (vms->ras) {
> - acpi_add_table(table_offsets, tables_blob);
> - acpi_build_hest(tables_blob, tables->hardware_errors, tables->linker,
> - vms->oem_id, vms->oem_table_id);
> + AcpiGedState *acpi_ged_state;
> + AcpiGhesState *ags;
> +
> + acpi_ged_state = ACPI_GED(vms->acpi_dev);
> + ags = &acpi_ged_state->ghes_state;
> + if (ags) {
> + acpi_add_table(table_offsets, tables_blob);
> + acpi_build_hest(ags, tables_blob, tables->hardware_errors,
> + tables->linker, vms->oem_id, vms->oem_table_id);
> + }
> }
>
> if (ms->numa_state->num_nodes > 0) {
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index f96ac3e85ca2..411f592662af 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -64,11 +64,21 @@ enum {
> ACPI_GHES_ERROR_SOURCE_COUNT
> };
>
> +/*
> + * AcpiGhesState stores GPA values that will be used to fill HEST entries.
> + *
> + * When use_hest_addr is false, the GPA of the etc/hardware_errors firmware
> + * is stored at hw_error_le. This is the default on QEMU 9.x.
> + *
> + * An GPA value equal to zero means that GHES is not present.
> + */
> typedef struct AcpiGhesState {
> uint64_t hw_error_le;
> + bool use_hest_addr; /* Currently, always false */
> } AcpiGhesState;
>
> -void acpi_build_hest(GArray *table_data, GArray *hardware_errors,
> +void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
> + GArray *hardware_errors,
> BIOSLinker *linker,
> const char *oem_id, const char *oem_table_id);
> void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
