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

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

On 07/22/2014 03:05 PM, Michael S. Tsirkin wrote:
> On Tue, Jul 22, 2014 at 03:55:38PM -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>
> Some comments below.
> Also pls remember to repost when 2.1 is out.

Will do.
See answers below.

>
>> ---
>>   hw/i386/Makefile.objs |  3 ++-
>>   hw/i386/acpi-build.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   hw/i386/acpi-defs.h   | 11 +++++++++++
>>   hw/i386/ssdt-tpm.dsl  | 28 ++++++++++++++++++++++++++++
>>   include/sysemu/tpm.h  |  5 +++++
>>   5 files changed, 89 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/i386/ssdt-tpm.dsl
>>
>> 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..e5c2cdb 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -38,6 +38,7 @@
>>   #include "hw/loader.h"
>>   #include "hw/isa/isa.h"
>>   #include "hw/acpi/memory_hotplug.h"
>> +#include "sysemu/tpm.h"
>>   
>>   /* Supported chipsets: */
>>   #include "hw/acpi/piix4.h"
>> @@ -75,6 +76,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 +195,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 +684,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 +1171,38 @@ 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 laml = 128 * 1024;
> what are these numbers for?

Follow above referenced specs, laml stands for log area minimum length, 
in bytes. So this is the number of bytes of a memory area to log into.

>
>> +    uint64_t lasa;
>> +
>> +    lasa = table_data->len + sizeof(*tcpa);
>> +
>> +    tcpa = acpi_data_push(table_data, sizeof(*tcpa) + laml);
>> +
>> +    tcpa->laml = cpu_to_le32(laml);
>> +    tcpa->lasa = cpu_to_le64((uint64_t)lasa);
>> +
>> +    /* LASA address to be filled by Guest linker */
>> +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
>> +                                   ACPI_BUILD_TABLE_FILE,
>> +                                   table_data, &tcpa->lasa,
>> +                                   sizeof(tcpa->lasa));
>> +    build_header(linker, table_data,
>> +                 (void *)tcpa, "TCPA", sizeof(*tcpa), 2);
>> +}
>> +
>> +static void
>> +build_tpm_ssdt(GArray *table_data, GArray *linker)
>> +{
>> +    void *tpm_ptr;
>> +
>> +    tpm_ptr = acpi_data_push(table_data, sizeof(ssdt_tpm_aml));
>> +    memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
>> +}
>> +
>>   typedef enum {
>>       MEM_AFFINITY_NOFLAGS      = 0,
>>       MEM_AFFINITY_ENABLED      = (1 << 0),
>> @@ -1489,6 +1525,13 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>           acpi_add_table(table_offsets, tables->table_data);
>>           build_hpet(tables->table_data, tables->linker);
>>       }
>> +    if (misc.has_tpm) {
>> +        acpi_add_table(table_offsets, tables->table_data);
>> +        build_tpm_ssdt(tables->table_data, tables->linker);
>> +
>> +        acpi_add_table(table_offsets, tables->table_data);
>> +        build_tpm_tcpa(tables->table_data, tables->linker);
>> +    }
>>       if (guest_info->numa_nodes) {
>>           acpi_add_table(table_offsets, tables->table_data);
>>           build_srat(tables->table_data, tables->linker, &cpu, guest_info);
>> diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h
>> index e93babb..8526215 100644
>> --- a/hw/i386/acpi-defs.h
>> +++ b/hw/i386/acpi-defs.h
>> @@ -314,4 +314,15 @@ struct AcpiTableMcfg {
>>   } QEMU_PACKED;
>>   typedef struct AcpiTableMcfg AcpiTableMcfg;
>>   
>> +/*
>> + * TCPA Description Table
>> + */
>> +struct Acpi20Tcpa {
>> +    ACPI_TABLE_HEADER_DEF                    /* ACPI common table header */
>> +    uint16_t platform_class;
>> +    uint32_t laml;
>> +    uint64_t lasa;
>> +} QEMU_PACKED;
>> +typedef struct Acpi20Tcpa Acpi20Tcpa;
>> +
>>   #endif
>> diff --git a/hw/i386/ssdt-tpm.dsl b/hw/i386/ssdt-tpm.dsl
>> new file mode 100644
>> index 0000000..fd53cfb
>> --- /dev/null
>> +++ b/hw/i386/ssdt-tpm.dsl
>> @@ -0,0 +1,28 @@
>> +#include "hw/acpi/pc-hotplug.h"
>> +
>> +ACPI_EXTRACT_ALL_CODE ssdt_tpm_aml
>> +
>> +DefinitionBlock (
>> +    "ssdt-tpm.aml",     // Output Filename
>> +    "SSDT",             // Signature
>> +    0x01,               // SSDT Compliance Revision
>> +    "BXPC",             // OEMID
>> +    "BXSSDT",           // TABLE ID
>> +    0x1                 // OEM Revision
>> +    )
>> +{
>> +    Scope(\_SB) {
>> +        /* TPM with emulated TPM TIS interface */
>> +        Device (TPM) {
>> +            Name (_HID, EisaID ("PNP0C31"))
>> +            Name (_CRS, ResourceTemplate ()
>> +            {
>> +                Memory32Fixed (ReadWrite, 0xFED40000, 0x00005000)
> what are these magic numbers?

TOM TIS MMIO area start and length.

> you can use defines for them.
>
>> +                //IRQNoFlags () {5}
> What's this?

If we run the TPM with IRQ, older Linux kernels may not work; so polling 
mode is unfortunately better.

>
>> +            })
>> +            Method (_STA, 0, NotSerialized) {
>> +                Return (0x0F)
>> +            }
>> +        }
>> +    }
>> +}
>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>> index 13febdd..7cf2fc3 100644
>> --- a/include/sysemu/tpm.h
>> +++ b/include/sysemu/tpm.h
>> @@ -20,4 +20,9 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
>>   int tpm_init(void);
>>   void tpm_cleanup(void);
>>   
>> +static inline bool tpm_find(void)
>> +{
>> +    return (object_resolve_path_type("", "tpm-tis", NULL) != NULL);
> outer () not needed.

Ok.

>
>> +}
>> +
>>   #endif /* QEMU_TPM_H */
>> -- 
>> 1.9.3