On Wed, 13 Nov 2024 09:37:03 +0100
Mauro Carvalho Chehab <mchehab+hua...@kernel.org> wrote:
> Create a new property (x-has-hest-addr) and use it to detect if
> the GHES table offsets can be calculated from the HEST address
> (qemu 9.2 and upper) or via the legacy way via an offset obtained
> from the hardware_errors firmware file.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
Comments inline.
Nice work hammering all this into shape.
Thanks,
Jonathan
> ---
> hw/acpi/generic_event_device.c | 1 +
> hw/acpi/ghes.c | 27 ++++++++++++++++++++-------
> hw/arm/virt-acpi-build.c | 30 ++++++++++++++++++++++++++----
> hw/core/machine.c | 2 ++
> include/hw/acpi/ghes.h | 1 +
> 5 files changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index c1116dd8d7ae..df6b4fab2d30 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -318,6 +318,7 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev,
> AcpiEventStatusBits ev)
>
> static Property acpi_ged_properties[] = {
> DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
> + DEFINE_PROP_BOOL("x-has-hest-addr", AcpiGedState,
> ghes_state.hest_lookup, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index 9ee25efe8abf..2d34a6ddf133 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -365,6 +365,8 @@ void acpi_build_hest(GArray *table_data, GArray
> *hardware_errors,
> {
> AcpiTable table = { .sig = "HEST", .rev = 1,
> .oem_id = oem_id, .oem_table_id = oem_table_id };
> + AcpiGedState *acpi_ged_state;
> + AcpiGhesState *ags = NULL;
> int i;
>
> build_ghes_error_table(hardware_errors, linker, num_sources);
> @@ -385,10 +387,19 @@ void acpi_build_hest(GArray *table_data, GArray
> *hardware_errors,
> * tell firmware to write into GPA the address of HEST via fw_cfg,
> * once initialized.
> */
> - bios_linker_loader_write_pointer(linker,
> - ACPI_HEST_ADDR_FW_CFG_FILE, 0,
> - sizeof(uint64_t),
> - ACPI_BUILD_TABLE_FILE, hest_offset);
> +
> + acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
> + NULL));
> + if (acpi_ged_state) {
Won't fail, but if it did (And given you have a check you are assuming
it might).
> + ags = &acpi_ged_state->ghes_state;
> + }
> +
Then ags is NULL and boom.
> + if (ags->hest_lookup) {
> + bios_linker_loader_write_pointer(linker,
> + ACPI_HEST_ADDR_FW_CFG_FILE, 0,
> + sizeof(uint64_t),
> + ACPI_BUILD_TABLE_FILE, hest_offset);
> + }
> }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 40f66792570c..930ba9e0a14c 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -893,6 +893,10 @@ static const AcpiNotificationSourceId hest_ghes_notify[]
> = {
> {ACPI_HEST_SRC_ID_SYNC, ACPI_GHES_NOTIFY_SEA},
> };
>
> +static const AcpiNotificationSourceId hest_ghes_notify_9_1[] = {
> + {ACPI_HEST_SRC_ID_SYNC, ACPI_GHES_NOTIFY_SEA},
{ ACPI...
> +};
