On Fri, May 05, 2017 at 11:10:37AM -0700, Luck, Tony wrote:
> From: Tony Luck <tony.luck@xxxxxxxxx>
>
> Quoting version 6.1 of the ACPI specification. Section 18.3.1 "Boot
> Error Source" says:
>
> The Boot Error Region is a range of addressable memory OSPM can access
> during initialization to determine if an unhandled error condition
> occurred. System firmware must report this memory range as firmware
> reserved. The format of the Boot Error Region follow that of an Error
> Status Block, this is defined in Section 18.3.2.7. The format of the
> error status block is described by Table 18-342.
>
> This clarifies some points that were obfuscated in earlier versions.
> E.g. there is no longer a separate table to describe the format of the
> "Boot Error Region" (which was identical to the "Error Status Block").
> Also saying "follow that of *an* Error Status Block" makes it clear that
> there is just one block (which can still contain multiple "Generic Error
> Data Entry structures").
>
> The loop inside bert_print_all() is unnecessary (but probably harmless
> as the "while (remain > sizeof(struct acpi_bert_region))" loop should
> terminate after we skipped over the first entry.
>
> We can drop the "bert_print_all()" function and just move the four
> relevant lines inline in "bert_init()". Since there are no remaining
> users of "struct acpi_bert_region" we delete it from <acpi/actbl1.h>
>
> Cc: Len Brown <lenb@xxxxxxxxxx>
> Cc: Huang Ying <ying.huang@xxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: Tomasz Nowicki <tomasz.nowicki@xxxxxxxxxx>
> Cc: Jonathan (Zhixiong) Zhang <zjzhang@xxxxxxxxxxxxxx>
> Cc: Tyler Baicar <tbaicar@xxxxxxxxxxxxxx>
> Cc: linux-acpi@xxxxxxxxxxxxxxx
> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> ---
> drivers/acpi/apei/bert.c | 54 +++++++-----------------------------------------
> include/acpi/actbl1.h | 11 +---------
> 2 files changed, 8 insertions(+), 57 deletions(-)
...
> -struct acpi_bert_region {
> - u32 block_status; /* Type of error information */
> - u32 raw_data_offset; /* Offset to raw error data */
> - u32 raw_data_length; /* Length of raw error data */
> - u32 data_length; /* Length of generic error data */
> - u32 error_severity; /* Severity code */
> + /* which is a struct acpi_hest_generic_status */
Just this nitpick: please merge this comment with the one above it into
a single:
u64 address; /*
* Physical address of the error region which is a Generic
* Error Status Block (struct acpi_hest_generic_status).
*/
I would like to advocate to use the actual names from the spec too
because those are so many and so dumbly named, so that one's head
starts spinning pretty quickly, trying to keep 'em apart.
Other than that,
Reviewed-by: Borislav Petkov <bp@xxxxxxx>
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
