Re: [Qemu-devel] [PATCH v2] Add ACPI tables for TPM
From:
Stefan Berger
Subject:
Re: [Qemu-devel] [PATCH v2] Add ACPI tables for TPM
Date:
Wed, 30 Jul 2014 12:03:46 -0400
Laszlo Ersek <address@hidden> wrote on 07/30/2014
11:58:52 AM:
> From: Laszlo Ersek <address@hidden> > To: Stefan Berger/Watson/address@hidden > Cc: "Michael S. Tsirkin" <address@hidden>,
address@hidden,
> Stefan Berger <address@hidden> > Date: 07/30/2014 11:59 AM > Subject: Re: [PATCH v2] Add ACPI tables for TPM >
> On 07/30/14 17:44, Stefan Berger wrote:
> > Laszlo Ersek <address@hidden> wrote on 07/30/2014 11:41:10
AM:
> >
> >> From: Laszlo Ersek <address@hidden>
> >> To: Stefan Berger/Watson/address@hidden, "Michael S. Tsirkin"
<address@hidden>
> >> Cc: address@hidden, Stefan Berger <address@hidden>
> >> Date: 07/30/2014 11:41 AM
> >> Subject: Re: [PATCH v2] Add ACPI tables for TPM
> >>
> >> On 07/30/14 17:29, Stefan Berger wrote:
> >> > "Michael S. Tsirkin" <address@hidden>
wrote on 07/30/2014 11:20:41 AM:
> >> >
> >> >> From: "Michael S. Tsirkin" <address@hidden>
> >> >> To: Stefan Berger/Watson/address@hidden
> >> >> Cc: Laszlo Ersek <address@hidden>, address@hidden,
Stefan
> >> >> Berger <address@hidden>
> >> >> Date: 07/30/2014 11:20 AM
> >> >> Subject: Re: [PATCH v2] Add ACPI tables for TPM
> >> >>
> >> >> On Wed, Jul 30, 2014 at 11:10:27AM -0400, Stefan
Berger wrote:
> >> >> > Laszlo Ersek <address@hidden> wrote
on 07/30/2014 10:36:38 AM:
> >> >> >
> >> >> > > From: Laszlo Ersek <address@hidden>
> >> >> > > To: "Michael S. Tsirkin" <address@hidden>,
Stefan
> >> > Berger/Watson/address@hidden
> >> >> > > Cc: address@hidden, Stefan Berger
> > <address@hidden>
> >> >> > > Date: 07/30/2014 10:36 AM
> >> >> > > Subject: Re: [PATCH v2] Add ACPI tables
for TPM
> >> >> > >
> >> >> > > On 07/30/14 15:20, Michael S. Tsirkin
wrote:
> >> >> > > > On Tue, Jul 29, 2014 at 06:52:19AM
-0400, Stefan Berger wrote:
> >> >> > > >> From: Stefan Berger <address@hidden>
> >> >> > > >>
> >> >> > > >> Add an SSDT ACPI table for the
TPM device.
> >> >> > > >> Add a TCPA table for BIOS logging
area when a TPM is being used.
> >> >> > > >>
> >> >> > > >> The latter follows this spec
here:
> >> >> > > >>
> >> >> > > >> http://www.trustedcomputinggroup.org/files/static_page_files/
> >> >> > > DCD4188E-1A4B-B294-D050A155FB6F7385/
> >> >> > > TCG_ACPIGeneralSpecification_PublicReview.pdf
> >> >> > >
> >> >> > > (Thanks for CC'ing me, Michael.)
> >> >> > >
> >> >> > > I skimmed this spec.
> >> >> > >
> >> >> > > >> +static void
> >> >> > > >> +build_tpm_tcpa(GArray *table_data,
GArray *linker)
> >> >> > > >> +{
> >> >> > > >> + Acpi20Tcpa *tcpa;
> >> >> > > >> + uint32_t log_area_minimum_length
=
> > TPM_LOG_AREA_MINIMUM_SIZE;
> >> >> > > >> + uint64_t log_area_start_address;
> >> >> > > >> + size_t len = log_area_minimum_length
+ sizeof(*tcpa);
> >> >> > > >> +
> >> >> > > >> + log_area_start_address
= table_data->len + sizeof(*tcpa);
> >> >> > > >> +
> >> >> > > >> + tcpa = acpi_data_push(table_data,
len);
> >> >> > > >> +
> >> >> > > >> + tcpa->platform_class
=
> >> > cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
> >> >> > > >> + tcpa->log_area_minimum_length
= cpu_to_le32
> >> >> (log_area_minimum_length);
> >> >> > > >> + tcpa->log_area_start_address
= cpu_to_le64
> >> >> (log_area_start_address);
> >> >> > > >> +
> >> >> > > >> + /* LASA address
to be filled by Guest linker */
> >> >> > > >
> >> >> > > > Hmm, you are simply allocating log
area as part of the ACPI
> >> > table. It
> >> >> > > > works because bios happens to allocate
tables from high memory.
> >> >> > > > But I think this is a problem in
practice because
> >> >> > > > bios is allowed to allocate acpi
memory differently.
> >> >> > > > On the other hand log presumably
needs to reside in
> >> >> > > > physical memory somewhere.
> >> >> > > >
> >> >> > > > If you need bios to allocate this
memory, then we will
> >> >> > > > need a new allocation type for this,
add it to linker
> >> >> > > > in bios and qemu.
> >> >> > > >
> >> >> > > > Alternatively, find some other way
to get hold of
> >> >> > > > physical memory.
> >> >> > > > Is there a way to disable the log
completely?
> >> >> > > > As defined in your patch, I doubt
there's anything there, ever ..
> >> >> > > >
> >> >> > > >
> >> >> > > >
> >> >> > > >> + bios_linker_loader_add_pointer(linker,
> > ACPI_BUILD_TABLE_FILE,
> >> >> > > >> +
ACPI_BUILD_TABLE_FILE,
> >> >> > > >> +
table_data,
> >> >> > > &tcpa->log_area_start_address,
> >> >> > > >> +
sizeof
> >> >> (tcpa->log_area_start_address));
> >> >> > > >> + build_header(linker,
table_data,
> >> >> > > >> +
(void *)tcpa, "TCPA", sizeof(*tcpa),
2);
> >> >> > > >> +}
> >> >> > >
> >> >> > > So here's my understanding. The spec referenced
above describes
> > three
> >> >> > > ACPI tables: two (client vs. server) for
TPM 1.2, and a third one
> >> >> > > (usable by both client & server platforms)
for TPM 2.0.
> >> >> > >
> >> >> > > The code above prepares a TPM 1.2 table.
(Signature: "TCPA".)
> >> >> > >
> >> >> > > This table has a field called LASA (Log
Area Start Address) which
> >> > points
> >> >> > > to somewhere in (guest-)physical memory.
The patch adds a "dummy
> >> > range"
> >> >> > > to the end of the TCPA table itself, and
asks the linker to set
> >> > LASA to
> >> >> > > the beginning of that range.
> >> >> > >
> >> >> > > This won't work in OVMF, and not just
because of the reason that
> >> > Michael
> >> >> > > mentions (ie. because the firmware, in
particular SeaBIOS, might
> >> >> > > allocate the TCPA table in an area that
is unsuitable as LASA
> > target).
> >> >> > >
> >> >> > > Rather, in OVMF this won't work because
OVMF doesn't implement the
> >> >> > > linking part of the linker. The *generic*
edk2 protocol
> >> >> > > (EFI_ACPI_TABLE_PROTOCOL, which is coded
outside of OVMF) that
> >> > OVMF uses
> >> >> > > (as a client) to install ACPI tables in
guest-phys memory requires
> >> >> > > tables to be passed in one-by-one.
> >> >> > >
> >> >> > > The EFI_ACPI_TABLE_PROTOCOL implementation
in edk2 handles *some*
> >> >> > > well-known tables specially. It has knowledge
of their internal
> >> >> > > pointers, and when you install an ACPI
table,
> > EFI_ACPI_TABLE_PROTOCOL
> >> >> > > updates pointers automatically. (For example
when you install the
> >> > FACS,
> >> >> > > the protocol links it automatically into
FACP.)
> >> >> > >
> >> >> > > The EFI_ACPI_TABLE_PROTOCOL implementation
in edk2 doesn't seem to
> >> > know
> >> >> > > anything about the TCPA table, let alone
the unstructured (?) TCG
> >> > event
> >> >> > > log that is pointed-to by TCPA.LASA.
> >> >> > >
> >> >> > > (I grepped for the TCPA signature,
> >> >> > >
> >> >> >
> >> >>
> >> >
> >>
> >
> EFI_ACPI_5_0_TRUSTED_COMPUTING_PLATFORM_ALLIANCE_CAPABILITIES_TABLE_SIGNATURE.)
> >> >> > >
> >> >> > > This means that if you pass down a TCPA
table, OVMF will install it
> >> >> > > right now, but TCPA.LASA will be bogus.
> >> >> > >
> >> >> > > If I wanted to implement the complete
linker as Michael
> > envisioned it,
> >> >> > > then I'd have to avoid edk2's EFI_ACPI_TABLE_PROTOCOL,
and
> > implement
> >> >> > > ACPI table installation from zero, trying
to mimic the SeaBIOS
> > client
> >> >> > > code, but in a way that matches the UEFI
environment. I'm not
> > ready to
> >> >> > > do that. Definitely not without an "official"
human-language
> >> >> > > specification of the linker-loader interface.
> >> >> > >
> >> >> > > I skimmed the patch but I'm not sure what
exactly the TPM
> > emulation in
> >> >> > > qemu depends on. Is it a command line
option? Is it default for
> > some
> >> >> > > machine types?
> >> >> > >
> >> >> > > Alternatively, I could recognize the TCPA
signature in OVMF when
> >> > parsing
> >> >> > > the ACPI blobs for table headers, and
filter it out.
> >> >> >
> >> >> > This is the code for what I would call 'pointer
relocation'. The
> >> >> TCPA table is
> >> >> > not the only place where this is used, but
why is it an issue
> >> >> there while not
> >> >> > with the following?
> >> >> >
> >> >> > fadt->firmware_ctrl = cpu_to_le32(facs);
> >> >> > /* FACS address to be filled
by Guest linker */
> >> >> > bios_linker_loader_add_pointer(linker,
ACPI_BUILD_TABLE_FILE,
> >> >> >
ACPI_BUILD_TABLE_FILE,
> >> >> >
table_data,
&fadt->firmware_ctrl,
> >> >> >
sizeof
fadt->firmware_ctrl);
> >> >> >
> >> >> > Regards,
> >> >> > Stefan
> >> >>
> >> >>
> >> >> Becase FACS is an ACPI table. So BIOS allocates
it
> >> >> from E820_RESERVED at the moment but it does not
have to,
> >> >> it could mark it with E820_ACPI.
> >> >> Guest can then interpret the tables and then release
the
> >> >> memory if it wishes.
> >> >>
> >> >> If you want to do it for TCPA you must tell bios
that
> >> >> this is not ACPI memory.
> >> >
> >> > I see. Presumably the whole slew of FADT, FACS, RSDP,
& RSDT would need
> >> > a similar tag to keep the S3 resume vector around?
> >>
> >> Not in OVMF, because edk2's EFI_ACPI_TABLE_PROTOCOL special
cases FACS
> >> (containing the S3 resume vector), allocating it in EfiACPIMemoryNVS
> > memory.
> >>
> >> Table 26. Memory Type Usage after ExitBootServices()
> >> EfiACPIMemoryNVS: This memory is to be preserved by the loader
and OS
> >>
in the working and ACPI S1–S3 states.
> >>
> >
> > So what is a solution then for OVMF? Add another special case
for TCPA?
> > Is this counter to the specs ? Skip TCPA?
>
> In the short term, probably skip TCPA, or advise users in documentation
> not to enable the TPM device when running OVMF.
I guess we can do that. You can skip the TCPA for
now; once the UEFI has TPM support, this table would then be needed.