| Message ID | a3579f6c2c24e764e544f83b6e1b2dbef730e3e3.1737560101.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Change ghes to use HEST-based offsets and add support for error inject | expand |
On Wed, 22 Jan 2025 16:46:19 +0100 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Store HEST table address at GPA, placing its content at > hest_addr_le variable. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > A few trivial things inline. Jonathan > --- > > Change from v8: > - hest_addr_lr is now pointing to the error source size and data. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Bonus. I guess you really like this patch :) > --- > hw/acpi/ghes.c | 17 ++++++++++++++++- > include/hw/acpi/ghes.h | 1 + > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > index 3f519ccab90d..34e3364d3fd8 100644 > --- a/hw/acpi/ghes.c > +++ b/hw/acpi/ghes.c > @@ -30,6 +30,7 @@ > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > /* The max size in bytes for one error block */ > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > } > > /* > - * tell firmware to write hardware_errors GPA into > + * Tell firmware to write hardware_errors GPA into Sneaky tidy up. No problem with it in general but adding noise here, so if there are others in the series maybe gather them up in a cleanup patch. > * hardware_errors_addr fw_cfg, once the former has been initialized. > */ > bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, > @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > acpi_table_begin(&table, table_data); > > + int hest_offset = table_data->len; Local style looks to be traditional C with definitions at top. Maybe define hest_offset up a few lines and just set it here? > + > /* Error Source Count */ > build_append_int_noprefix(table_data, num_sources, 4); > for (i = 0; i < num_sources; i++) { > @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > } > > acpi_table_end(linker, &table); > + > + /* > + * tell firmware to write into GPA the address of HEST via fw_cfg, Given the tidy up above, fix this one to have a capital T, or was this where you meant to change it? > + * once initialized. > + */ > + bios_linker_loader_write_pointer(linker, > + ACPI_HEST_ADDR_FW_CFG_FILE, 0, Could wrap less and stay under 80 chars as both lines above add up to 70 something > + sizeof(uint64_t), > + ACPI_BUILD_TABLE_FILE, hest_offset); > }
Em Thu, 23 Jan 2025 10:02:17 +0000 Jonathan Cameron <Jonathan.Cameron@huawei.com> escreveu: > > --- > > > > Change from v8: > > - hest_addr_lr is now pointing to the error source size and data. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Bonus. I guess you really like this patch :) There is something wrong here with git rebase - maybe related to .git/hooks/pre-commit running checkpatch: every time I do a rebase, it adds my SOB at the end of description, if not there already. Not a problem for normal patches, but when the patch has a version history, it ends placing such duplicated SOBs. That happens even using --no-signoff and with: [format] signOff = false at git config. No idea how to fix it. Thanks, Mauro --- This is the pre-commit hook: #!/bin/sh # # TMP=$(mktemp) git diff --cached HEAD >$TMP $PWD/scripts/checkpatch.pl --no-signoff -q $TMP >&2 ERR=$? rm $TMP exit $ERR
On Thu, 23 Jan 2025 10:02:17 +0000 Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > On Wed, 22 Jan 2025 16:46:19 +0100 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > Store HEST table address at GPA, placing its content at > > hest_addr_le variable. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > A few trivial things inline. > > Jonathan > > > --- > > > > Change from v8: > > - hest_addr_lr is now pointing to the error source size and data. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Bonus. I guess you really like this patch :) > > --- > > hw/acpi/ghes.c | 17 ++++++++++++++++- > > include/hw/acpi/ghes.h | 1 + > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > > index 3f519ccab90d..34e3364d3fd8 100644 > > --- a/hw/acpi/ghes.c > > +++ b/hw/acpi/ghes.c > > @@ -30,6 +30,7 @@ > > > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > > > /* The max size in bytes for one error block */ > > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > > } > > > > /* > > - * tell firmware to write hardware_errors GPA into > > + * Tell firmware to write hardware_errors GPA into > > Sneaky tidy up. No problem with it in general but adding noise here, so if there > are others in the series maybe gather them up in a cleanup patch. +1 > > > * hardware_errors_addr fw_cfg, once the former has been initialized. > > */ > > bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, > > @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > > > acpi_table_begin(&table, table_data); > > > > + int hest_offset = table_data->len; should be unsigned, and better uint32_t but we have a zoo wrt type here all over the place. > > Local style looks to be traditional C with definitions at top. Maybe define > hest_offset up a few lines and just set it here? yep, it applies to whole QEMU (i.e. definitions only at the start of the block) > > > + > > /* Error Source Count */ > > build_append_int_noprefix(table_data, num_sources, 4); > > for (i = 0; i < num_sources; i++) { > > @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > } > > > > acpi_table_end(linker, &table); > > + > > + /* > > + * tell firmware to write into GPA the address of HEST via fw_cfg, > > Given the tidy up above, fix this one to have a capital T, or was this > where you meant to change it? > > > + * once initialized. > > + */ > > + bios_linker_loader_write_pointer(linker, > > + ACPI_HEST_ADDR_FW_CFG_FILE, 0, > > Could wrap less and stay under 80 chars as both lines above add up to 70 something > > > + sizeof(uint64_t), > > + ACPI_BUILD_TABLE_FILE, hest_offset); > > } >
Em Thu, 23 Jan 2025 10:02:17 +0000 Jonathan Cameron <Jonathan.Cameron@huawei.com> escreveu: > On Wed, 22 Jan 2025 16:46:19 +0100 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > Store HEST table address at GPA, placing its content at > > hest_addr_le variable. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > A few trivial things inline. > > Jonathan > > > --- > > > > Change from v8: > > - hest_addr_lr is now pointing to the error source size and data. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Bonus. I guess you really like this patch :) > > --- > > hw/acpi/ghes.c | 17 ++++++++++++++++- > > include/hw/acpi/ghes.h | 1 + > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > > index 3f519ccab90d..34e3364d3fd8 100644 > > --- a/hw/acpi/ghes.c > > +++ b/hw/acpi/ghes.c > > @@ -30,6 +30,7 @@ > > > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > > > /* The max size in bytes for one error block */ > > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > > } > > > > /* > > - * tell firmware to write hardware_errors GPA into > > + * Tell firmware to write hardware_errors GPA into > > Sneaky tidy up. No problem with it in general but adding noise here, so if there > are others in the series maybe gather them up in a cleanup patch. There are no other cleanups pending. Besides, as you noticed, this aligns with the comment below. So, I'm opting to add a note at the patch's description. > > > * hardware_errors_addr fw_cfg, once the former has been initialized. > > */ > > bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, > > @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > > > acpi_table_begin(&table, table_data); > > > > + int hest_offset = table_data->len; > > Local style looks to be traditional C with definitions at top. Maybe define > hest_offset up a few lines and just set it here? Ok. I'll follow Igor's suggestion of using uint32_t. > > + > > /* Error Source Count */ > > build_append_int_noprefix(table_data, num_sources, 4); > > for (i = 0; i < num_sources; i++) { > > @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > } > > > > acpi_table_end(linker, &table); > > + > > + /* > > + * tell firmware to write into GPA the address of HEST via fw_cfg, > > Given the tidy up above, fix this one to have a capital T, or was this > where you meant to change it? OK. > > + * once initialized. > > + */ > > + bios_linker_loader_write_pointer(linker, > > + ACPI_HEST_ADDR_FW_CFG_FILE, 0, > > Could wrap less and stay under 80 chars as both lines above add up to 70 something Why? This follows QEMU coding style and lines aren't longer than 80 columns. Besides, at least for my eyes and some experience doing maintainership on other projects over the years, it is a lot quicker to identify function parameters if they're properly aligned with the parenthesis. Thanks, Mauro
Em Thu, 23 Jan 2025 18:01:35 +0100 Igor Mammedov <imammedo@redhat.com> escreveu: > On Thu, 23 Jan 2025 10:02:17 +0000 > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > > On Wed, 22 Jan 2025 16:46:19 +0100 > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > > > Store HEST table address at GPA, placing its content at > > > hest_addr_le variable. > > > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > > A few trivial things inline. > > > > Jonathan > > > > > --- > > > > > > Change from v8: > > > - hest_addr_lr is now pointing to the error source size and data. > > > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Bonus. I guess you really like this patch :) > > > --- > > > hw/acpi/ghes.c | 17 ++++++++++++++++- > > > include/hw/acpi/ghes.h | 1 + > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > > > index 3f519ccab90d..34e3364d3fd8 100644 > > > --- a/hw/acpi/ghes.c > > > +++ b/hw/acpi/ghes.c > > > @@ -30,6 +30,7 @@ > > > > > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > > > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > > > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > > > > > /* The max size in bytes for one error block */ > > > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > > > } > > > > > > /* > > > - * tell firmware to write hardware_errors GPA into > > > + * Tell firmware to write hardware_errors GPA into > > > > Sneaky tidy up. No problem with it in general but adding noise here, so if there > > are others in the series maybe gather them up in a cleanup patch. > > +1 If Ok, I would prefer to keep this here, as there's no other cleanups anymore, and writing a patch just for this seems overkill. Besides, it replicates a comment with a similar content on this patch. So, instead, if OK to you, I would prefer to add a comment about it at the patch description. > > > > > * hardware_errors_addr fw_cfg, once the former has been initialized. > > > */ > > > bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, > > > @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > > > > > acpi_table_begin(&table, table_data); > > > > > > + int hest_offset = table_data->len; > should be unsigned, and better uint32_t > but we have a zoo wrt type here all over the place. Changed this one to uint32_t. > > Local style looks to be traditional C with definitions at top. Maybe define > > hest_offset up a few lines and just set it here? > > yep, it applies to whole QEMU (i.e. definitions only at the start of the block) Good to know. That's my personal style too. Yet, I guess I saw somewhere other places declaring variables in the middle of the code. > > > + > > > /* Error Source Count */ > > > build_append_int_noprefix(table_data, num_sources, 4); > > > for (i = 0; i < num_sources; i++) { > > > @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > > } > > > > > > acpi_table_end(linker, &table); > > > + > > > + /* > > > + * tell firmware to write into GPA the address of HEST via fw_cfg, > > > > Given the tidy up above, fix this one to have a capital T, or was this > > where you meant to change it? > > > > > + * once initialized. > > > + */ > > > + bios_linker_loader_write_pointer(linker, > > > + ACPI_HEST_ADDR_FW_CFG_FILE, 0, > > > > Could wrap less and stay under 80 chars as both lines above add up to 70 something > > > > > + sizeof(uint64_t), > > > + ACPI_BUILD_TABLE_FILE, hest_offset); > > > } > > > Thanks, Mauro
On Tue, 28 Jan 2025 11:00:34 +0100 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Em Thu, 23 Jan 2025 10:02:17 +0000 > Jonathan Cameron <Jonathan.Cameron@huawei.com> escreveu: > > > On Wed, 22 Jan 2025 16:46:19 +0100 > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > > > Store HEST table address at GPA, placing its content at > > > hest_addr_le variable. > > > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > > A few trivial things inline. > > > > Jonathan > > > > > --- > > > > > > Change from v8: > > > - hest_addr_lr is now pointing to the error source size and data. > > > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Bonus. I guess you really like this patch :) > > > --- > > > hw/acpi/ghes.c | 17 ++++++++++++++++- > > > include/hw/acpi/ghes.h | 1 + > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > > > index 3f519ccab90d..34e3364d3fd8 100644 > > > --- a/hw/acpi/ghes.c > > > +++ b/hw/acpi/ghes.c > > > @@ -30,6 +30,7 @@ > > > > > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > > > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > > > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > > > > > /* The max size in bytes for one error block */ > > > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > > > } > > > > > > /* > > > - * tell firmware to write hardware_errors GPA into > > > + * Tell firmware to write hardware_errors GPA into > > > > Sneaky tidy up. No problem with it in general but adding noise here, so if there > > are others in the series maybe gather them up in a cleanup patch. > > There are no other cleanups pending. Besides, as you noticed, this > aligns with the comment below. So, I'm opting to add a note at the > patch's description. ok. > > > > + * once initialized. > > > + */ > > > + bios_linker_loader_write_pointer(linker, > > > + ACPI_HEST_ADDR_FW_CFG_FILE, 0, > > > > Could wrap less and stay under 80 chars as both lines above add up to 70 something > > Why? This follows QEMU coding style and lines aren't longer than 80 > columns. Besides, at least for my eyes and some experience doing maintainership > on other projects over the years, it is a lot quicker to identify function > parameters if they're properly aligned with the parenthesis. Ah. I didn't state this clearly enough. bios_linker_loader_write_pointer(linker, ACPI_HEST_ADDR_FW_CFG_FILE, 0, is also under 80 chars. > > Thanks, > Mauro
On Wed, 22 Jan 2025 16:46:19 +0100 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Store HEST table address at GPA, placing its content at > hest_addr_le variable. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > --- > > Change from v8: > - hest_addr_lr is now pointing to the error source size and data. that's confusing, variable name say it's HEST table address, while in practice it's (that + offset). I'd very much prefer it being table start and then you'd add offset later on where it's going to be used. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/acpi/ghes.c | 17 ++++++++++++++++- > include/hw/acpi/ghes.h | 1 + > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > index 3f519ccab90d..34e3364d3fd8 100644 > --- a/hw/acpi/ghes.c > +++ b/hw/acpi/ghes.c > @@ -30,6 +30,7 @@ > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > /* The max size in bytes for one error block */ > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > } > > /* > - * tell firmware to write hardware_errors GPA into > + * Tell firmware to write hardware_errors GPA into drop this hunk as it's not related to the patch > * hardware_errors_addr fw_cfg, once the former has been initialized. > */ > bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, > @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > acpi_table_begin(&table, table_data); > > + int hest_offset = table_data->len; > + > /* Error Source Count */ > build_append_int_noprefix(table_data, num_sources, 4); > for (i = 0; i < num_sources; i++) { > @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > } > > acpi_table_end(linker, &table); > + > + /* > + * 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); > } > > void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s, > @@ -375,6 +387,9 @@ 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); > + > ags->present = true; > } > > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h > index 9f0120d0d596..237721fec0a2 100644 > --- a/include/hw/acpi/ghes.h > +++ b/include/hw/acpi/ghes.h > @@ -58,6 +58,7 @@ enum AcpiGhesNotifyType { > }; > > 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 */ > } AcpiGhesState;
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c index 3f519ccab90d..34e3364d3fd8 100644 --- a/hw/acpi/ghes.c +++ b/hw/acpi/ghes.c @@ -30,6 +30,7 @@ #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" /* The max size in bytes for one error block */ #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, } /* - * tell firmware to write hardware_errors GPA into + * Tell firmware to write hardware_errors GPA into * hardware_errors_addr fw_cfg, once the former has been initialized. */ bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, acpi_table_begin(&table, table_data); + int hest_offset = table_data->len; + /* Error Source Count */ build_append_int_noprefix(table_data, num_sources, 4); for (i = 0; i < num_sources; i++) { @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, } acpi_table_end(linker, &table); + + /* + * 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); } void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s, @@ -375,6 +387,9 @@ 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); + ags->present = true; } diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h index 9f0120d0d596..237721fec0a2 100644 --- a/include/hw/acpi/ghes.h +++ b/include/hw/acpi/ghes.h @@ -58,6 +58,7 @@ enum AcpiGhesNotifyType { }; 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 */ } AcpiGhesState;