Laszlo Ersek <ler...@redhat.com> wrote on 07/30/2014 12:07:28 PM: > From: Laszlo Ersek <ler...@redhat.com> > To: "Michael S. Tsirkin" <m...@redhat.com> > Cc: Stefan Berger/Watson/IBM@IBMUS, qemu-devel@nongnu.org, Stefan > Berger <stef...@linux.vnet.ibm.com> > Date: 07/30/2014 12:07 PM > Subject: Re: [PATCH v2] Add ACPI tables for TPM > > On 07/30/14 17:52, Michael S. Tsirkin wrote: > > On Wed, Jul 30, 2014 at 05:37:26PM +0200, Laszlo Ersek wrote: > >> On 07/30/14 17:10, Stefan Berger wrote: > >>> Laszlo Ersek <ler...@redhat.com> wrote on 07/30/2014 10:36:38 AM: > >>> > >>>> From: Laszlo Ersek <ler...@redhat.com> > >>>> To: "Michael S. Tsirkin" <m...@redhat.com>, Stefan Berger/Watson/IBM@IBMUS > >>>> Cc: qemu-devel@nongnu.org, Stefan Berger <stef...@linux.vnet.ibm.com> > >>>> 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 <stef...@linux.vnet.ibm.com> > >>>>>> > >>>>>> 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); > >> > >> (Responding wrt. to OVMF:) because EFI_ACPI_TABLE_PROTOCOL special cases > >> the installation of FACS. This is visible both in the UEFI > >> specification, and in the edk2 implementation. > >> > >> That is, OVMF can avoid relocating the "fadt->firmware_ctrl" pointer, > >> because edk2's EFI_ACPI_TABLE_PROTOCOL will do that automatically, when > >> OVMF requests the installation of FACS. > >> > >> No such special case exists for the installation of the TCG event log > >> area (which is by itself not even an ACPI table as I gather). *If* the > >> TCG event log were an ACPI table in its own right, *and* edk2's > >> EFI_ACPI_TABLE_PROTOCOL implementation had a special case for it, then > >> when OVMF installed the event log, EFI_ACPI_TABLE_PROTOCOL would update > >> TCPA.LASA to it. > >> > >> Thanks > >> Laszlo > > > > How does EFI want to handle TCPA? Does caller allocate it > > log and fill in the address? > > TPM 1.2 seems to be completely absent from edk2, so I have no clue. TPM > 2.0 appears to be supported, but I already grepped edk2 for how it > handles the related ACPI table(s), and it doesn't. As far as I can see. > > In the long term, if you can find a way to make it work with SeaBIOS, > inside the linker/loader framework, that should work under OVMF as well. > I hope. Famous last words.
The TCPA table works using SeaBIOS's pointer relocation facility. Stefan
