Re: [PATCH] efistub/tpm: Use ACPI reclaim memory for event log to avoid corruption

[Gregory Price <gourry@gourry.net>]

On Wed, Oct 30, 2024 at 08:43:01PM +0100, Ard Biesheuvel wrote:
> On Wed, 30 Oct 2024 at 19:26, Gregory Price <gourry@gourry.net> wrote:
> >
> > On Wed, Oct 30, 2024 at 05:13:14PM +0000, Usama Arif wrote:
> > >
> > >
> > > On 30/10/2024 05:25, Jiri Slaby wrote:
> > > > On 25. 10. 24, 15:27, Usama Arif wrote:
> > > >> Could you share the e820 map, reserve setup_data and TPMEventLog address with and without the patch?
> > > >> All of these should be just be in the dmesg.
> > > > efi: EFI v2.6 by American Megatrends
> >
> > Tossing in another observation - the AMI EFI we've been working with has been
> >
> > EFI v2.8 by American Megatrends
> > or
> > EFI v2.9 by American Megatrends
> >
> > We have not seen this particular behavior (cold boot corruption issues) on top
> > of these version.  Might be worth investigating this issue.
> >
> > you may also want to investigate this patch set:
> >
> > https://lore.kernel.org/all/20240913231954.20081-1-gourry@gourry.net/
> >
> > which I believe would have caught your "eat all memory" sign extention issue.
> >
> > This is queued up for v6.13 i think - but possibly 1/4 deserves a stable mark.
> >
> 
> To me, it does not seem obvious at all that the TPM code is the
> culprit here. The firmware produces a corrupted memory attributes
> table now that the EFI stub uses ACPI reclaim memory for the TPM event
> log, but to me, it smells like a firmware issue, not an issue in the
> EFI stub. (Pool allocations can trigger page allocations, affecting
> the layout of the EFI memory map).
> 
> So let's keep an open mind here, and not stare ourselves blind on the
> TPM event log code.

Agreed, just working backward a bit - this definitely feels like something
is wrong in the state of Denm... firmware.

~Gregory