qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support

From: Stefan Berger
Subject: Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
Date: Fri, 15 May 2015 11:31:30 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 
On 05/15/2015 10:44 AM, Igor Mammedov wrote:

On Fri,  8 May 2015 11:52:46 -0400
Stefan Berger <address@hidden> wrote:


Add a TPM2 ACPI table if a TPM 2 is used in the backend.
Also add an SSDT for the TPM 2.

Rename tpm_find() to tpm_get_version() and have this function
return the version of the TPM found, TPMVersion_Unspec if
no TPM is found. Use the version number to build version
specific ACPI tables.

Signed-off-by: Stefan Berger <address@hidden>

---

v2->v3:
    - followed Igor's suggestion of a default case with an assert()
    - added an SSDT for TPM2; it will be a bit different than TPM1.2's
      SSDT once we add PPI to it
---
  hw/i386/Makefile.objs |  2 +-
  hw/i386/acpi-build.c  | 38 ++++++++++++++++++++++++++++++++++----
  hw/i386/acpi-defs.h   | 18 ++++++++++++++++++
  hw/i386/ssdt-tpm2.dsl | 44 ++++++++++++++++++++++++++++++++++++++++++++
  hw/tpm/tpm_tis.c      | 11 +++++++++++
  include/hw/acpi/tpm.h |  5 +++++
  include/sysemu/tpm.h  | 11 +++++++++--
  7 files changed, 122 insertions(+), 7 deletions(-)
  create mode 100644 hw/i386/ssdt-tpm2.dsl

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index e058a39..0be5d97 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -9,7 +9,7 @@ obj-y += kvmvapic.o
  obj-y += acpi-build.o
  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
        hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
-       hw/i386/ssdt-tpm.hex
+       hw/i386/ssdt-tpm.hex hw/i386/ssdt-tpm2.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 e761005..e648ab5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -42,6 +42,7 @@
  #include "hw/acpi/memory_hotplug.h"
  #include "sysemu/tpm.h"
  #include "hw/acpi/tpm.h"
+#include "sysemu/tpm_backend.h"
/* Supported chipsets: */ 
  #include "hw/acpi/piix4.h"
@@ -111,7 +112,7 @@ typedef struct AcpiPmInfo {
typedef struct AcpiMiscInfo { 
      bool has_hpet;
-    bool has_tpm;
+    TPMVersion tpm_version;
      const unsigned char *dsdt_code;
      unsigned dsdt_size;
      uint16_t pvpanic_port;
@@ -239,7 +240,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->tpm_version = tpm_get_version();
      info->pvpanic_port = pvpanic_port();
      info->applesmc_io_base = applesmc_port();
  }
@@ -468,6 +469,7 @@ build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo 
*cpu,
  }
