qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/3] tpm: Upgrade TPM TIS for support of a TPM 2
@ 2015-05-08 15:52 Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 1/3] Extend TPM TIS interface to support " Stefan Berger
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Stefan Berger @ 2015-05-08 15:52 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: imammedo, stefanb, quan.xu, Stefan Berger

After the previous upgrade of the TPM TIS to version 1.3,
we now upgrade it to support a TPM 2 in the backend.

Stefan Berger (3):
  Extend TPM TIS interface to support TPM 2
  tpm: Probe for connected TPM 1.2 or TPM 2
  TPM2 ACPI table support

 backends/tpm.c               |  14 +++++
 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/Makefile.objs         |   2 +-
 hw/tpm/tpm_int.h             |   7 +++
 hw/tpm/tpm_passthrough.c     |  65 ++++++----------------
 hw/tpm/tpm_tis.c             | 119 ++++++++++++++++++++++++++++++++++++----
 hw/tpm/tpm_tis.h             |   1 +
 hw/tpm/tpm_util.c            | 126 +++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_util.h            |  28 ++++++++++
 include/hw/acpi/tpm.h        |   5 ++
 include/sysemu/tpm.h         |  17 +++++-
 include/sysemu/tpm_backend.h |  23 ++++++++
 15 files changed, 443 insertions(+), 66 deletions(-)
 create mode 100644 hw/i386/ssdt-tpm2.dsl
 create mode 100644 hw/tpm/tpm_util.c
 create mode 100644 hw/tpm/tpm_util.h

-- 
1.9.3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v3 1/3] Extend TPM TIS interface to support TPM 2
  2015-05-08 15:52 [Qemu-devel] [PATCH v3 0/3] tpm: Upgrade TPM TIS for support of a TPM 2 Stefan Berger
