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] [day] [month] [year] [list]
Message-ID: <20250530163558.2182fcb3@imammedo.users.ipa.redhat.com>
Date: Fri, 30 May 2025 16:35:58 +0200
From: Igor Mammedov <imammedo@...hat.com>
To: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc: "Michael S . Tsirkin" <mst@...hat.com>, Jonathan Cameron
 <Jonathan.Cameron@...wei.com>, Shiju Jose <shiju.jose@...wei.com>,
 qemu-arm@...gnu.org, qemu-devel@...gnu.org, Ani Sinha
 <anisinha@...hat.com>, Dongjiu Geng <gengdongjiu1@...il.com>, Peter Maydell
 <peter.maydell@...aro.org>, Shannon Zhao <shannon.zhaosl@...il.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 1/3] acpi/ghes: move use_hest_addr out of
 acpi_build_hest()

On Tue,  4 Mar 2025 14:30:56 +0100
Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:

> The only reason why we're passing ags to acpi HEST table build
> is to check if migration will be used or not.
> 
> Well, we only need migration for arm, as other architectures
> will only use the new code. So, move this out of acpi_build_hest(),

> as otherwise we can't use it for x86, as the hotplug logic there
> may not initialize ags during acpi table build time.

can you expand on what hotplug logic you are talking about?

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> ---
>  hw/acpi/ghes.c           | 16 ++++++++--------
>  hw/arm/virt-acpi-build.c | 12 ++++++++----
>  include/hw/acpi/ghes.h   | 25 +++++++++++++------------
>  3 files changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index 1fd5ba941771..ea00fed75c16 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -243,7 +243,7 @@ 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(AcpiGhesState *ags, GArray *hardware_errors,
> +static void build_ghes_error_table(GArray *hardware_errors, bool use_hest_addr,
>                                     BIOSLinker *linker, int num_sources)
>  {
>      int i, error_status_block_offset;
> @@ -289,7 +289,7 @@ static void build_ghes_error_table(AcpiGhesState *ags, GArray *hardware_errors,
>                                         i * ACPI_GHES_MAX_RAW_DATA_LENGTH);
>      }
>  
> -    if (!ags->use_hest_addr) {
> +    if (!use_hest_addr) {
>          /*
>           * Tell firmware to write hardware_errors GPA into
>           * hardware_errors_addr fw_cfg, once the former has been initialized.
> @@ -372,7 +372,7 @@ static void build_ghes_v2_entry(GArray *table_data,
>  }
>  
>  /* Build Hardware Error Source Table */
> -void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
> +void acpi_build_hest(GArray *table_data, bool use_hest_addr,
>                       GArray *hardware_errors,
>                       BIOSLinker *linker,
>                       const AcpiNotificationSourceId *notif_source,
> @@ -386,7 +386,7 @@ void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
>  
>      hest_offset = table_data->len;
>  
> -    build_ghes_error_table(ags, hardware_errors, linker, num_sources);
> +    build_ghes_error_table(hardware_errors, use_hest_addr, linker, num_sources);
>  
>      acpi_table_begin(&table, table_data);
>  
> @@ -398,7 +398,7 @@ void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
>  
>      acpi_table_end(linker, &table);
>  
> -    if (ags->use_hest_addr) {
> +    if (use_hest_addr) {
>          /*
>           * Tell firmware to write into GPA the address of HEST via fw_cfg,
>           * once initialized.
> @@ -411,13 +411,13 @@ void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
>  }
>  
>  void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> -                          GArray *hardware_error)
> +                          bool use_hest_addr, GArray *hardware_error)
>  {
>      /* Create a read-only fw_cfg file for GHES */
>      fw_cfg_add_file(s, ACPI_HW_ERROR_FW_CFG_FILE, hardware_error->data,
>                      hardware_error->len);
>  
> -    if (ags->use_hest_addr) {
> +    if (use_hest_addr) {
>          fw_cfg_add_file_callback(s, ACPI_HEST_ADDR_FW_CFG_FILE, NULL, NULL,
>              NULL, &(ags->hest_addr_le), sizeof(ags->hest_addr_le), false);
>      } else {
> @@ -529,7 +529,7 @@ void ghes_record_cper_errors(AcpiGhesState *ags, const void *cper, size_t len,
>          return;
>      }
>  
> -    if (!ags->use_hest_addr) {
> +    if (ags->hw_error_le) {
>          get_hw_error_offsets(le64_to_cpu(ags->hw_error_le),
>                               &cper_addr, &read_ack_register_addr);
>      } else {
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 154337e1a77b..71da17b652b2 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -964,9 +964,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>          acpi_ged_state = ACPI_GED(vms->acpi_dev);
>          ags = &acpi_ged_state->ghes_state;
>          if (ags) {
> +            bool use_hest_addr = ags->use_hest_addr;
> +
>              acpi_add_table(table_offsets, tables_blob);
>  
> -            if (!ags->use_hest_addr) {
> +            if (!use_hest_addr) {
>                  notify = hest_ghes_notify_9_2;
>                  notify_sz = ARRAY_SIZE(hest_ghes_notify_9_2);
>              } else {
> @@ -974,7 +976,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>                  notify_sz = ARRAY_SIZE(hest_ghes_notify);
>              }
>  
> -            acpi_build_hest(ags, tables_blob, tables->hardware_errors,
> +            acpi_build_hest(tables_blob, use_hest_addr, tables->hardware_errors,
>                              tables->linker, notify, notify_sz,
>                              vms->oem_id, vms->oem_table_id);
>          }
> @@ -1143,8 +1145,10 @@ void virt_acpi_setup(VirtMachineState *vms)
>      if (vms->ras) {
>          assert(vms->acpi_dev);
>          acpi_ged_state = ACPI_GED(vms->acpi_dev);
> -        acpi_ghes_add_fw_cfg(&acpi_ged_state->ghes_state,
> -                             vms->fw_cfg, tables.hardware_errors);
> +        AcpiGhesState *ags = &acpi_ged_state->ghes_state;
> +
> +        acpi_ghes_add_fw_cfg(ags, vms->fw_cfg, ags->use_hest_addr,
> +                             tables.hardware_errors);
>      }
>  
>      build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index df2ecbf6e4a9..eae6d4d0a562 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -73,31 +73,32 @@ typedef struct AcpiNotificationSourceId {
>      enum AcpiGhesNotifyType notify;
>  } AcpiNotificationSourceId;
>  
> -/*
> - * AcpiGhesState stores GPA values that will be used to fill HEST entries.
> +/**
> + * struct AcpiGhesState - 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.
> + * @hest_addr_le: GPA of the HEST table. Used on QEMU 10.x and above.
> + * @hw_error_le: GPA of the etc/hardware_errors firmware.
> + *               Used only on arm64 virt-9.x to preserve compatibility
> + *               with QEMU 9.x.
> + * @use_hest_addr: True if HEST address is present. Used only on arm64,
> + *                 to identify if QEMU 9.x migration is needed.
>   *
> - * When use_hest_addr is true, the GPA of the HEST table is stored at
> - * hest_addr_le. This is the default for QEMU 10.x and above.
> - *
> - * Whe both GPA values are equal to zero means that GHES is not present.
> + * When both GPA values are equal to zero means that GHES is not present.
>   */
>  typedef struct AcpiGhesState {
>      uint64_t hest_addr_le;
>      uint64_t hw_error_le;
> -    bool use_hest_addr; /* True if HEST address is present */
> +    bool use_hest_addr;
>  } AcpiGhesState;
>  
> -void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
> +void acpi_build_hest(GArray *table_data, bool use_hest_addr,
>                       GArray *hardware_errors,
>                       BIOSLinker *linker,
>                       const AcpiNotificationSourceId * const notif_source,
>                       int num_sources,
>                       const char *oem_id, const char *oem_table_id);
> -void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
> -                          GArray *hardware_errors);
> +void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> +                          bool use_hest_addr, GArray *hardware_error);
>  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,


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