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

[Laszlo Ersek]

On 07/30/14 17:03, Igor Mammedov wrote:
> On Wed, 30 Jul 2014 16:36:38 +0200
> Laszlo Ersek <address@hidden> wrote:
> 
>> 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.

> Can we allocate memory for log in QEMU and just pass its address in
> ACPI tables, marking arrea as reserved in e820? No linker would be
> involved and arrea would be easily accesssible to qemu/mgmt tools.

UEFI considers e820 legacy, and replaces it with the "memory space map"
(which describes hardware properties of memory ranges, approximately)
and the "memory map" (which describes what range is used how).

But, in any case, the platform code of OVMF does not "consume" these
memory maps, it *produces* them. Keying off from fw_cfg, CMOS, and so on.

(For example, the /etc/pci-info fw_cfg file would have been one input
point for this logic, to influence the _CRS, but that need has been
lifted by two chages, the first being the PCI hole allocation change in
qemu (for OVMF's ACPI generator), the second being exposing the ACPI
tables whole-sale to OVMF (which prevents OVMF's ACPI generator from
running).)

(If qemu currently exposes some e820 map in an fw_cfg file (I don't
remember), then OVMF certainly doesn't look at it. Even if it did, it
would have to translate it entry by entry to a memory space map (by
producing memory resource descriptor hand-off blocks in PlatformPei),
and that translation itself would be fraught with incompatibilities.)

IOW the issue is the same as it always has been: exposing information to
OVMF via easily parseable fw_cfg files (for hot-patching builtin
templates / generators), or via something else that it doesn't have to
parse *at all*, just pass through to the OS blindly.

Thanks
Laszlo