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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241003162728.1de6fc62@imammedo.users.ipa.redhat.com>
Date: Thu, 3 Oct 2024 16:27:28 +0200
From: Igor Mammedov <imammedo@...hat.com>
To: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>, Shiju Jose
 <shiju.jose@...wei.com>, "Michael S. Tsirkin" <mst@...hat.com>, Philippe
 Mathieu-Daudé <philmd@...aro.org>, Ani Sinha
 <anisinha@...hat.com>, Dongjiu Geng <gengdongjiu1@...il.com>, Eduardo
 Habkost <eduardo@...kost.net>, Marcel Apfelbaum
 <marcel.apfelbaum@...il.com>, Yanan Wang <wangyanan55@...wei.com>, Zhao Liu
 <zhao1.liu@...el.com>, linux-kernel@...r.kernel.org, qemu-arm@...gnu.org,
 qemu-devel@...gnu.org
Subject: Re: [PATCH RFC 4/5] acpi/generic_event_device: add logic to detect
 if HEST addr is available

On Tue,  1 Oct 2024 13:42:49 +0200
Mauro Carvalho Chehab <mchehab+huawei@...nel.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@...nel.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;
>  
>  /*