Laszlo Ersek <ler...@redhat.com> wrote on 07/30/2014 11:41:10 AM: > From: Laszlo Ersek <ler...@redhat.com> > To: Stefan Berger/Watson/IBM@IBMUS, "Michael S. Tsirkin" <m...@redhat.com> > Cc: qemu-devel@nongnu.org, Stefan Berger <stef...@linux.vnet.ibm.com> > 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" <m...@redhat.com> wrote on 07/30/2014 11:20:41 AM: > > > >> From: "Michael S. Tsirkin" <m...@redhat.com> > >> To: Stefan Berger/Watson/IBM@IBMUS > >> Cc: Laszlo Ersek <ler...@redhat.com>, qemu-devel@nongnu.org, Stefan > >> Berger <stef...@linux.vnet.ibm.com> > >> 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 <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); > >> > > >> > 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? Stefan