@ 2015-05-08 15:52 ` Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 2/3] tpm: Probe for connected TPM 1.2 or " Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support Stefan Berger
  2 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2015-05-08 15:52 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: imammedo, stefanb, quan.xu, Stefan Berger

Following the recent upgrade to version 1.3, extend the TPM TIS
interface with capabilities introduced for support of a TPM 2.

TPM TIS for TPM 2 introduced the following extensions beyond the
TPM TIS 1.3 (used for TPM 1.2):

- A new 32bit interface Id register was introduced.
- New flags for the status (STS) register were defined.
- New flags for the capability flags were defined.

Support the above if a TPM TIS 1.3 for TPM 2 is used with a TPM 2
on the backend side. Support the old TPM TIS 1.3 configuration if a
TPM 1.2 is being used. A subsequent patch will then determine which
TPM version is being used in the backend.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>


---

v1->v2:
 - fixed enums to be all upper case
 - enum TPMVersion converted to typedef
---
 backends/tpm.c               |  14 ++++++
 hw/tpm/tpm_int.h             |   1 +
 hw/tpm/tpm_passthrough.c     |  14 ++++++
 hw/tpm/tpm_tis.c             | 108 +++++++++++++++++++++++++++++++++++++++----
 hw/tpm/tpm_tis.h             |   1 +
 include/sysemu/tpm.h         |   6 +++
 include/sysemu/tpm_backend.h |  23 +++++++++
 7 files changed, 158 insertions(+), 9 deletions(-)

diff --git a/backends/tpm.c b/backends/tpm.c
index 4efe367..9b0d0a5 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -96,6 +96,20 @@ bool tpm_backend_get_tpm_established_flag(TPMBackend *s)
     return k->ops->get_tpm_established_flag(s);
 }
 
+int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty)
+{
+    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+
+    return k->ops->reset_tpm_established_flag(s, locty);
+}
+
+TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
+{
+    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+
+    return k->ops->get_tpm_version(s);
+}
+
 static bool tpm_backend_prop_get_opened(Object *obj, Error **errp)
 {
     TPMBackend *s = TPM_BACKEND(obj);
diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index 2b35fe2..9866c79 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -29,6 +29,7 @@ struct TPMState {
 
     char *backend;
     TPMBackend *be_driver;
+    TPMVersion be_tpm_version;
 };
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 73ca906..f1361d2 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -267,6 +267,13 @@ static bool tpm_passthrough_get_tpm_established_flag(TPMBackend *tb)
     return false;
 }
 
+static int tpm_passthrough_reset_tpm_established_flag(TPMBackend *tb,
+                                                      uint8_t locty)
+{
+    /* only a TPM 2.0 will support this */
+    return 0;
+}
+
 static bool tpm_passthrough_get_startup_error(TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -324,6 +331,11 @@ static const char *tpm_passthrough_create_desc(void)
     return "Passthrough TPM backend driver";
 }
 
+static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
+{
+    return TPM_VERSION_1_2;
+}
+
 /*
  * A basic test of a TPM device. We expect a well formatted response header
  * (error response is fine) within one second.
@@ -540,6 +552,8 @@ static const TPMDriverOps tpm_passthrough_driver = {
     .deliver_request          = tpm_passthrough_deliver_request,
     .cancel_cmd               = tpm_passthrough_cancel_cmd,
     .get_tpm_established_flag = tpm_passthrough_get_tpm_established_flag,
+    .reset_tpm_established_flag = tpm_passthrough_reset_tpm_established_flag,
+    .get_tpm_version          = tpm_passthrough_get_tpm_version,
 };
 
 static void tpm_passthrough_inst_init(Object *obj)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 25b3afb..69fbfae 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -17,6 +17,9 @@
  * supports version 1.3, 21 March 2013
  * In the developers menu choose the PC Client section then find the TIS
  * specification.
+ *
+ * TPM TIS for TPM 2 implementation following TCG PC Client Platform
+ * TPM Profile (PTP) Specification, Familiy 2.0, Revision 00.43
  */
 
 #include "sysemu/tpm_backend.h"
@@ -49,6 +52,7 @@
 #define TPM_TIS_REG_INTF_CAPABILITY       0x14
 #define TPM_TIS_REG_STS                   0x18
 #define TPM_TIS_REG_DATA_FIFO             0x24
+#define TPM_TIS_REG_INTERFACE_ID          0x30
 #define TPM_TIS_REG_DATA_XFIFO            0x80
 #define TPM_TIS_REG_DATA_XFIFO_END        0xbc
 #define TPM_TIS_REG_DID_VID               0xf00
@@ -57,6 +61,12 @@
 /* vendor-specific registers */
 #define TPM_TIS_REG_DEBUG                 0xf90
 
+#define TPM_TIS_STS_TPM_FAMILY_MASK         (0x3 << 26)/* TPM 2.0 */
+#define TPM_TIS_STS_TPM_FAMILY1_2           (0 << 26)  /* TPM 2.0 */
+#define TPM_TIS_STS_TPM_FAMILY2_0           (1 << 26)  /* TPM 2.0 */
+#define TPM_TIS_STS_RESET_ESTABLISHMENT_BIT (1 << 25)  /* TPM 2.0 */
+#define TPM_TIS_STS_COMMAND_CANCEL          (1 << 24)  /* TPM 2.0 */
+
 #define TPM_TIS_STS_VALID                 (1 << 7)
 #define TPM_TIS_STS_COMMAND_READY         (1 << 6)
 #define TPM_TIS_STS_TPM_GO                (1 << 5)
@@ -102,15 +112,42 @@
 #endif
 
 #define TPM_TIS_CAP_INTERFACE_VERSION1_3 (2 << 28)
+#define TPM_TIS_CAP_INTERFACE_VERSION1_3_FOR_TPM2_0 (3 << 28)
 #define TPM_TIS_CAP_DATA_TRANSFER_64B    (3 << 9)
 #define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
 #define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0 << 8)
 #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is mandatory */
-#define TPM_TIS_CAPABILITIES_SUPPORTED   (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
-                                          TPM_TIS_CAP_BURST_COUNT_DYNAMIC | \
-                                          TPM_TIS_CAP_DATA_TRANSFER_64B | \
-                                          TPM_TIS_CAP_INTERFACE_VERSION1_3 | \
-                                          TPM_TIS_INTERRUPTS_SUPPORTED)
+#define TPM_TIS_CAPABILITIES_SUPPORTED1_3 \
+    (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
+     TPM_TIS_CAP_BURST_COUNT_DYNAMIC | \
+     TPM_TIS_CAP_DATA_TRANSFER_64B | \
+     TPM_TIS_CAP_INTERFACE_VERSION1_3 | \
+     TPM_TIS_INTERRUPTS_SUPPORTED)
+
+#define TPM_TIS_CAPABILITIES_SUPPORTED2_0 \
+    (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
+     TPM_TIS_CAP_BURST_COUNT_DYNAMIC | \
+     TPM_TIS_CAP_DATA_TRANSFER_64B | \
+     TPM_TIS_CAP_INTERFACE_VERSION1_3_FOR_TPM2_0 | \
+     TPM_TIS_INTERRUPTS_SUPPORTED)
+
+#define TPM_TIS_IFACE_ID_INTERFACE_TIS1_3   (0xf)     /* TPM 2.0 */
+#define TPM_TIS_IFACE_ID_INTERFACE_FIFO     (0x0)     /* TPM 2.0 */
+#define TPM_TIS_IFACE_ID_INTERFACE_VER_FIFO (0 << 4)  /* TPM 2.0 */
+#define TPM_TIS_IFACE_ID_CAP_5_LOCALITIES   (1 << 8)  /* TPM 2.0 */
+#define TPM_TIS_IFACE_ID_CAP_TIS_SUPPORTED  (1 << 13) /* TPM 2.0 */
+#define TPM_TIS_IFACE_ID_INT_SEL_LOCK       (1 << 19) /* TPM 2.0 */
+
+#define TPM_TIS_IFACE_ID_SUPPORTED_FLAGS1_3 \
+    (TPM_TIS_IFACE_ID_INTERFACE_TIS1_3 | \
+     (~0 << 4)/* all of it is don't care */)
+
+/* if backend was a TPM 2.0: */
+#define TPM_TIS_IFACE_ID_SUPPORTED_FLAGS2_0 \
+    (TPM_TIS_IFACE_ID_INTERFACE_FIFO | \
+     TPM_TIS_IFACE_ID_INTERFACE_VER_FIFO | \
+     TPM_TIS_IFACE_ID_CAP_5_LOCALITIES | \
+     TPM_TIS_IFACE_ID_CAP_TIS_SUPPORTED)
 
 #define TPM_TIS_TPM_DID       0x0001
 #define TPM_TIS_TPM_VID       PCI_VENDOR_ID_IBM
@@ -154,7 +191,8 @@ static void tpm_tis_show_buffer(const TPMSizedBuffer *sb, const char *string)
 
 /*
  * Set the given flags in the STS register by clearing the register but
- * preserving the SELFTEST_DONE flag and then setting the new flags.
+ * preserving the SELFTEST_DONE and TPM_FAMILY_MASK flags and then setting
+ * the new flags.
  *
  * The SELFTEST_DONE flag is acquired from the backend that determines it by
  * peeking into TPM commands.
@@ -166,7 +204,7 @@ static void tpm_tis_show_buffer(const TPMSizedBuffer *sb, const char *string)
  */
 static void tpm_tis_sts_set(TPMLocality *l, uint32_t flags)
 {
-    l->sts &= TPM_TIS_STS_SELFTEST_DONE;
+    l->sts &= TPM_TIS_STS_SELFTEST_DONE | TPM_TIS_STS_TPM_FAMILY_MASK;
     l->sts |= flags;
 }
 
@@ -489,7 +527,17 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
         val = tis->loc[locty].ints;
         break;
     case TPM_TIS_REG_INTF_CAPABILITY:
-        val = TPM_TIS_CAPABILITIES_SUPPORTED;
+        switch (s->be_tpm_version) {
+        case TPM_VERSION_UNSPEC:
+            val = 0;
+            break;
+        case TPM_VERSION_1_2:
+            val = TPM_TIS_CAPABILITIES_SUPPORTED1_3;
+            break;
+        case TPM_VERSION_2_0:
+            val = TPM_TIS_CAPABILITIES_SUPPORTED2_0;
+            break;
+        }
         break;
     case TPM_TIS_REG_STS:
         if (tis->active_locty == locty) {
@@ -536,6 +584,9 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
             shift = 0; /* no more adjustments */
         }
         break;
+    case TPM_TIS_REG_INTERFACE_ID:
+        val = tis->loc[locty].iface_id;
+        break;
     case TPM_TIS_REG_DID_VID:
         val = (TPM_TIS_TPM_DID << 16) | TPM_TIS_TPM_VID;
         break;
@@ -736,6 +787,25 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr,
             break;
         }
 
+        if (s->be_tpm_version == TPM_VERSION_2_0) {
+            /* some flags that are only supported for TPM 2 */
+            if (val & TPM_TIS_STS_COMMAND_CANCEL) {
+                if (tis->loc[locty].state == TPM_TIS_STATE_EXECUTION) {
+                    /*
+                     * request the backend to cancel. Some backends may not
+                     * support it
+                     */
+                    tpm_backend_cancel_cmd(s->be_driver);
+                }
+            }
+
+            if (val & TPM_TIS_STS_RESET_ESTABLISHMENT_BIT) {
+                if (locty == 3 || locty == 4) {
+                    tpm_backend_reset_tpm_established_flag(s->be_driver, locty);
+                }
+            }
+        }
+
         val &= (TPM_TIS_STS_COMMAND_READY | TPM_TIS_STS_TPM_GO |
                 TPM_TIS_STS_RESPONSE_RETRY);
 
@@ -860,6 +930,13 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr,
             }
         }
         break;
+    case TPM_TIS_REG_INTERFACE_ID:
+        if (val & TPM_TIS_IFACE_ID_INT_SEL_LOCK) {
+            for (l = 0; l < TPM_TIS_NUM_LOCALITIES; l++) {
+                tis->loc[l].iface_id |= TPM_TIS_IFACE_ID_INT_SEL_LOCK;
+            }
+        }
+        break;
     }
 }
 
@@ -894,6 +971,8 @@ static void tpm_tis_reset(DeviceState *dev)
     TPMTISEmuState *tis = &s->s.tis;
     int c;
 
+    s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
+
     tpm_backend_reset(s->be_driver);
 
     tis->active_locty = TPM_TIS_NO_LOCALITY;
@@ -902,7 +981,18 @@ static void tpm_tis_reset(DeviceState *dev)
 
     for (c = 0; c < TPM_TIS_NUM_LOCALITIES; c++) {
         tis->loc[c].access = TPM_TIS_ACCESS_TPM_REG_VALID_STS;
-        tis->loc[c].sts = 0;
+        switch (s->be_tpm_version) {
+        case TPM_VERSION_UNSPEC:
+            break;
+        case TPM_VERSION_1_2:
+            tis->loc[c].sts = TPM_TIS_STS_TPM_FAMILY1_2;
+            tis->loc[c].iface_id = TPM_TIS_IFACE_ID_SUPPORTED_FLAGS1_3;
+            break;
+        case TPM_VERSION_2_0:
+            tis->loc[c].sts = TPM_TIS_STS_TPM_FAMILY2_0;
+            tis->loc[c].iface_id = TPM_TIS_IFACE_ID_SUPPORTED_FLAGS2_0;
+            break;
+        }
         tis->loc[c].inte = TPM_TIS_INT_POLARITY_LOW_LEVEL;
         tis->loc[c].ints = 0;
         tis->loc[c].state = TPM_TIS_STATE_IDLE;
diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index db78d51..a1df41f 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -42,6 +42,7 @@ typedef struct TPMLocality {
     TPMTISState state;
     uint8_t access;
     uint32_t sts;
+    uint32_t iface_id;
     uint32_t inte;
     uint32_t ints;
 
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 9b81ce9..848df41 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -20,6 +20,12 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
 int tpm_init(void);
 void tpm_cleanup(void);
 
+typedef enum  TPMVersion {
+    TPM_VERSION_UNSPEC = 0,
+    TPM_VERSION_1_2 = 1,
+    TPM_VERSION_2_0 = 2,
+} TPMVersion;
+
 #define TYPE_TPM_TIS                "tpm-tis"
 
 static inline bool tpm_find(void)
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 540ee25..0a366be 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -88,6 +88,10 @@ struct TPMDriverOps {
     void (*cancel_cmd)(TPMBackend *t);
 
     bool (*get_tpm_established_flag)(TPMBackend *t);
+
+    int (*reset_tpm_established_flag)(TPMBackend *t, uint8_t locty);
+
+    TPMVersion (*get_tpm_version)(TPMBackend *t);
 };
 
 
@@ -192,6 +196,15 @@ void tpm_backend_cancel_cmd(TPMBackend *s);
 bool tpm_backend_get_tpm_established_flag(TPMBackend *s);
 
 /**
+ * tpm_backend_reset_tpm_established_flag:
+ * @s: the backend
+ * @locty: the locality number
+ *
+ * Reset the TPM establishment flag.
+ */
+int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty);
+
+/**
  * tpm_backend_open:
  * @s: the backend to open
  * @errp: a pointer to return the #Error object if an error occurs.
@@ -201,6 +214,16 @@ bool tpm_backend_get_tpm_established_flag(TPMBackend *s);
  */
 void tpm_backend_open(TPMBackend *s, Error **errp);
 
+/**
+ * tpm_backend_get_tpm_version:
+ * @s: the backend to call into
+ *
+ * Get the TPM Version that is emulated at the backend.
+ *
+ * Returns TPMVersion.
+ */
+TPMVersion tpm_backend_get_tpm_version(TPMBackend *s);
+
 TPMBackend *qemu_find_tpm(const char *id);
 
 const TPMDriverOps *tpm_get_backend_driver(const char *type);
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v3 2/3] tpm: Probe for connected TPM 1.2 or TPM 2
  2015-05-08 15:52 [Qemu-devel] [PATCH v3 0/3] tpm: Upgrade TPM TIS for support of a TPM 2 Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 1/3] Extend TPM TIS interface to support " Stefan Berger
@ 2015-05-08 15:52 ` Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support Stefan Berger
  2 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2015-05-08 15:52 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: imammedo, stefanb, quan.xu, Stefan Berger

