| 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
| ||
|
Message-ID: <20250129154620.4f7ac01c@imammedo.users.ipa.redhat.com>
Date: Wed, 29 Jan 2025 15:46:20 +0100
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, 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>, Peter Maydell <peter.maydell@...aro.org>,
Shannon Zhao <shannon.zhaosl@...il.com>, Yanan Wang
<wangyanan55@...wei.com>, Zhao Liu <zhao1.liu@...el.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 06/13] acpi/generic_event_device: add logic to detect
if HEST addr is available
On Wed, 29 Jan 2025 09:04:12 +0100
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 10.0 and upper) or via the legacy way via an offset obtained
^^^
s/via/where/
> from the hardware_errors firmware file.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> ---
> hw/acpi/generic_event_device.c | 1 +
> hw/acpi/ghes.c | 20 +++++++++++++-------
> hw/arm/virt-acpi-build.c | 30 ++++++++++++++++++++++++++----
> hw/core/machine.c | 2 ++
> include/hw/acpi/ghes.h | 4 +++-
> 5 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 5346cae573b7..70729b6238a5 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 const Property acpi_ged_properties[] = {
> DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
> + DEFINE_PROP_BOOL("x-has-hest-addr", AcpiGedState, ghes_state.use_hest_addr, true),
> };
>
> static const VMStateDescription vmstate_memhp_state = {
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index adf80945c6db..736287766989 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -353,7 +353,8 @@ static void build_ghes_v2_entry(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 AcpiNotificationSourceId *notif_source,
> int num_sources,
> @@ -382,10 +383,13 @@ 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);
> +
> + if (ags->use_hest_addr) {
> + bios_linker_loader_write_pointer(linker,
> + ACPI_HEST_ADDR_FW_CFG_FILE, 0,
> + sizeof(uint64_t),
> + ACPI_BUILD_TABLE_FILE, hest_offset);
> + }
if ags->use_hest_addr is true, then we can safely exclude
hardware_errors_addr fw_cfg
it shouldn't be used when use_hest_addr == true,
something along lines:
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index 7362877669..a30f65f9f5 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -272,6 +272,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker,
i * ACPI_GHES_MAX_RAW_DATA_LENGTH);
}
+if (!ags->use_hest_addr) {
/*
* Tell firmware to write hardware_errors GPA into
* hardware_errors_addr fw_cfg, once the former has been initialized.
@@ -280,6 +281,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker,
sizeof(uint64_t),
ACPI_HW_ERROR_FW_CFG_FILE, 0);
}
+}
/* Build Generic Hardware Error Source version 2 (GHESv2) */
static void build_ghes_v2_entry(GArray *table_data,
@@ -399,13 +401,13 @@ 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) {
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 {
+ /* 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);
}
ags->present = true;
> }
>
> void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> @@ -399,8 +403,10 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> 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);
>
> - 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);
> + if (ags->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);
> + }
>
> ags->present = true;
> }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 3d411787fc37..be1e51e0bb29 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -897,6 +897,10 @@ static const AcpiNotificationSourceId hest_ghes_notify[] = {
> { ACPI_HEST_SRC_ID_SYNC, ACPI_GHES_NOTIFY_SEA },
> };
>
> +static const AcpiNotificationSourceId hest_ghes_notify_9_2[] = {
> + { ACPI_HEST_SRC_ID_SYNC, ACPI_GHES_NOTIFY_SEA },
> +};
> +
> static
> void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
> {
> @@ -950,10 +954,28 @@ 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,
> - hest_ghes_notify, ARRAY_SIZE(hest_ghes_notify),
> - vms->oem_id, vms->oem_table_id);
> + AcpiGhesState *ags;
> + AcpiGedState *acpi_ged_state;
I'd do something like this, instead of calling acpi_build_hest() twice
notify = hest_ghes_notify_9_2
notify_sz = ...
> +
> + acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
> + NULL));
> + if (acpi_ged_state) {
> + ags = &acpi_ged_state->ghes_state;
> +
> + acpi_add_table(table_offsets, tables_blob);
> +
> + if (!ags->use_hest_addr) {
notify = hest_ghes_notify
notify_sz =
> + }
acpi_build_hest(ags, tables_blob, tables->hardware_errors,
tables->linker, notify,
notify_sz,
vms->oem_id, vms->oem_table_id);
> + }
> }
>
> if (ms->numa_state->num_nodes > 0) {
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index c23b39949649..0d0cde481954 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,10 +34,12 @@
> #include "hw/virtio/virtio-pci.h"
> #include "hw/virtio/virtio-net.h"
> #include "hw/virtio/virtio-iommu.h"
> +#include "hw/acpi/generic_event_device.h"
> #include "audio/audio.h"
>
> GlobalProperty hw_compat_9_2[] = {
> {"arm-cpu", "backcompat-pauth-default-use-qarma5", "true"},
> + { TYPE_ACPI_GED, "x-has-hest-addr", "false" },
> };
> const size_t hw_compat_9_2_len = G_N_ELEMENTS(hw_compat_9_2);
>
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index 237721fec0a2..bfc8fd851648 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 use_hest_addr; /* True if HEST address is present */
> } AcpiGhesState;
>
> /*
> @@ -75,7 +76,8 @@ typedef struct AcpiNotificationSourceId {
> enum AcpiGhesNotifyType notify;
> } AcpiNotificationSourceId;
>
> -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 AcpiNotificationSourceId * const notif_source,
> int num_sources,
Powered by blists - more mailing lists