Re: [Qemu-devel] [PATCH v2] Add ACPI tables for TPM

[Laszlo Ersek <lersek@redhat.com>]

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 <lersek@redhat.com> wrote on 07/30/2014 10:36:38 AM:
>>>
>>>> From: Laszlo Ersek <lersek@redhat.com>
>>>> To: "Michael S. Tsirkin" <mst@redhat.com>, Stefan Berger/Watson/IBM@IBMUS
>>>> Cc: qemu-devel@nongnu.org, Stefan Berger <stefanb@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 <stefanb@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.

Laszlo