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

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 07/30/2014 09:20 AM, 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
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   hw/i386/Makefile.objs |  3 ++-
>>   hw/i386/acpi-build.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/i386/acpi-defs.h   | 11 +++++++++++
>>   hw/i386/ssdt-tpm.dsl  | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   hw/tpm/tpm_tis.h      |  5 +----
>>   include/hw/acpi/tpm.h | 29 +++++++++++++++++++++++++++++
>>   include/sysemu/tpm.h  |  5 +++++
>>   7 files changed, 137 insertions(+), 5 deletions(-)
>>   create mode 100644 hw/i386/ssdt-tpm.dsl
>>   create mode 100644 include/hw/acpi/tpm.h
>>
>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
>> index 48014ab..3688cf8 100644
>> --- a/hw/i386/Makefile.objs
>> +++ b/hw/i386/Makefile.objs
>> @@ -10,7 +10,8 @@ obj-y += bios-linker-loader.o
>>   hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
>>   	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
>>   	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
>> -	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex
>> +	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
>> +	hw/i386/ssdt-tpm.hex
>>   
>>   iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>>       ; then echo "$(2)"; else echo "$(3)"; fi ;)
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index ebc5f03..d767e37 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -38,6 +38,8 @@
>>   #include "hw/loader.h"
>>   #include "hw/isa/isa.h"
>>   #include "hw/acpi/memory_hotplug.h"
>> +#include "sysemu/tpm.h"
>> +#include "hw/acpi/tpm.h"
>>   
>>   /* Supported chipsets: */
>>   #include "hw/acpi/piix4.h"
>> @@ -75,6 +77,7 @@ typedef struct AcpiPmInfo {
>>   
>>   typedef struct AcpiMiscInfo {
>>       bool has_hpet;
>> +    bool has_tpm;
>>       DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
>>       const unsigned char *dsdt_code;
>>       unsigned dsdt_size;
>> @@ -193,6 +196,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
>>   static void acpi_get_misc_info(AcpiMiscInfo *info)
>>   {
>>       info->has_hpet = hpet_find();
>> +    info->has_tpm = tpm_find();
>>       info->pvpanic_port = pvpanic_port();
>>   }
>>   
>> @@ -681,6 +685,7 @@ static inline char acpi_get_hex(uint32_t val)
>>   
>>   #include "hw/i386/ssdt-misc.hex"
>>   #include "hw/i386/ssdt-pcihp.hex"
>> +#include "hw/i386/ssdt-tpm.hex"
>>   
>>   static void
>>   build_append_notify_method(GArray *device, const char *name,
>> @@ -1167,6 +1172,40 @@ build_hpet(GArray *table_data, GArray *linker)
>>                    (void *)hpet, "HPET", sizeof(*hpet), 1);
>>   }
>>   
>> +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.

Why does the BIOS 'need' to allocate it? Why can it not just use the 
memory that QEMU allocates? Obviously I am using the 'pointer 
relocation' feature of the BIOS to bend the pointer in the TCPA table to 
this log area.

>
> 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 ..

There is currently no way to disable it. For a machine with a TPM, there 
should be support for an SSDT and this TCPA table for the BIOS to write 
logs into. So I allocate both and Linux for example can then show an 
empty table in /sys/kernel/security/tpm0/ascii_bios_measurements when 
the passthrough driver is used. I am working on a TPM driver for a CUSE 
TPM(CUSE = character device in user space) where we want the BIOS to 
behave exactly like the BIOS on real hardware and write its measurements 
into this log. I know at least that this then works the way it is 
implemented now.

    Stefan