ACPI: Add APEI GHES table generation support
Quoted 8 lines
Quoted 2 lines
+ */
+static voidacpi_ghes_build_notify(GArray *table, const uint8_t type)typically format should be
build_WHAT(), so
build_ghes_hw_error_notification()And I'd move this out into its own patch.
this applies to other trivial in-depended sub-tables, that take all data needed to construct them from supplied arguments.
I very used your suggested method in previous series[1], but other maintainer suggested to move this function to this file, because he think only GHES used it
[1]:
https://patchwork.ozlabs.org/cover/1099428/
Quoted 28 lines
Quoted 14 lines
(quoted diff) +13
-0+{ + /* Type */ + build_append_int_noprefix(table, type, 1); + /* + * Length: + * Total length of the structure in bytes + */ + build_append_int_noprefix(table, 28, 1); + /* Configuration Write Enable */ + build_append_int_noprefix(table, 0, 2); + /* Poll Interval */ + build_append_int_noprefix(table, 0, 4); + /* Vector */Review
Quoted 10 lines
(quoted diff) +10
-0+ build_append_int_noprefix(table, 0, 4); + /* Switch To Polling Threshold Value */ + build_append_int_noprefix(table, 0, 4); + /* Switch To Polling Threshold Window */ + build_append_int_noprefix(table, 0, 4); + /* Error Threshold Value */ + build_append_int_noprefix(table, 0, 4); + /* Error Threshold Window */ + build_append_int_noprefix(table, 0, 4); } +/*
Initialize "etc/hardware_errors" and "etc/hardware_errors_addr" fwcfg blobs. Seedocs/specs/acpi_hest_ghes.rstfor blobs format */
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+void acpi_ghes_build_error_table(GArray *hardware_errors, BIOSLinker +*linker)
build_ghes_error_table()also I'd move this function into its own patch along with other related code that initializes and wires it into virt board.
I ever use your suggested method[1], but other maintainer, it seems Michael, suggested to move these functions to this file that used it, because he think only GHES used it.
[1]:
https://patchwork.ozlabs.org/patch/1099424/
https://patchwork.ozlabs.org/patch/1099425/
https://patchwork.ozlabs.org/patch/1099430/
Quoted 10 lines · On Fri, 15 Nov 2019 14:32:47 +0000 gengdongjiu <[email protected]> wrote:
Quoted 8 lines
Quoted 2 lines
+ */
+static voidacpi_ghes_build_notify(GArray *table, const uint8_t type)typically format should be
build_WHAT(), so
build_ghes_hw_error_notification()And I'd move this out into its own patch.
this applies to other trivial in-depended sub-tables, that take all data needed to construct them from supplied arguments.I very used your suggested method in previous series[1], but other maintainer suggested to move this function to this file, because he think only GHES used it
Using this file is ok, what I've meant for you to split function from this patch into a separate smaller patch.
With the way code split now I have to review 2 big complex patches at the same which makes reviewing hard and I have a hard time convincing myself that code it correct.
Moving small self-contained chunks of code in to separate smaller patches makes review easier.
Quoted 46 lines
[1]:
https://patchwork.ozlabs.org/cover/1099428/Quoted 25 lines
Quoted 14 lines
(quoted diff) +13
-0+{ + /* Type */ + build_append_int_noprefix(table, type, 1); + /* + * Length: + * Total length of the structure in bytes + */ + build_append_int_noprefix(table, 28, 1); + /* Configuration Write Enable */ + build_append_int_noprefix(table, 0, 2); + /* Poll Interval */ + build_append_int_noprefix(table, 0, 4); + /* Vector */Review
Quoted 10 lines
(quoted diff) +10
-0+ build_append_int_noprefix(table, 0, 4); + /* Switch To Polling Threshold Value */ + build_append_int_noprefix(table, 0, 4); + /* Switch To Polling Threshold Window */ + build_append_int_noprefix(table, 0, 4); + /* Error Threshold Value */ + build_append_int_noprefix(table, 0, 4); + /* Error Threshold Window */ + build_append_int_noprefix(table, 0, 4); } +Quoted 8 lines
/*
Initialize "etc/hardware_errors" and "etc/hardware_errors_addr" fwcfg blobs. Seedocs/specs/acpi_hest_ghes.rstfor blobs format */Review
Quoted 2 lines
(quoted diff) +2
-0+void acpi_ghes_build_error_table(GArray *hardware_errors, BIOSLinker +*linker)
build_ghes_error_table()also I'd move this function into its own patch along with other related code that initializes and wires it into virt board.
I ever use your suggested method[1], but other maintainer, it seems Michael, suggested to move these functions to this file that used it, because he think only GHES used it.
[1]:
https://patchwork.ozlabs.org/patch/1099424/
https://patchwork.ozlabs.org/patch/1099425/
https://patchwork.ozlabs.org/patch/1099430/