Re: [PATCH v4] ACPI / APEI: Boot Error Record Table processing was needlessly complicated

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 15, 2017 at 03:41:21PM -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").

Hmm, ok, so do we know whether some fw writers "misunderstood" the
previous version of the spec and actually added that separate table and
now we need to handle the different layout?

Probably not but I'm not going to be surprised someone did. At all.

> 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()".
> 
> Also this driver clears the "block_status" status field of the error
> status block in ACPI memory. I can see no justification for doing this
> in the ACPI spec.  It is also highly kernel-ego-centric.  There are
> user mode BERT harvesting tools that dig into /sys/firmware/acpi/tables/BERT
> which might like to see this summary value.
> 
> Drop the:
> 	estatus->block_status = 0;

This whole treatise of the clearing of block_status should most likely
be a separate patch, though. As it is clearly a fix. Probably make it
the first patch and the second one removing bert_print_all().

Something like that.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
--
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



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux