Re: [PATCH RFC 4/5] acpi/generic_event_device: add logic to detect if HEST addr is available

[Igor Mammedov <imammedo@redhat.com>]

On Tue,  1 Oct 2024 13:42:49 +0200
Mauro Carvalho Chehab <mchehab+huawei@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+huawei@kernel.org>
> ---
>  hw/acpi/generic_event_device.c | 1 +
>  hw/acpi/ghes.c                 | 2 +-
>  hw/core/machine.c              | 4 +++-
>  include/hw/acpi/ghes.h         | 1 +
>  4 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 49ca1fb8e84a..c4677c9ae6b4 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 313a6e453af6..86bad865168c 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -513,7 +513,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
>      }
>      ags = &acpi_ged_state->ghes_state;
>  
> -    if (!ags->hest_addr_le) {
> +    if (!ags->hest_lookup) {
>          get_ghes_offsets(le64_to_cpu(ags->hw_error_le),
>                           &cper_addr, &read_ack_register_addr);

just fencing off lookup is not enough,
to be compatible with qemu-9.1 (virt-9.1) we also should not publish hest_addr fwcfg.

Also have assert (to be removed later) to make sure that we put only
1 (existing) error source in HEST.
When you switch to multiple sources, this will become a condition
to switch HEST generation between 9.1 and 9.2+ variants.


>      } else {
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index adaba17ebac1..b58afe48aa71 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,7 +34,9 @@
>  #include "hw/virtio/virtio-iommu.h"
>  #include "audio/audio.h"
>  
> -GlobalProperty hw_compat_9_1[] = {};
> +GlobalProperty hw_compat_9_1[] = {
> +    {"x-has-hest-addr", "false"},
> +};
>  const size_t hw_compat_9_1_len = G_N_ELEMENTS(hw_compat_9_1);
>  
>  GlobalProperty hw_compat_9_0[] = {
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index a07c30ef13b7..040d6ee366b2 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -61,6 +61,7 @@ typedef struct AcpiGhesState {
>      uint64_t hest_addr_le;
>      uint64_t hw_error_le;
>      bool present; /* True if GHES is present at all on this board */
> +    bool hest_lookup; /* True if HEST address is present */
>  } AcpiGhesState;
>  
>  /*