In the TPM passthrough backend driver, modify the probing code so
that we can check whether a TPM 1.2 or TPM 2 is being used
and adapt the behavior of the TPM TIS accordingly.

Move the code that tested for a TPM 1.2 into tpm_utils.c
and extend it with test for probing for TPM 2. Have the
function return the version of TPM found.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

---

v1->v2:
 - prefixed TPM 2 #define's with TPM2
---
 hw/tpm/Makefile.objs     |   2 +-
 hw/tpm/tpm_int.h         |   6 +++
 hw/tpm/tpm_passthrough.c |  59 +++-------------------
 hw/tpm/tpm_util.c        | 126 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_util.h        |  28 +++++++++++
 5 files changed, 167 insertions(+), 54 deletions(-)
 create mode 100644 hw/tpm/tpm_util.c
 create mode 100644 hw/tpm/tpm_util.h

diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index 99f5983..64cecc3 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,2 +1,2 @@
 common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
-common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
+common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o tpm_util.o
diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index 9866c79..f2f285b 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -66,4 +66,10 @@ struct tpm_resp_hdr {
 #define TPM_ORD_ContinueSelfTest  0x53
 #define TPM_ORD_GetTicks          0xf1
 
+
+/* TPM2 defines */
+#define TPM2_ST_NO_SESSIONS       0x8001
+
+#define TPM2_CC_ReadClock         0x00000181
+
 #endif /* TPM_TPM_INT_H */
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index f1361d2..8d8523a 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -33,6 +33,7 @@
 #include "hw/i386/pc.h"
 #include "sysemu/tpm_backend_int.h"
 #include "tpm_tis.h"
+#include "tpm_util.h"
 
 #define DEBUG_TPM 0
 
@@ -69,6 +70,8 @@ struct TPMPassthruState {
     bool tpm_op_canceled;
     int cancel_fd;
     bool had_startup_error;
+
+    TPMVersion tpm_version;
 };
 
 typedef struct TPMPassthruState TPMPassthruState;
@@ -333,59 +336,9 @@ static const char *tpm_passthrough_create_desc(void)
 
 static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
 {
-    return TPM_VERSION_1_2;
-}
-
-/*
- * A basic test of a TPM device. We expect a well formatted response header
- * (error response is fine) within one second.
- */
-static int tpm_passthrough_test_tpmdev(int fd)
-{
-    struct tpm_req_hdr req = {
-        .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-        .len = cpu_to_be32(sizeof(req)),
-        .ordinal = cpu_to_be32(TPM_ORD_GetTicks),
-    };
-    struct tpm_resp_hdr *resp;
-    fd_set readfds;
-    int n;
-    struct timeval tv = {
-        .tv_sec = 1,
-        .tv_usec = 0,
-    };
-    unsigned char buf[1024];
-
-    n = write(fd, &req, sizeof(req));
-    if (n < 0) {
-        return errno;
-    }
-    if (n != sizeof(req)) {
-        return EFAULT;
-    }
-
-    FD_ZERO(&readfds);
-    FD_SET(fd, &readfds);
-
-    /* wait for a second */
-    n = select(fd + 1, &readfds, NULL, NULL, &tv);
-    if (n != 1) {
-        return errno;
-    }
-
-    n = read(fd, &buf, sizeof(buf));
-    if (n < sizeof(struct tpm_resp_hdr)) {
-        return EFAULT;
-    }
-
-    resp = (struct tpm_resp_hdr *)buf;
-    /* check the header */
-    if (be16_to_cpu(resp->tag) != TPM_TAG_RSP_COMMAND ||
-        be32_to_cpu(resp->len) != n) {
-        return EBADMSG;
-    }
+    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
 
-    return 0;
+    return tpm_pt->tpm_version;
 }
 
 /*
@@ -455,7 +408,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
         goto err_free_parameters;
     }
 
-    if (tpm_passthrough_test_tpmdev(tpm_pt->tpm_fd)) {
+    if (tpm_util_test_tpmdev(tpm_pt->tpm_fd, &tpm_pt->tpm_version)) {
         error_report("'%s' is not a TPM device.",
                      tpm_pt->tpm_dev);
         goto err_close_tpmdev;
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
new file mode 100644
index 0000000..4ace585
--- /dev/null
+++ b/hw/tpm/tpm_util.c
@@ -0,0 +1,126 @@
+/*
+ * TPM utility functions
+ *
+ *  Copyright (c) 2010 - 2015 IBM Corporation
+ *  Authors:
+ *    Stefan Berger <stefanb@us.ibm.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "tpm_util.h"
+#include "tpm_int.h"
+
+/*
+ * A basic test of a TPM device. We expect a well formatted response header
+ * (error response is fine) within one second.
+ */
+static int tpm_util_test(int fd,
+                         unsigned char *request,
+                         size_t requestlen,
+                         uint16_t *return_tag)
+{
+    struct tpm_resp_hdr *resp;
+    fd_set readfds;
+    int n;
+    struct timeval tv = {
+        .tv_sec = 1,
+        .tv_usec = 0,
+    };
+    unsigned char buf[1024];
+
+    n = write(fd, request, requestlen);
+    if (n < 0) {
+        return errno;
+    }
+    if (n != requestlen) {
+        return EFAULT;
+    }
+
+    FD_ZERO(&readfds);
+    FD_SET(fd, &readfds);
+
+    /* wait for a second */
+    n = select(fd + 1, &readfds, NULL, NULL, &tv);
+    if (n != 1) {
+        return errno;
+    }
+
+    n = read(fd, &buf, sizeof(buf));
+    if (n < sizeof(struct tpm_resp_hdr)) {
+        return EFAULT;
+    }
+
+    resp = (struct tpm_resp_hdr *)buf;
+    /* check the header */
+    if (be32_to_cpu(resp->len) != n) {
+        return EBADMSG;
+    }
+
+    *return_tag = be16_to_cpu(resp->tag);
+
+    return 0;
+}
+
+/*
+ * Probe for the TPM device in the back
+ * Returns 0 on success with the version of the probed TPM set, 1 on failure.
+ */
+int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
+{
+    /*
+     * Sending a TPM1.2 command to a TPM2 should return a TPM1.2
+     * header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
+     *
+     * Sending a TPM2 command to a TPM 2 will give a TPM 2 tag in the
+     * header.
+     * Sending a TPM2 command to a TPM 1.2 will give a TPM 1.2 tag
+     * in the header and an error code.
+     */
+    const struct tpm_req_hdr test_req = {
+        .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
+        .len = cpu_to_be32(sizeof(test_req)),
+        .ordinal = cpu_to_be32(TPM_ORD_GetTicks),
+    };
+
+    const struct tpm_req_hdr test_req_tpm2 = {
+        .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+        .len = cpu_to_be32(sizeof(test_req_tpm2)),
+        .ordinal = cpu_to_be32(TPM2_CC_ReadClock),
+    };
+    uint16_t return_tag;
+    int ret;
+
+    /* Send TPM 2 command */
+    ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2,
+                        sizeof(test_req_tpm2), &return_tag);
+    /* TPM 2 would respond with a tag of TPM2_ST_NO_SESSIONS */
+    if (!ret && return_tag == TPM2_ST_NO_SESSIONS) {
+        *tpm_version = TPM_VERSION_2_0;
+        return 0;
+    }
+
+    /* Send TPM 1.2 command */
+    ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req,
+                        sizeof(test_req), &return_tag);
+    if (!ret && return_tag == TPM_TAG_RSP_COMMAND) {
+        *tpm_version = TPM_VERSION_1_2;
+        /* this is a TPM 1.2 */
+        return 0;
+    }
+
+    *tpm_version = TPM_VERSION_UNSPEC;
+
+    return 1;
+}
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
new file mode 100644
index 0000000..e7f354a
--- /dev/null
+++ b/hw/tpm/tpm_util.h
@@ -0,0 +1,28 @@
+/*
+ * TPM utility functions
+ *
+ *  Copyright (c) 2010 - 2015 IBM Corporation
+ *  Authors:
+ *    Stefan Berger <stefanb@us.ibm.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+#ifndef TPM_TPM_UTILS_H
+#define TPM_TPM_UTILS_H
+
+#include "sysemu/tpm_backend.h"
+
+int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
+
+#endif /* TPM_TPM_UTILS_H */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
  2015-05-08 15:52 [Qemu-devel] [PATCH v3 0/3] tpm: Upgrade TPM TIS for support of a TPM 2 Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 1/3] Extend TPM TIS interface to support " Stefan Berger
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 2/3] tpm: Probe for connected TPM 1.2 or " Stefan Berger
@ 2015-05-08 15:52 ` Stefan Berger
  2015-05-15 14:44   ` Igor Mammedov
  2 siblings, 1 reply; 9+ messages in thread
