The hardware error firmware is where HEST error structures are
stored. Those can be GHESv2, but they can also be other types.
Better name the location of the hardware error.
No functional changes.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/generic_event_device.c | 4 ++--
hw/acpi/ghes.c | 4 ++--
include/hw/acpi/ghes.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 663d9cb09380..17baf36132a8 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -364,7 +364,7 @@ static const VMStateDescription vmstate_ghes = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (const VMStateField[]) {
- VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
+ VMSTATE_UINT64(hw_error_le, AcpiGhesState),
VMSTATE_END_OF_LIST()
},
};
@@ -372,7 +372,7 @@ static const VMStateDescription vmstate_ghes = {
static bool ghes_needed(void *opaque)
{
AcpiGedState *s = opaque;
- return s->ghes_state.ghes_addr_le;
+ return s->ghes_state.hw_error_le;
}
static const VMStateDescription vmstate_ghes_state = {
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index 52c2b69d3664..90d76b9c2d8c 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -359,7 +359,7 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
/* 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->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
+ NULL, &(ags->hw_error_le), sizeof(ags->hw_error_le), false);
ags->present = true;
}
@@ -385,7 +385,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
}
ags = &acpi_ged_state->ghes_state;
- start_addr = le64_to_cpu(ags->ghes_addr_le);
+ start_addr = le64_to_cpu(ags->hw_error_le);
start_addr += source_id * sizeof(uint64_t);
diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index 21666a4bcc8b..39619a2457cb 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -65,7 +65,7 @@ enum {
};
typedef struct AcpiGhesState {
- uint64_t ghes_addr_le;
+ uint64_t hw_error_le;
bool present; /* True if GHES is present at all on this board */
} AcpiGhesState;
--
2.47.1On Wed, 4 Dec 2024 08:41:21 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> The hardware error firmware is where HEST error structures are
^^^^^^^^^^^^^^^^^^^^^^^ I can't parse this, suspect you've meant something else here
> stored. Those can be GHESv2, but they can also be other types.
>
> Better name the location of the hardware error.
>
> No functional changes.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/acpi/generic_event_device.c | 4 ++--
> hw/acpi/ghes.c | 4 ++--
> include/hw/acpi/ghes.h | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 663d9cb09380..17baf36132a8 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -364,7 +364,7 @@ static const VMStateDescription vmstate_ghes = {
> .version_id = 1,
> .minimum_version_id = 1,
> .fields = (const VMStateField[]) {
> - VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> + VMSTATE_UINT64(hw_error_le, AcpiGhesState),
> VMSTATE_END_OF_LIST()
> },
> };
> @@ -372,7 +372,7 @@ static const VMStateDescription vmstate_ghes = {
> static bool ghes_needed(void *opaque)
> {
> AcpiGedState *s = opaque;
> - return s->ghes_state.ghes_addr_le;
> + return s->ghes_state.hw_error_le;
> }
>
> static const VMStateDescription vmstate_ghes_state = {
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index 52c2b69d3664..90d76b9c2d8c 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -359,7 +359,7 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
>
> /* 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->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
> + NULL, &(ags->hw_error_le), sizeof(ags->hw_error_le), false);
>
> ags->present = true;
> }
> @@ -385,7 +385,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
> }
> ags = &acpi_ged_state->ghes_state;
>
> - start_addr = le64_to_cpu(ags->ghes_addr_le);
> + start_addr = le64_to_cpu(ags->hw_error_le);
>
> start_addr += source_id * sizeof(uint64_t);
>
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index 21666a4bcc8b..39619a2457cb 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -65,7 +65,7 @@ enum {
> };
>
> typedef struct AcpiGhesState {
> - uint64_t ghes_addr_le;
> + uint64_t hw_error_le;
> bool present; /* True if GHES is present at all on this board */
> } AcpiGhesState;
>
Em Wed, 4 Dec 2024 17:37:59 +0100
Igor Mammedov <imammedo@redhat.com> escreveu:
> On Wed, 4 Dec 2024 08:41:21 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
> > The hardware error firmware is where HEST error structures are
> ^^^^^^^^^^^^^^^^^^^^^^^ I can't parse this, suspect you've meant something else here
>
> > stored. Those can be GHESv2, but they can also be other types.
> >
> > Better name the location of the hardware error.
> >
> > No functional changes.
I meant this fw_cfg file:
#define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors"
#define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr"
What about changing description to:
The etc/hardware_errors fw_cfg file is where the HEST error
source structures are stored. Those can be GHESv2, but they can also
be other types.
For more details about error source structure, see:
https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#acpi-error-source
Better name the address variable from ghes_error_le to hw_error_le
to better reflect that.
No functional changes.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > hw/acpi/generic_event_device.c | 4 ++--
> > hw/acpi/ghes.c | 4 ++--
> > include/hw/acpi/ghes.h | 2 +-
> > 3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > index 663d9cb09380..17baf36132a8 100644
> > --- a/hw/acpi/generic_event_device.c
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -364,7 +364,7 @@ static const VMStateDescription vmstate_ghes = {
> > .version_id = 1,
> > .minimum_version_id = 1,
> > .fields = (const VMStateField[]) {
> > - VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> > + VMSTATE_UINT64(hw_error_le, AcpiGhesState),
> > VMSTATE_END_OF_LIST()
> > },
> > };
> > @@ -372,7 +372,7 @@ static const VMStateDescription vmstate_ghes = {
> > static bool ghes_needed(void *opaque)
> > {
> > AcpiGedState *s = opaque;
> > - return s->ghes_state.ghes_addr_le;
> > + return s->ghes_state.hw_error_le;
> > }
> >
> > static const VMStateDescription vmstate_ghes_state = {
> > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > index 52c2b69d3664..90d76b9c2d8c 100644
> > --- a/hw/acpi/ghes.c
> > +++ b/hw/acpi/ghes.c
> > @@ -359,7 +359,7 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> >
> > /* 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->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
> > + NULL, &(ags->hw_error_le), sizeof(ags->hw_error_le), false);
> >
> > ags->present = true;
> > }
> > @@ -385,7 +385,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
> > }
> > ags = &acpi_ged_state->ghes_state;
> >
> > - start_addr = le64_to_cpu(ags->ghes_addr_le);
> > + start_addr = le64_to_cpu(ags->hw_error_le);
> >
> > start_addr += source_id * sizeof(uint64_t);
> >
> > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> > index 21666a4bcc8b..39619a2457cb 100644
> > --- a/include/hw/acpi/ghes.h
> > +++ b/include/hw/acpi/ghes.h
> > @@ -65,7 +65,7 @@ enum {
> > };
> >
> > typedef struct AcpiGhesState {
> > - uint64_t ghes_addr_le;
> > + uint64_t hw_error_le;
> > bool present; /* True if GHES is present at all on this board */
> > } AcpiGhesState;
> >
>
Thanks,
Mauro
On Thu, 5 Dec 2024 08:59:59 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> Em Wed, 4 Dec 2024 17:37:59 +0100
> Igor Mammedov <imammedo@redhat.com> escreveu:
>
> > On Wed, 4 Dec 2024 08:41:21 +0100
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> >
> > > The hardware error firmware is where HEST error structures are
> > ^^^^^^^^^^^^^^^^^^^^^^^ I can't parse this, suspect you've meant something else here
> >
> > > stored. Those can be GHESv2, but they can also be other types.
> > >
> > > Better name the location of the hardware error.
> > >
> > > No functional changes.
>
> I meant this fw_cfg file:
> #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors"
> #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr"
>
> What about changing description to:
>
> The etc/hardware_errors fw_cfg file is where the HEST error
> source structures are stored. Those can be GHESv2, but they can also
> be other types.
As I understand it, etc/hardware_errors is a blob
for '18.3.2.7.1. Generic Error Data' with some extra fields
to accommodate GHESv2 handling (i.e. err addr indirection and ack reg).
While error sources are described in HEST (and only GHES ones would
reference etc/hardware_errors via error status addr/read ack register addr)
>
> For more details about error source structure, see:
> https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#acpi-error-source
>
> Better name the address variable from ghes_error_le to hw_error_le
> to better reflect that.
>
> No functional changes.
>
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > hw/acpi/generic_event_device.c | 4 ++--
> > > hw/acpi/ghes.c | 4 ++--
> > > include/hw/acpi/ghes.h | 2 +-
> > > 3 files changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > > index 663d9cb09380..17baf36132a8 100644
> > > --- a/hw/acpi/generic_event_device.c
> > > +++ b/hw/acpi/generic_event_device.c
> > > @@ -364,7 +364,7 @@ static const VMStateDescription vmstate_ghes = {
> > > .version_id = 1,
> > > .minimum_version_id = 1,
> > > .fields = (const VMStateField[]) {
> > > - VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> > > + VMSTATE_UINT64(hw_error_le, AcpiGhesState),
> > > VMSTATE_END_OF_LIST()
> > > },
> > > };
> > > @@ -372,7 +372,7 @@ static const VMStateDescription vmstate_ghes = {
> > > static bool ghes_needed(void *opaque)
> > > {
> > > AcpiGedState *s = opaque;
> > > - return s->ghes_state.ghes_addr_le;
> > > + return s->ghes_state.hw_error_le;
> > > }
> > >
> > > static const VMStateDescription vmstate_ghes_state = {
> > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > > index 52c2b69d3664..90d76b9c2d8c 100644
> > > --- a/hw/acpi/ghes.c
> > > +++ b/hw/acpi/ghes.c
> > > @@ -359,7 +359,7 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> > >
> > > /* 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->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
> > > + NULL, &(ags->hw_error_le), sizeof(ags->hw_error_le), false);
> > >
> > > ags->present = true;
> > > }
> > > @@ -385,7 +385,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
> > > }
> > > ags = &acpi_ged_state->ghes_state;
> > >
> > > - start_addr = le64_to_cpu(ags->ghes_addr_le);
> > > + start_addr = le64_to_cpu(ags->hw_error_le);
> > >
> > > start_addr += source_id * sizeof(uint64_t);
> > >
> > > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> > > index 21666a4bcc8b..39619a2457cb 100644
> > > --- a/include/hw/acpi/ghes.h
> > > +++ b/include/hw/acpi/ghes.h
> > > @@ -65,7 +65,7 @@ enum {
> > > };
> > >
> > > typedef struct AcpiGhesState {
> > > - uint64_t ghes_addr_le;
> > > + uint64_t hw_error_le;
> > > bool present; /* True if GHES is present at all on this board */
> > > } AcpiGhesState;
> > >
> >
>
>
>
> Thanks,
> Mauro
>
Em Thu, 5 Dec 2024 10:22:19 +0100
Igor Mammedov <imammedo@redhat.com> escreveu:
> On Thu, 5 Dec 2024 08:59:59 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
> > Em Wed, 4 Dec 2024 17:37:59 +0100
> > Igor Mammedov <imammedo@redhat.com> escreveu:
> >
> > > On Wed, 4 Dec 2024 08:41:21 +0100
> > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> > >
> > > > The hardware error firmware is where HEST error structures are
> > > ^^^^^^^^^^^^^^^^^^^^^^^ I can't parse this, suspect you've meant something else here
> > >
> > > > stored. Those can be GHESv2, but they can also be other types.
> > > >
> > > > Better name the location of the hardware error.
> > > >
> > > > No functional changes.
> >
> > I meant this fw_cfg file:
> > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors"
> > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr"
> >
> > What about changing description to:
> >
> > The etc/hardware_errors fw_cfg file is where the HEST error
> > source structures are stored. Those can be GHESv2, but they can also
> > be other types.
>
> As I understand it, etc/hardware_errors is a blob
> for '18.3.2.7.1. Generic Error Data' with some extra fields
> to accommodate GHESv2 handling (i.e. err addr indirection and ack reg).
>
> While error sources are described in HEST (and only GHES ones would
> reference etc/hardware_errors via error status addr/read ack register addr)
Yeah, what it is stored there is not the error source structures
themselves, but the additional address data they contain. For GHESv2,
such data is the Error Status Address (where CPER data is stored) and
the Read Ack Register.
Currently, only GHES (type 9) and GHESv2 (type 10) have those
Generic Address Structure stored there.
What about then:
The etc/hardware_errors fw_cfg file is where the HEST error
source structures store registers pointed by Generic Address
Structures, as defined at:
https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#generic-error-data-entry
and
https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#generic-hardware-error-source-version-2-ghesv2-structure
As the name of the firmware file is hardware_errors, better
name the variable where the offset pointing to it will be stored
from ghes_error_le to hw_error_le.
No functional changes.
Regards,
Mauro
>
> >
> > For more details about error source structure, see:
> > https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#acpi-error-source
> >
> > Better name the address variable from ghes_error_le to hw_error_le
> > to better reflect that.
> >
> > No functional changes.
> >
> > > >
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > > hw/acpi/generic_event_device.c | 4 ++--
> > > > hw/acpi/ghes.c | 4 ++--
> > > > include/hw/acpi/ghes.h | 2 +-
> > > > 3 files changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > > > index 663d9cb09380..17baf36132a8 100644
> > > > --- a/hw/acpi/generic_event_device.c
> > > > +++ b/hw/acpi/generic_event_device.c
> > > > @@ -364,7 +364,7 @@ static const VMStateDescription vmstate_ghes = {
> > > > .version_id = 1,
> > > > .minimum_version_id = 1,
> > > > .fields = (const VMStateField[]) {
> > > > - VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> > > > + VMSTATE_UINT64(hw_error_le, AcpiGhesState),
> > > > VMSTATE_END_OF_LIST()
> > > > },
> > > > };
> > > > @@ -372,7 +372,7 @@ static const VMStateDescription vmstate_ghes = {
> > > > static bool ghes_needed(void *opaque)
> > > > {
> > > > AcpiGedState *s = opaque;
> > > > - return s->ghes_state.ghes_addr_le;
> > > > + return s->ghes_state.hw_error_le;
> > > > }
> > > >
> > > > static const VMStateDescription vmstate_ghes_state = {
> > > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > > > index 52c2b69d3664..90d76b9c2d8c 100644
> > > > --- a/hw/acpi/ghes.c
> > > > +++ b/hw/acpi/ghes.c
> > > > @@ -359,7 +359,7 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
> > > >
> > > > /* 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->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
> > > > + NULL, &(ags->hw_error_le), sizeof(ags->hw_error_le), false);
> > > >
> > > > ags->present = true;
> > > > }
> > > > @@ -385,7 +385,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
> > > > }
> > > > ags = &acpi_ged_state->ghes_state;
> > > >
> > > > - start_addr = le64_to_cpu(ags->ghes_addr_le);
> > > > + start_addr = le64_to_cpu(ags->hw_error_le);
> > > >
> > > > start_addr += source_id * sizeof(uint64_t);
> > > >
> > > > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> > > > index 21666a4bcc8b..39619a2457cb 100644
> > > > --- a/include/hw/acpi/ghes.h
> > > > +++ b/include/hw/acpi/ghes.h
> > > > @@ -65,7 +65,7 @@ enum {
> > > > };
> > > >
> > > > typedef struct AcpiGhesState {
> > > > - uint64_t ghes_addr_le;
> > > > + uint64_t hw_error_le;
> > > > bool present; /* True if GHES is present at all on this board */
> > > > } AcpiGhesState;
> > > >
> > >
> >
> >
> >
> > Thanks,
> > Mauro
> >
>
Thanks,
Mauro
© 2016 - 2025 Red Hat, Inc.