#include "hw/i386/ssdt-tpm.hex" 
+#include "hw/i386/ssdt-tpm2.hex"
/* Assign BSEL property to all buses. In the future, this can be changed 
   * to only assign to buses that support hotplug.
@@ -1065,6 +1067,25 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
  }
+static void 
+build_tpm2(GArray *table_data, GArray *linker)
+{
+    Acpi20TPM2 *tpm2_ptr;
+    void *tpm_ptr;
+
+    tpm_ptr = acpi_data_push(table_data, sizeof(ssdt_tpm2_aml));
+    memcpy(tpm_ptr, ssdt_tpm2_aml, sizeof(ssdt_tpm2_aml));

^^^ does almost the same as build_tpm_ssdt(), replacing
AML template with dynamic AML generation would help to consolidate
v1 and v2 versions of  build_tpm_ssdt().


So the SSDT's for the TPM v1.2 and TPM 2 differ, but only slightly so.
What does building the AML template via Opcodes etc. help in this case? Doesn't this make the code more or less unreadable? I don't understand why we shoudln't use 
the AML compiler on it?





+
+    tpm2_ptr = acpi_data_push(table_data, sizeof *tpm2_ptr);
+
+    tpm2_ptr->platform_class = cpu_to_le16(TPM2_ACPI_CLASS_CLIENT);
+    tpm2_ptr->control_area_address = cpu_to_le64(0);
+    tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO);
+
+    build_header(linker, table_data,
+                 (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4);
+}
+
  typedef enum {
      MEM_AFFINITY_NOFLAGS      = 0,
      MEM_AFFINITY_ENABLED      = (1 << 0),
@@ -1428,12 +1449,21 @@ void acpi_build(PcGuestInfo *guest_info, 
AcpiBuildTables *tables)
          acpi_add_table(table_offsets, tables_blob);
          build_hpet(tables_blob, tables->linker);
      }
-    if (misc.has_tpm) {
+    if (misc.tpm_version != TPM_VERSION_UNSPEC) {
          acpi_add_table(table_offsets, tables_blob);
          build_tpm_tcpa(tables_blob, tables->linker, tables->tcpalog);
acpi_add_table(table_offsets, tables_blob); 
-        build_tpm_ssdt(tables_blob, tables->linker);
+        switch (misc.tpm_version) {
+        case TPM_VERSION_1_2:
+            build_tpm_ssdt(tables_blob, tables->linker);
+            break;
+        case TPM_VERSION_2_0:
+            build_tpm2(tables_blob, tables->linker);
+            break;
+        default:
+            assert(false);
+        }
      }
      if (guest_info->numa_nodes) {
          acpi_add_table(table_offsets, tables_blob);
diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h
index c4468f8..79ee9b1 100644
--- a/hw/i386/acpi-defs.h
+++ b/hw/i386/acpi-defs.h
@@ -316,6 +316,9 @@ typedef struct AcpiTableMcfg AcpiTableMcfg;
/* 
   * TCPA Description Table
+ *
+ * Following Level 00, Rev 00.37 of specs:
+ * http://www.trustedcomputinggroup.org/resources/tcg_acpi_specification
   */
  struct Acpi20Tcpa {
      ACPI_TABLE_HEADER_DEF                    /* ACPI common table header */
@@ -325,6 +328,21 @@ struct Acpi20Tcpa {
  } QEMU_PACKED;
  typedef struct Acpi20Tcpa Acpi20Tcpa;
+/* 
+ * TPM2
+ *
+ * Following Level 00, Rev 00.37 of specs:
+ * http://www.trustedcomputinggroup.org/resources/tcg_acpi_specification
+ */
+struct Acpi20TPM2 {
+    ACPI_TABLE_HEADER_DEF
+    uint16_t platform_class;
+    uint16_t reserved;
+    uint64_t control_area_address;
+    uint32_t start_method;
+} QEMU_PACKED;
+typedef struct Acpi20TPM2 Acpi20TPM2;
+
  /* DMAR - DMA Remapping table r2.2 */
  struct AcpiTableDmar {
      ACPI_TABLE_HEADER_DEF
diff --git a/hw/i386/ssdt-tpm2.dsl b/hw/i386/ssdt-tpm2.dsl
new file mode 100644
index 0000000..96936ed
--- /dev/null
+++ b/hw/i386/ssdt-tpm2.dsl
@@ -0,0 +1,44 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "hw/acpi/tpm.h"
+
+ACPI_EXTRACT_ALL_CODE ssdt_tpm2_aml
+
+DefinitionBlock (
+    "ssdt-tpm2.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, TPM_TIS_ADDR_BASE, TPM_TIS_ADDR_SIZE)
+
+                /* TPM 2 is recent enough to support interrupts */
+                IRQNoFlags () {TPM_TIS_IRQ}
+            })
+            Method (_STA, 0, NotSerialized) {
+                Return (0x0F)
+            }
+        }
+    }
+}

Pls, don't add another ASL template.
Pls tell me why it is better to compile by hand rather than use a higher level language here? 

   Regards,
       Stefan


It would be better to rewrite above using AML API in C
and while at it convert hw/i386/ssdt-tpm.dsl to AML API as well.
hw/i386/ssdt-tpm.dsl is pretty much the same as ssdt-tpm2.aml
except of IRQNoFlags() so they could share common code
and for v2 you'll add extra IRQ resource.

Also I'd suggest to put TPM device under \_SB.PCI0 scope
so that its _CRS wouldn't conflict with PCI0._CRS but rather
be a consumer of it.




diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 69fbfae..daf2ac9 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -32,6 +32,7 @@
  #include "tpm_tis.h"
  #include "qemu-common.h"
  #include "qemu/main-loop.h"
+#include "sysemu/tpm_backend.h"
#define DEBUG_TIS 0 @@ -962,6 +963,16 @@ static int tpm_tis_do_startup_tpm(TPMState *s) 
  }
/* 
+ * Get the TPMVersion of the backend device being used
+ */
+TPMVersion tpm_tis_get_tpm_version(Object *obj)
+{
+    TPMState *s = TPM(obj);
+
+    return tpm_backend_get_tpm_version(s->be_driver);
+}
+
+/*
   * This function is called when the machine starts, resets or due to
   * S3 resume.
   */
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 792fcbf..6d516c6 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -26,4 +26,9 @@
  #define TPM_TCPA_ACPI_CLASS_CLIENT  0
  #define TPM_TCPA_ACPI_CLASS_SERVER  1
+#define TPM2_ACPI_CLASS_CLIENT 0 
+#define TPM2_ACPI_CLASS_SERVER      1
+
+#define TPM2_START_METHOD_MMIO      6
+
  #endif /* HW_ACPI_TPM_H */
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 848df41..c143890 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -26,11 +26,18 @@ typedef enum  TPMVersion {
      TPM_VERSION_2_0 = 2,
  } TPMVersion;
+TPMVersion tpm_tis_get_tpm_version(Object *obj); 
+
  #define TYPE_TPM_TIS                "tpm-tis"
-static inline bool tpm_find(void) 
+static inline TPMVersion tpm_get_version(void)
  {
-    return object_resolve_path_type("", TYPE_TPM_TIS, NULL);
+    Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
+
+    if (obj) {
+        return tpm_tis_get_tpm_version(obj);
+    }
+    return TPM_VERSION_UNSPEC;
  }
#endif /* QEMU_TPM_H */
reply via email to
[Prev in Thread] Current Thread [Next in Thread]