Re: [edk2-devel] TPM2 EventLog EFI vs. ACPI

["Jason Andryuk" <jandryuk@gmail.com>]

On Mon, Sep 19, 2022 at 1:39 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>
>
> On 9/19/22 12:55, Jason Andryuk wrote:
> > Hi, Stefan,
> >
> > On Mon, Sep 19, 2022 at 8:22 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
> >>
> >>
> >>
> >> On 9/19/22 05:17, Igor Mammedov wrote:
> >>> On Fri, 16 Sep 2022 15:45:38 -0400
> >>> "Jason Andryuk" <jandryuk@gmail.com> wrote:
> >>>
> >>> CCing Stefan as he is probably the best person to talk about qemu
> >>> impl. of TPM
> >>>
> >>>> Hi,
> >>>>
> >>>> I've noticed an issue with the TPM2 EventLog.  OVMF exposes the TPM
> >>>> Event Log via EFI and ACPI, but they have different addresses.  The
> >>>> EFI one retrievable by GetEventLog() is populated.  The ACPI is empty.
> >>
> >> The ACPI one is for SeaBIOS.
> >
> > Yes, ACPI is the only option for SeaBIOS.  Still, I expect GetEventLog
> > and the ACPI table to point at the same location.
> >
> >>>> Oh, there are actually two EFI Event Logs for the two formats:
> >>>> EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2
> >>>> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2
> >>>>
> >>>> The debug log from the Fedora 36 OVMF shows:
> >>>> Tcg2GetEventLog (EventLogLocation - 7EEB2000)
> >>>> which matches the address retrieved with GetEventLog().
> >>>> And hexdump-ing the TPM2 ACPI table shows 0x7fbe6000.
> >>>>
> >>>> On a different build, I added output for both EFI logs, and the addresses are:
> >>>> 0x7ec3d000 - EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2
> >>>> 0x7ec1b000 - EFI_TCG2_EVENT_LOG_FORMAT_TCG_2
> >>
> >> I am also not familiar with the origin of the EDK2 code as to why it was
> >> done this way. Maybe typical builds for EDK2 don't include TPM 1.2 and
> >> TPM 2 and OVMF is an outlier here...
> >
> > The two log formats are both in the TPM 2 code, so it's independent
> > from including TPM 1.2 and 2 hardware support.
> >
> >>>> 0x7fbe6000 - ACPI
> >>>>
> >>>> The ACPI one is a little more user friendly as its address is
> >>>> available through the table during runtime.  The EFI addresses can
> >>>> only be grabbed before exiting boot services.
> >>>>
> >>>> I think the issue is that the ACPI tables are created from Qemu fw_cfg
> >>>> data, which allocates memory for the log and places the address in
> >>>> ACPI tables.  Meanwhile,
> >>
> >> That's because of SeaBIOS iirc.
> >
> > I looked at SeaBIOS, and it finds the address in the ACPI TPM2 table
> > and uses it as its log area.
> >
> >>>> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c:SetupEventLog() allocates its own
> >>>> event log memory.  SetupEventLog() saves the size and address in
> >>>> PcdTpm2AcpiTableLaml & PcdTpm2AcpiTableLasa, but nothing puts those
> >>>> values in the actual ACPI tables.
> >>>>
> >>>> It seems like SetupEventLog would be better structured to check
> >>>> existing ACPI tables and look for a log in a TPM2 section.  If found,
> >>>> use that, otherwise create a new log area.
> >>>>
> >>>> The other wrinkle is that the Tcg2 code is keeping two event logs in
> >>>> the two formats.  It seems to me that for TPM2, it would be easier to
> >>
> >> Does it log everything twice?
> >
> > Yes, it logs everything twice.  The sha1 hashes match between the two
> > logs.  For the newer format, it generates sha1, sha256, sha384 &
> > sha512 digests for each entry.
> >
> > So there are 3 ~64k memory regions set aside from logs.  OVMF code
> > populates the 2 EFI ones.  Linux only grabs the EFI logs when entered
> > via EFI stub - a direct UEFI load of the kernel.  Booting via grub-efi
> > doesn't grab the EFI log addresses, so only the empty ACPI entry is
> > discovered.  Being empty, Linux doesn't expost a TPM event log through
> > sysfs.
> >
> > I tried searching for the TPM2 table in SetupEventLog(), but it wasn't
> > found.  SetupEventLog() runs before InstallQemuFwCfgTables(), which
> > makes sense given the existing code to supply the log addresses to
> > Tpm2Acpi.  OVMF has already logged things into the event log by the
> > time InstallQemuFwCfgTables() is called.
> >
> > Thanks for taking a look../SecurityPkg/Tcg/TcgDxe/TcgDxe.c:707:SetupEventLog (
>
>
> I did take a look and it surprises me that we have 2 logs for TPM 1.2
> and TPM 2 each plus the ACPI one. There are setup functions for TPM 1.2
> and TPM 2 each:
>
> ./SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c:1546:SetupEventLog
> ./SecurityPkg/Tcg/TcgDxe/TcgDxe.c:707:SetupEventLog (
>
> Though only one of them should initialize the log because calls to them
> are gated by DriverEntry checking which TPM version is in use (also my
> logs seems to say this that only TPM 2's setup is done).
>
>    Status = Tpm2RequestUseTpm ();
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "TPM2 not detected!\n"));
>      return Status;
>    }
>
>
>    Status = Tpm12RequestUseTpm ();
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "TPM not detected!\n"));
>      return Status;
>    }

Yes, TPM 1.2 and 2.0 duplicate log setup, but only one of them should run.

> I have tried to skip over the allocation of memory based on the ability
> to find the TPM2 table in the SetupEventLog function for TPM 2:
>
>    tpm2 = EfiLocateFirstAcpiTable (SIGNATURE_32('T', 'P, 'M', '2'));
>
> It doesn't find the table at this point. So maybe that's the wrong
> function to call?

I tried that too, but it's only later that InstallQemuFwCfgTables()
runs and installs the ACPI tables.

> Another idea may be to search for the TPM2 table at the end and copy the
> log into the ACPI log area allocated by QEMU, if there is one (with QEMU
> there will be one), and use that address then also for the TPM2 log and
> free the UEFI log area that currently seems to be a duplicate. I don't
> know where that should be done, though.

I'm trying to modify Process2ndPassCmdAddPointer() to see if I can
update the ACPI table with the SetupEventLog memory.  Just to see if
something works.  I'm having issues with Pcd stuff.

> I always run into the following issue with EDK2 these days...
>
>
> Reserved variable store memory: 0x7FE7C000; size: 528kb
> NvVarStore Variable header State was invalid.
> ASSERT
> /home/stefanb/dev/edk2/OvmfPkg/Library/PlatformInitLib/Platform.c(807):
> ((BOOLEAN)(0==1))

I've been building 101f4c789221716585b972f2c2a22a85c078ef1d from April
since I hadn't updated head.

Regards,
Jason