| Message ID | 5d53042ebc5bc73bbc71f600e1ec1dea41f346b9.1722634602.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ACPI CPER firmware first error injection on ARM emulation | expand |
On Fri, 2 Aug 2024 23:43:59 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Add error notification to GHES v2 using the GPIO source. The gpio / external interrupt follows through. > > [mchehab: do some cleanups at ACPI_HEST_SRC_ID_* checks] > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/acpi/ghes.c | 16 ++++++++++------ > include/hw/acpi/ghes.h | 3 ++- > 2 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > index 8d0262e6c1aa..a745dcc7be5e 100644 > --- a/hw/acpi/ghes.c > +++ b/hw/acpi/ghes.c > @@ -34,8 +34,8 @@ > /* The max size in bytes for one error block */ > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > -/* Now only support ARMv8 SEA notification type error source */ > -#define ACPI_GHES_ERROR_SOURCE_COUNT 1 > +/* Support ARMv8 SEA notification type error source and GPIO interrupt. */ > +#define ACPI_GHES_ERROR_SOURCE_COUNT 2 > > /* Generic Hardware Error Source version 2 */ > #define ACPI_GHES_SOURCE_GENERIC_ERROR_V2 10 > @@ -290,6 +290,9 @@ void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker) > static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) > { > uint64_t address_offset; > + > + assert(source_id < ACPI_HEST_SRC_ID_RESERVED); > + > /* > * Type: > * Generic Hardware Error Source version 2(GHESv2 - Type 10) > @@ -327,6 +330,9 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) > */ > build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_SEA); > break; > + case ACPI_HEST_SRC_ID_GPIO: > + build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_GPIO); > + break; > default: > error_report("Not support this error source"); > abort(); > @@ -370,6 +376,7 @@ void acpi_build_hest(GArray *table_data, BIOSLinker *linker, > /* Error Source Count */ > build_append_int_noprefix(table_data, ACPI_GHES_ERROR_SOURCE_COUNT, 4); > build_ghes_v2(table_data, ACPI_HEST_SRC_ID_SEA, linker); > + build_ghes_v2(table_data, ACPI_HEST_SRC_ID_GPIO, linker); > > acpi_table_end(linker, &table); > } > @@ -406,10 +413,7 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address) > start_addr = le64_to_cpu(ags->ghes_addr_le); > > if (physical_address) { > - > - if (source_id < ACPI_HEST_SRC_ID_RESERVED) { > - start_addr += source_id * sizeof(uint64_t); > - } > + start_addr += source_id * sizeof(uint64_t); > > cpu_physical_memory_read(start_addr, &error_block_addr, > sizeof(error_block_addr)); > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h > index 6891eafff5ab..33be1eb5acf4 100644 > --- a/include/hw/acpi/ghes.h > +++ b/include/hw/acpi/ghes.h > @@ -59,9 +59,10 @@ enum AcpiGhesNotifyType { > ACPI_GHES_NOTIFY_RESERVED = 12 > }; > > +/* Those are used as table indexes when building GHES tables */ > enum { > ACPI_HEST_SRC_ID_SEA = 0, > - /* future ids go here */ > + ACPI_HEST_SRC_ID_GPIO, > ACPI_HEST_SRC_ID_RESERVED, > }; >
Em Mon, 5 Aug 2024 17:56:17 +0100 Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu: > On Fri, 2 Aug 2024 23:43:59 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > Add error notification to GHES v2 using the GPIO source. > > The gpio / external interrupt follows through. True. As session 18.3.2.7 of the spec says: The OSPM evaluates the control method associated with this event as indicated in The Event Method for Handling GPIO Signaled Events and The Event Method for Handling Interrupt Signaled Events. E. g. defining two methods: - GED GPIO; - GED interrupt I'm doing this rename: ACPI_HEST_SRC_ID_GPIO -> ACPI_HEST_SRC_ID_GED_INT To clearly state what it is implemented there. I'm also changing patch description to: acpi/ghes: Add support for General Purpose Event As a GED error device is now defined, add another type of notification. Add error notification to GHES v2 using the GPIO source. [mchehab: do some cleanups at ACPI_HEST_SRC_ID_* checks and rename HEST event to better identify GED interrupt OSPM] Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Regards, Mauro
On Tue, 6 Aug 2024 08:09:28 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Em Mon, 5 Aug 2024 17:56:17 +0100 > Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu: > > > On Fri, 2 Aug 2024 23:43:59 +0200 > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > > > Add error notification to GHES v2 using the GPIO source. > > > > The gpio / external interrupt follows through. > > True. As session 18.3.2.7 of the spec says: > > The OSPM evaluates the control method associated with this event > as indicated in The Event Method for Handling GPIO Signaled Events > and The Event Method for Handling Interrupt Signaled Events. > > E. g. defining two methods: > - GED GPIO; > - GED interrupt > > I'm doing this rename: > > ACPI_HEST_SRC_ID_GPIO -> ACPI_HEST_SRC_ID_GED_INT > > To clearly state what it is implemented there. > > I'm also changing patch description to: > > acpi/ghes: Add support for General Purpose Event > > As a GED error device is now defined, add another type > of notification. > > Add error notification to GHES v2 using the GPIO source. ^^^^ did you mean: GED? > > [mchehab: do some cleanups at ACPI_HEST_SRC_ID_* checks and > rename HEST event to better identify GED interrupt OSPM] > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Regards, > Mauro >
On Fri, 2 Aug 2024 23:43:59 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Add error notification to GHES v2 using the GPIO source. > > [mchehab: do some cleanups at ACPI_HEST_SRC_ID_* checks] > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/acpi/ghes.c | 16 ++++++++++------ > include/hw/acpi/ghes.h | 3 ++- > 2 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > index 8d0262e6c1aa..a745dcc7be5e 100644 > --- a/hw/acpi/ghes.c > +++ b/hw/acpi/ghes.c > @@ -34,8 +34,8 @@ > /* The max size in bytes for one error block */ > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > -/* Now only support ARMv8 SEA notification type error source */ > -#define ACPI_GHES_ERROR_SOURCE_COUNT 1 > +/* Support ARMv8 SEA notification type error source and GPIO interrupt. */ > +#define ACPI_GHES_ERROR_SOURCE_COUNT 2 > > /* Generic Hardware Error Source version 2 */ > #define ACPI_GHES_SOURCE_GENERIC_ERROR_V2 10 > @@ -290,6 +290,9 @@ void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker) > static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) > { > uint64_t address_offset; > + > + assert(source_id < ACPI_HEST_SRC_ID_RESERVED); > + > /* > * Type: > * Generic Hardware Error Source version 2(GHESv2 - Type 10) > @@ -327,6 +330,9 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) > */ > build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_SEA); > break; > + case ACPI_HEST_SRC_ID_GPIO: > + build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_GPIO); perhaps ACPI_GHES_NOTIFY_EXTERNAL fits better here? > + break; > default: > error_report("Not support this error source"); > abort(); > @@ -370,6 +376,7 @@ void acpi_build_hest(GArray *table_data, BIOSLinker *linker, > /* Error Source Count */ > build_append_int_noprefix(table_data, ACPI_GHES_ERROR_SOURCE_COUNT, 4); > build_ghes_v2(table_data, ACPI_HEST_SRC_ID_SEA, linker); > + build_ghes_v2(table_data, ACPI_HEST_SRC_ID_GPIO, linker); > > acpi_table_end(linker, &table); > } > @@ -406,10 +413,7 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address) > start_addr = le64_to_cpu(ags->ghes_addr_le); > > if (physical_address) { > - > - if (source_id < ACPI_HEST_SRC_ID_RESERVED) { > - start_addr += source_id * sizeof(uint64_t); > - } > + start_addr += source_id * sizeof(uint64_t); why check is being removed? > > cpu_physical_memory_read(start_addr, &error_block_addr, > sizeof(error_block_addr)); > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h > index 6891eafff5ab..33be1eb5acf4 100644 > --- a/include/hw/acpi/ghes.h > +++ b/include/hw/acpi/ghes.h > @@ -59,9 +59,10 @@ enum AcpiGhesNotifyType { > ACPI_GHES_NOTIFY_RESERVED = 12 > }; > > +/* Those are used as table indexes when building GHES tables */ > enum { > ACPI_HEST_SRC_ID_SEA = 0, > - /* future ids go here */ > + ACPI_HEST_SRC_ID_GPIO, > ACPI_HEST_SRC_ID_RESERVED, > }; >
Em Tue, 6 Aug 2024 11:32:19 +0200 Igor Mammedov <imammedo@redhat.com> escreveu: > > @@ -327,6 +330,9 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) > > */ > > build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_SEA); > > break; > > + case ACPI_HEST_SRC_ID_GPIO: > > + build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_GPIO); > > perhaps ACPI_GHES_NOTIFY_EXTERNAL fits better here? Symbol already used to map the 12 possible notification types from ACPI spec. I did a: sed s,ACPI_HEST_SRC_ID_GED_INT,ACPI_HEST_NOTIFY_EXTERNAL, instead. Thanks, Mauro
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c index 8d0262e6c1aa..a745dcc7be5e 100644 --- a/hw/acpi/ghes.c +++ b/hw/acpi/ghes.c @@ -34,8 +34,8 @@ /* The max size in bytes for one error block */ #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) -/* Now only support ARMv8 SEA notification type error source */ -#define ACPI_GHES_ERROR_SOURCE_COUNT 1 +/* Support ARMv8 SEA notification type error source and GPIO interrupt. */ +#define ACPI_GHES_ERROR_SOURCE_COUNT 2 /* Generic Hardware Error Source version 2 */ #define ACPI_GHES_SOURCE_GENERIC_ERROR_V2 10 @@ -290,6 +290,9 @@ void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker) static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) { uint64_t address_offset; + + assert(source_id < ACPI_HEST_SRC_ID_RESERVED); + /* * Type: * Generic Hardware Error Source version 2(GHESv2 - Type 10) @@ -327,6 +330,9 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker) */ build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_SEA); break; + case ACPI_HEST_SRC_ID_GPIO: + build_ghes_hw_error_notification(table_data, ACPI_GHES_NOTIFY_GPIO); + break; default: error_report("Not support this error source"); abort(); @@ -370,6 +376,7 @@ void acpi_build_hest(GArray *table_data, BIOSLinker *linker, /* Error Source Count */ build_append_int_noprefix(table_data, ACPI_GHES_ERROR_SOURCE_COUNT, 4); build_ghes_v2(table_data, ACPI_HEST_SRC_ID_SEA, linker); + build_ghes_v2(table_data, ACPI_HEST_SRC_ID_GPIO, linker); acpi_table_end(linker, &table); } @@ -406,10 +413,7 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address) start_addr = le64_to_cpu(ags->ghes_addr_le); if (physical_address) { - - if (source_id < ACPI_HEST_SRC_ID_RESERVED) { - start_addr += source_id * sizeof(uint64_t); - } + start_addr += source_id * sizeof(uint64_t); cpu_physical_memory_read(start_addr, &error_block_addr, sizeof(error_block_addr)); diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h index 6891eafff5ab..33be1eb5acf4 100644 --- a/include/hw/acpi/ghes.h +++ b/include/hw/acpi/ghes.h @@ -59,9 +59,10 @@ enum AcpiGhesNotifyType { ACPI_GHES_NOTIFY_RESERVED = 12 }; +/* Those are used as table indexes when building GHES tables */ enum { ACPI_HEST_SRC_ID_SEA = 0, - /* future ids go here */ + ACPI_HEST_SRC_ID_GPIO, ACPI_HEST_SRC_ID_RESERVED, };