From: Stefan Berger @ 2015-05-08 15:52 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: imammedo, stefanb, quan.xu, Stefan Berger

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 <stefanb@linux.vnet.ibm.com>

---

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));
+
+    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)
+            }
+        }
+    }
+}
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 */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
  2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support Stefan Berger
@ 2015-05-15 14:44   ` Igor Mammedov
  2015-05-15 15:31     ` Stefan Berger
  0 siblings, 1 reply; 9+ messages in thread
From: Igor Mammedov @ 2015-05-15 14:44 UTC (permalink / raw)
  To: Stefan Berger; +Cc: stefanb, qemu-devel, quan.xu, mst

On Fri,  8 May 2015 11:52:46 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> 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 <stefanb@linux.vnet.ibm.com>
> 
> ---
> 
> 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().

> +
> +    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.
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 */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
  2015-05-15 14:44   ` Igor Mammedov
@ 2015-05-15 15:31     ` Stefan Berger
  2015-05-15 16:57       ` Igor Mammedov
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Berger @ 2015-05-15 15:31 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: stefanb, mst, qemu-devel, George Wilson, Dimitrios Pendarakis,
	quan.xu

On 05/15/2015 10:44 AM, Igor Mammedov wrote:
> On Fri,  8 May 2015 11:52:46 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> 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 <stefanb@linux.vnet.ibm.com>
>>
>> ---
>>
>> 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 */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
  2015-05-15 15:31     ` Stefan Berger
