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

[Usama Arif <usamaarif642@gmail.com>]

On 30/10/2024 18:02, Gregory Price 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.
>>>
>>> It's shared in the aforementioned bug [1] already.
>>>
>>> 6.11.2 dmesg (bad run):
>>> https://bugzilla.suse.com/attachment.cgi?id=877874
>>>
>>> 6.12-rc2 dmesg (good run):
>>> https://bugzilla.suse.com/attachment.cgi?id=877887
>>>
>>> FWIW from https://bugzilla.suse.com/attachment.cgi?id=878051:
>>> good TPMEventLog=0x682aa018
>>> bad  TPMEventLog=0x65a6b018
>>>
>>> [1] https://bugzilla.suse.com/show_bug.cgi?id=1231465
>>>
> ... snip ...
>>> efi: EFI v2.6 by American Megatrends
>>> efi: ACPI=0x7a255000 ACPI 2.0=0x7a255000 SMBIOS=0x7b140000 SMBIOS 3.0=0x7b13f000 TPMFinalLog=0x7a892000 ESRT=0x7b0deb18 [-MEMATTR=0x77535018-] {+MEMATTR=0x77526018+} MOKvar=0x7b13e000 RNG=0x7a254018 [-TPMEventLog=0x65a6f018-] {+TPMEventLog=0x682ac018+}
>>>
>>>
>>> thanks,
>>
>> Thanks for sharing this.
>>
>> This looks a bit weird for me.
>>
>> The issue this patch was trying to fix was TPMEventLog being overwritten during kexec.
>> We are using efi libstub.
>> Without this patch we would see
>> BIOS-e820: [mem 0x0000000000100000-0x0000000064763fff] usable 
>> TPMEventLog=0x5ed47018
>> i.e. TPMEventLog was usable memory and therefore was prone to corruption during kexec.
>>
>> With this patch 
>> BIOS-e820: [mem 0x00000000a8c01000-0x00000000a8cebfff] ACPI data
>> TPMEventLog=0xa8ca8018 
>> i.e.  TPMEventLog is reserved as ACPI data, hence cant be corrupted during kexec.
>>
>>
>> In your case, from the logs you shared, good run without the patch:
>> [    0.000000] [      T0] BIOS-e820: [mem 0x0000000065a6f000-0x0000000065a7dfff] ACPI data
>> [    0.000000] [      T0] BIOS-e820: [mem 0x0000000065a7e000-0x000000006a5acfff] usable
>> [    0.000000] [      T0] BIOS-e820: [mem 0x000000006a5ad000-0x000000006a5adfff] ACPI NVS
>> TPMEventLog=0x65a6f018 
>> bad run with the patch:
>> [    0.000000] [      T0] BIOS-e820: [mem 0x00000000682ac000-0x00000000682bafff] ACPI data
>> [    0.000000] [      T0] BIOS-e820: [mem 0x00000000682bb000-0x000000006a5acfff] usable
>> [    0.000000] [      T0] BIOS-e820: [mem 0x000000006a5ad000-0x000000006a5adfff] ACPI NVS
>> TPMEventLog=0x682ac018
>> Both with and without the fix, the TPMEventLog is part of ACPI data.
>>
> 
> Just wondering - why would the TPM log move a total of ~40MB between COLD boots.
> 
> I would expect this location to be relatively fixed (give or take a small amount of
> memory) - especially since it's so early in boot.
> 

This is a good point. I think if we suspect this patch, it would be good to compare
dmesg of kernel cold boot with this patch vs the commit just before this patch, rather
than comparing with 6.12.

Thanks

>> It means your firmware has already marked that area as ACPI data. Are you using efi/libstub?
>>
>> Thanks,
>> Usama
>>
>>
>>
>>