@ 2015-05-15 16:57       ` Igor Mammedov
  2015-05-15 19:13         ` Stefan Berger
  0 siblings, 1 reply; 9+ messages in thread
From: Igor Mammedov @ 2015-05-15 16:57 UTC (permalink / raw)
  To: Stefan Berger
  Cc: stefanb, mst, qemu-devel, George Wilson, Dimitrios Pendarakis,
	quan.xu

On Fri, 15 May 2015 11:31:30 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 05/15/2015 10:44 AM, Igor Mammedov wrote:
> > On Fri,  8 May 2015 11:52:46 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> 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 <stefanb@linux.vnet.ibm.com>
> >>
> >> ---
> >>
> >> 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?
Generating AML code directly from C allows us to drop maintaining
binary blobs for AML templates in QEMU source tree simplifying
code in most cases (but sometimes it's a bit verbose).
So far we've got rid of SSDT templates creating it completely
from C and when I'm get my hands in remaining DSDT eventually
it would be possible to drop dependency on IASL.

AML API is not operating in terms of opcodes but mostly
in terms that resemble ASL.
For example look at:

 8ac6f7a pc: acpi-build: drop template patching and create Device(SMC) dynamically

which I think is the closest to what is needed to convert TPM device.
Also since you are building a dedicated SSDT table for TPM
you might need to look at

 011bb749 pc: acpi-build: use aml_scope() for \_SB scope

to see how API is initialized, but instead of generating a
separate SSDT I'd suggest to move Device(TPM) generation into
common SSDT (build_ssdt()).

Ask me if you have any questions regarding usage of API,
I should be able to point to relevant commits.

> 
> 
> >
> >> +
> >> +    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?
It addition to above reasons creating it dynamically
it would be possible to make TPM_TIS_ADDR_BASE not
hardcoded throughout QEMU and SeaBIOS but configurable
as tpm device property.

> 
>     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 */
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
  2015-05-15 16:57       ` Igor Mammedov
@ 2015-05-15 19:13         ` Stefan Berger
  2015-05-19 13:16           ` Igor Mammedov
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Berger @ 2015-05-15 19:13 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Stefan Berger, mst, qemu-devel, George Wilson,
	Dimitrios Pendarakis, quan.xu

[-- Attachment #1: Type: text/plain, Size: 14615 bytes --]

Igor Mammedov <imammedo@redhat.com> wrote on 05/15/2015 12:57:40 PM:

> From: Igor Mammedov <imammedo@redhat.com>
> To: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: qemu-devel@nongnu.org, mst@redhat.com, quan.xu@intel.com, Stefan
> Berger/Watson/IBM@IBMUS, Dimitrios Pendarakis/Watson/IBM@IBMUS, 
> George Wilson/Austin/IBM@IBMUS
> Date: 05/15/2015 01:07 PM
> Subject: Re: [PATCH v3 3/3] TPM2 ACPI table support
> 
> On Fri, 15 May 2015 11:31:30 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> 
> > On 05/15/2015 10:44 AM, Igor Mammedov wrote:
> > > On Fri,  8 May 2015 11:52:46 -0400
> > > Stefan Berger <stefanb@linux.vnet.ibm.com> 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 <stefanb@linux.vnet.ibm.com>
> > >>
> > >> ---
> > >>
> > >> 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?
> Generating AML code directly from C allows us to drop maintaining
> binary blobs for AML templates in QEMU source tree simplifying
> code in most cases (but sometimes it's a bit verbose).

Phew, I looked at the code. It becomes quite a hurdle to contribute 
something in this area.
Being able to write and learn ASL is one thing, building something similar 
to
the ASL in C is yet another thing.

In the TPM case we have 2 SSDTs, one for TPM 1.2 and one for TPM 2. They 
are very similar
and could be merged into one asl code with #if around the parts that are 
different,
which are related to the IRQ and the check for ranges of opcodes 
supported. Couldn't we
use these first and then, if absolutely necessary, convert it?

> So far we've got rid of SSDT templates creating it completely
> from C and when I'm get my hands in remaining DSDT eventually
> it would be possible to drop dependency on IASL.

IASL is from an rpm, right ? Why not use it for the simpler cases?

> 
> AML API is not operating in terms of opcodes but mostly
> in terms that resemble ASL.
> For example look at:
> 
>  8ac6f7a pc: acpi-build: drop template patching and create Device
> (SMC) dynamically
> 
> which I think is the closest to what is needed to convert TPM device.
> Also since you are building a dedicated SSDT table for TPM
> you might need to look at
> 
>  011bb749 pc: acpi-build: use aml_scope() for \_SB scope
> 
> to see how API is initialized, but instead of generating a
> separate SSDT I'd suggest to move Device(TPM) generation into
> common SSDT (build_ssdt()).
> 
> Ask me if you have any questions regarding usage of API,
> I should be able to point to relevant commits.


   Stefan


> 
> > 
> > 
> > >
> > >> +
> > >> +    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?
> It addition to above reasons creating it dynamically
> it would be possible to make TPM_TIS_ADDR_BASE not
> hardcoded throughout QEMU and SeaBIOS but configurable
> as tpm device property.
> 
> > 
> >     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 */
> > 
> 

[-- Attachment #2: Type: text/html, Size: 23645 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support
  2015-05-15 19:13         ` Stefan Berger
@ 2015-05-19 13:16           ` Igor Mammedov
  0 siblings, 0 replies; 9+ messages in thread
From: Igor Mammedov @ 2015-05-19 13:16 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Stefan Berger, mst, qemu-devel, George Wilson,
	Dimitrios Pendarakis, quan.xu

On Fri, 15 May 2015 15:13:14 -0400
Stefan Berger <stefanb@us.ibm.com> wrote:

> Igor Mammedov <imammedo@redhat.com> wrote on 05/15/2015 12:57:40 PM:
> 
> > From: Igor Mammedov <imammedo@redhat.com>
> > To: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > Cc: qemu-devel@nongnu.org, mst@redhat.com, quan.xu@intel.com, Stefan
> > Berger/Watson/IBM@IBMUS, Dimitrios Pendarakis/Watson/IBM@IBMUS, 
> > George Wilson/Austin/IBM@IBMUS
> > Date: 05/15/2015 01:07 PM
> > Subject: Re: [PATCH v3 3/3] TPM2 ACPI table support
> > 
> > On Fri, 15 May 2015 11:31:30 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> > 
> > > On 05/15/2015 10:44 AM, Igor Mammedov wrote:
> > > > On Fri,  8 May 2015 11:52:46 -0400
> > > > Stefan Berger <stefanb@linux.vnet.ibm.com> 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 <stefanb@linux.vnet.ibm.com>
> > > >>
> > > >> ---
> > > >>
> > > >> 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?
> > Generating AML code directly from C allows us to drop maintaining
> > binary blobs for AML templates in QEMU source tree simplifying
> > code in most cases (but sometimes it's a bit verbose).
> 
> Phew, I looked at the code. It becomes quite a hurdle to contribute 
> something in this area.
> Being able to write and learn ASL is one thing, building something similar 
> to
> the ASL in C is yet another thing.
for current TPM conversion it's trivial (including TPM2):

+        scope = aml_scope("\\_SB.PCI0");
+        dev = aml_device("TPM");
+
+        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
+        /* device present, functioning, decoding, not shown in UI */
+        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
+        crs = aml_resource_template();
+        aml_append(crs,
+            aml_memory32_fixed(TPM_TIS_ADDR_BASE, TPM_TIS_ADDR_SIZE, aml_ReadWrite)
+        );
+        if (tpm2) {
+            aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
+        }
+        aml_append(dev, aml_name_decl("_CRS", crs));
+        aml_append(scope, dev);
+
+        aml_append(ssdt, scope);

aml_memory32_fixed is not in mainline yet, but should be there soon:
"[PATCH v6 04/22] hw/acpi/aml-build: Add aml_memory32_fixed() term"

> In the TPM case we have 2 SSDTs, one for TPM 1.2 and one for TPM 2. They 
> are very similar
> and could be merged into one asl code with #if around the parts that are 
> different,
> which are related to the IRQ and the check for ranges of opcodes 
> supported. Couldn't we
> use these first and then, if absolutely necessary, convert it?
you might have noticed, a new ARM ACPI patches don't use templates
at all and use only C API to build tables.
x86 ASL parts in process of converting, it would be nice not to add
more conversion material.
 
> > So far we've got rid of SSDT templates creating it completely
> > from C and when I'm get my hands in remaining DSDT eventually
> > it would be possible to drop dependency on IASL.
> 
> IASL is from an rpm, right ? Why not use it for the simpler cases?
Maintaining binary template blobs is hassle, there were numerous cases
when master tree build failed or didn't work as expected after ACPI
patches were pulled in, since developer and maintainers usually have IASL
on their boxes and forget about need to update blobs as well.
Dropping dependency on IASL and precompiled blobs helps us to make
less mistakes here.
 
> > AML API is not operating in terms of opcodes but mostly
> > in terms that resemble ASL.
> > For example look at:
> > 
> >  8ac6f7a pc: acpi-build: drop template patching and create Device
> > (SMC) dynamically
> > 
> > which I think is the closest to what is needed to convert TPM device.
> > Also since you are building a dedicated SSDT table for TPM
> > you might need to look at
> > 
> >  011bb749 pc: acpi-build: use aml_scope() for \_SB scope
> > 
> > to see how API is initialized, but instead of generating a
> > separate SSDT I'd suggest to move Device(TPM) generation into
> > common SSDT (build_ssdt()).
> > 
> > Ask me if you have any questions regarding usage of API,
> > I should be able to point to relevant commits.
> 
> 
>    Stefan
> 
> 
> > 
> > > 
> > > 
> > > >
> > > >> +
> > > >> +    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?
> > It addition to above reasons creating it dynamically
> > it would be possible to make TPM_TIS_ADDR_BASE not
> > hardcoded throughout QEMU and SeaBIOS but configurable
> > as tpm device property.
> > 
> > > 
> > >     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 */
> > > 
> > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-05-19 13:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-08 15:52 [Qemu-devel] [PATCH v3 0/3] tpm: Upgrade TPM TIS for support of a TPM 2 Stefan Berger
2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 1/3] Extend TPM TIS interface to support " Stefan Berger
2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 2/3] tpm: Probe for connected TPM 1.2 or " Stefan Berger
2015-05-08 15:52 ` [Qemu-devel] [PATCH v3 3/3] TPM2 ACPI table support Stefan Berger
2015-05-15 14:44   ` Igor Mammedov
2015-05-15 15:31     ` Stefan Berger
2015-05-15 16:57       ` Igor Mammedov
2015-05-15 19:13         ` Stefan Berger
2015-05-19 13:16           ` Igor Mammedov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).