All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
@ 2015-04-15 22:38 Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 1/5] Provide support for the CUSE TPM Stefan Berger
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-15 22:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: safford, Stefan Berger, quan.xu, mst

The following series of patches extends TPM support with an
external TPM that offers a Linux CUSE (character device in userspace)
interface. This TPM lets each VM access its own private vTPM.
The CUSE TPM supports suspend/resume and migration. Much
out-of-band functionality necessary to control the CUSE TPM is
implemented using ioctl's.

The series extends the TPM support so far that most functionality of
TPM support on a physical platform is now available to each x86 VM,
this includes the Physical Presence Interface support that has
its counter-part in the SeaBIOS and is implemented using ACPI.

http://www.seabios.org/pipermail/seabios/2015-March/008978.html


Stefan Berger (5):
  Provide support for the CUSE TPM
  Support Physical Presence Interface Spec
  Introduce condition to notifiy waiters of completed command
  Introduce condition in TPM backend for notification
  Add support for VM suspend/resume for TPM TIS

 hmp.c                        |   6 +
 hw/i386/acpi-tpm-core.dsl    | 277 +++++++++++++++++++++++++++++
 hw/i386/acpi-tpm2.dsl        |  27 +++
 hw/i386/q35-acpi-dsdt.dsl    |   1 +
 hw/i386/ssdt-tpm.dsl         |  12 +-
 hw/tpm/tpm_int.h             |   4 +
 hw/tpm/tpm_ioctl.h           | 178 +++++++++++++++++++
 hw/tpm/tpm_passthrough.c     | 410 +++++++++++++++++++++++++++++++++++++++++--
 hw/tpm/tpm_tis.c             | 152 +++++++++++++++-
 hw/tpm/tpm_tis.h             |   2 +
 hw/tpm/tpm_util.c            | 206 ++++++++++++++++++++++
 hw/tpm/tpm_util.h            |   7 +
 include/sysemu/tpm_backend.h |  12 ++
 qapi-schema.json             |  17 +-
 qemu-options.hx              |  21 ++-
 qmp-commands.hx              |   2 +-
 tpm.c                        |  11 +-
 17 files changed, 1316 insertions(+), 29 deletions(-)
 create mode 100644 hw/i386/acpi-tpm-core.dsl
 create mode 100644 hw/i386/acpi-tpm2.dsl
 create mode 100644 hw/tpm/tpm_ioctl.h

-- 
1.9.3

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

* [Qemu-devel] [PATCH 1/5] Provide support for the CUSE TPM
  2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
@ 2015-04-15 22:38 ` Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 2/5] Support Physical Presence Interface Spec Stefan Berger
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-15 22:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: safford, Stefan Berger, quan.xu, mst

Rather than integrating TPM functionality into QEMU directly
using the TPM emulation of libtpms, we now integrate an external
emulated TPM device. This device is expected to implement a Linux
CUSE interface (CUSE = character device in userspace).

QEMU talks to the CUSE TPM using much functionality of the
passthrough driver. For example, the TPM commands and responses
are sent to the CUSE TPM using the read()/write() interface.
However, some out-of-band control needs to be done using the CUSE
TPM's ioctl's. The CUSE TPM currently defines and implements 14
different ioctls for controlling certain life-cycle aspects of
the emulated TPM. The ioctls can be regarded as a replacement for
direct function calls to a TPM emulator if the TPM were to be
directly integrated into QEMU.

One  of the ioctl's allows to get a bitmask of supported capabilities.
Each returned bit indicates which capabilties have been implemented.
An include file defining the various ioctls is added to QEMU.

The CUSE TPM and associated tools can be found here:

https://github.com/stefanberger/swtpm


To use the external CUSE TPM, the CUSE TPM should be started as follows:

/usr/bin/swtpm_cuse -n vtpm-test

QEMU can then be started using the following parameters:

qemu-system-x86_64 \
	[...] \
        -tpmdev cuse-tpm,id=tpm0,cancel-path=/dev/null,path=/dev/vtpm-test \
        -device tpm-tis,id=tpm0,tpmdev=tpm0 \
	[...]


Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Eric Blake <eblake@redhat.com>
---
 hmp.c                    |   6 ++
 hw/tpm/tpm_int.h         |   1 +
 hw/tpm/tpm_ioctl.h       | 178 +++++++++++++++++++++++++++++++
 hw/tpm/tpm_passthrough.c | 271 +++++++++++++++++++++++++++++++++++++++++++++--
 qapi-schema.json         |  17 ++-
 qemu-options.hx          |  21 +++-
 qmp-commands.hx          |   2 +-
 tpm.c                    |  11 +-
 8 files changed, 490 insertions(+), 17 deletions(-)
 create mode 100644 hw/tpm/tpm_ioctl.h

diff --git a/hmp.c b/hmp.c
index f31ae27..1b1ce8e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -813,6 +813,12 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
                            tpo->has_cancel_path ? ",cancel-path=" : "",
                            tpo->has_cancel_path ? tpo->cancel_path : "");
             break;
+        case TPM_TYPE_OPTIONS_KIND_CUSE_TPM:
+            tpo = ti->options->passthrough;
+            monitor_printf(mon, "%s%s",
+                           tpo->has_path ? ",path=" : "",
+                           tpo->has_path ? tpo->path : "");
+            break;
         case TPM_TYPE_OPTIONS_KIND_MAX:
             break;
         }
diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index f2f285b..6b2c9c9 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -61,6 +61,7 @@ struct tpm_resp_hdr {
 #define TPM_TAG_RSP_AUTH1_COMMAND 0xc5
 #define TPM_TAG_RSP_AUTH2_COMMAND 0xc6
 
+#define TPM_SUCCESS               0
 #define TPM_FAIL                  9
 
 #define TPM_ORD_ContinueSelfTest  0x53
diff --git a/hw/tpm/tpm_ioctl.h b/hw/tpm/tpm_ioctl.h
new file mode 100644
index 0000000..d36e702
--- /dev/null
+++ b/hw/tpm/tpm_ioctl.h
@@ -0,0 +1,178 @@
+/*
+ * tpm_ioctl.h
+ *
+ * This file is licensed under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <stdint.h>
+#include <sys/uio.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+
+/*
+ * Every response from a command involving a TPM command execution must hold
+ * the ptmres_t as the first element.
+ * ptmres_t corresponds to the error code of a command executed by the TPM.
+ */
+
+typedef uint32_t ptmres_t;
+
+/* PTM_GET_TPMESTABLISHED */
+struct ptmest {
+    ptmres_t tpm_result;
+    unsigned char bit; /* TPM established bit */
+};
+
+/* PTM_RESET_PTMESTABLIHSED: reset establishment bit */
+struct ptmreset_est {
+    union {
+        struct {
+            uint8_t loc; /* locality to use */
+        } req;
+        struct {
+            ptmres_t tpm_result;
+        } resp;
+    } u;
+};
+
+/* PTM_INIT */
+struct ptminit {
+    union {
+        struct {
+            uint32_t init_flags; /* see definitions below */
+        } req;
+        struct {
+            ptmres_t tpm_result;
+        } resp;
+    } u;
+};
+
+/* above init_flags */
+#define INIT_FLAG_DELETE_VOLATILE (1 << 0)
+    /* delete volatile state file after reading it */
+
+/* PTM_SET_LOCALITY */
+struct ptmloc {
+    union {
+        struct {
+            uint8_t loc; /* locality to set */
+        } req;
+        struct {
+            ptmres_t tpm_result;
+        } resp;
+    } u;
+};
+
+/* PTM_HASH_DATA: hash given data */
+struct ptmhdata {
+    union {
+        struct {
+            uint32_t length;
+            uint8_t data[4096];
+        } req;
+        struct {
+            ptmres_t tpm_result;
+        } resp;
+    } u;
+};
+
+/*
+ * size of the TPM state blob to transfer; x86_64 can handle 8k, ppc64le only ~7k
+ * keep the response below a 4k page size
+ */
+#define STATE_BLOB_SIZE (3 * 1024)
+
+/*
+ * Data structure to get state blobs from the TPM. If the size of the
+ * blob exceeds the STATE_BLOB_SIZE, multiple reads with
+ * adjusted offset are necessary. The last packet is indicated by
+ * the length being smaller than the STATE_BLOB_SIZE.
+ */
+struct ptm_getstate {
+    union {
+        struct {
+            uint32_t state_flags; /* may be: STATE_FLAG_DECRYPTED */
+            uint32_t tpm_number;  /* always set to zero */
+            uint8_t type;         /* which blob to pull */
+            uint32_t offset;      /* offset from where to read */
+        } req;
+        struct {
+            ptmres_t tpm_result;
+            uint32_t state_flags; /* may be: STATE_FLAG_ENCRYPTED */
+            uint32_t length;
+            uint8_t  data[STATE_BLOB_SIZE];
+        } resp;
+    } u;
+};
+
+/* TPM state blob types */
+#define PTM_BLOB_TYPE_PERMANENT  1
+#define PTM_BLOB_TYPE_VOLATILE   2
+#define PTM_BLOB_TYPE_SAVESTATE  3
+
+/* state_flags above : */
+#define STATE_FLAG_DECRYPTED     1 /* on input:  get decrypted state */
+#define STATE_FLAG_ENCRYPTED     2 /* on output: state is encrytped */
+
+/*
+ * Data structure to set state blobs in the TPM. If the size of the
+ * blob exceeds the STATE_BLOB_SIZE, multiple 'writes' are necessary.
+ * The last packet is indicated by the length being smaller than the
+ * STATE_BLOB_SIZE.
+ */
+struct ptm_setstate {
+    union {
+        struct {
+            uint32_t state_flags; /* may be STATE_FLAG_ENCRYPTED */
+            uint32_t tpm_number;  /* always set to 0 */
+            uint8_t type;         /* which blob to set */
+            uint32_t length;
+            uint8_t data[STATE_BLOB_SIZE];
+        } req;
+        struct {
+            ptmres_t tpm_result;
+        } resp;
+    } u;
+};
+
+
+typedef uint64_t ptmcap_t;
+typedef struct ptmest  ptmest_t;
+typedef struct ptmreset_est ptmreset_est_t;
+typedef struct ptmloc  ptmloc_t;
+typedef struct ptmhdata ptmhdata_t;
+typedef struct ptminit ptminit_t;
+typedef struct ptm_getstate ptm_getstate_t;
+typedef struct ptm_setstate ptm_setstate_t;
+
+/* capability flags returned by PTM_GET_CAPABILITY */
+#define PTM_CAP_INIT               (1)
+#define PTM_CAP_SHUTDOWN           (1<<1)
+#define PTM_CAP_GET_TPMESTABLISHED (1<<2)
+#define PTM_CAP_SET_LOCALITY       (1<<3)
+#define PTM_CAP_HASHING            (1<<4)
+#define PTM_CAP_CANCEL_TPM_CMD     (1<<5)
+#define PTM_CAP_STORE_VOLATILE     (1<<6)
+#define PTM_CAP_RESET_TPMESTABLISHED (1<<7)
+#define PTM_CAP_GET_STATEBLOB      (1<<8)
+#define PTM_CAP_SET_STATEBLOB      (1<<9)
+#define PTM_CAP_STOP               (1<<10)
+
+enum {
+    PTM_GET_CAPABILITY     = _IOR('P', 0, ptmcap_t),
+    PTM_INIT               = _IOWR('P', 1, ptminit_t),
+    PTM_SHUTDOWN           = _IOR('P', 2, ptmres_t),
+    PTM_GET_TPMESTABLISHED = _IOR('P', 3, ptmest_t),
+    PTM_SET_LOCALITY       = _IOWR('P', 4, ptmloc_t),
+    PTM_HASH_START         = _IOR('P', 5, ptmres_t),
+    PTM_HASH_DATA          = _IOWR('P', 6, ptmhdata_t),
+    PTM_HASH_END           = _IOR('P', 7, ptmres_t),
+    PTM_CANCEL_TPM_CMD     = _IOR('P', 8, ptmres_t),
+    PTM_STORE_VOLATILE     = _IOR('P', 9, ptmres_t),
+    PTM_RESET_TPMESTABLISHED = _IOWR('P', 10, ptmreset_est_t),
+    PTM_GET_STATEBLOB      = _IOWR('P', 11, ptm_getstate_t),
+    PTM_SET_STATEBLOB      = _IOWR('P', 12, ptm_setstate_t),
+    PTM_STOP               = _IOR('P', 13, ptmres_t),
+};
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 8d8523a..26abfc9 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -34,6 +34,7 @@
 #include "sysemu/tpm_backend_int.h"
 #include "tpm_tis.h"
 #include "tpm_util.h"
+#include "tpm_ioctl.h"
 
 #define DEBUG_TPM 0
 
@@ -46,6 +47,7 @@
 #define TYPE_TPM_PASSTHROUGH "tpm-passthrough"
 #define TPM_PASSTHROUGH(obj) \
     OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH)
+#define TYPE_TPM_CUSE "tpm-cuse"
 
 static const TPMDriverOps tpm_passthrough_driver;
 
@@ -72,12 +74,18 @@ struct TPMPassthruState {
     bool had_startup_error;
 
     TPMVersion tpm_version;
+    ptmcap_t cuse_cap; /* capabilties of the CUSE TPM */
+    uint8_t cur_locty_number; /* last set locality */
 };
 
 typedef struct TPMPassthruState TPMPassthruState;
 
 #define TPM_PASSTHROUGH_DEFAULT_DEVICE "/dev/tpm0"
 
+#define TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt) (tpm_pt->cuse_cap != 0)
+
+#define TPM_CUSE_IMPLEMENTS(tpm_tr, cap) ((tpm_pt->cuse_cap & cap) == cap)
+
 /* functions */
 
 static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
@@ -124,7 +132,30 @@ static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len)
     return false;
 }
 
+static int tpm_passthrough_set_locality(TPMPassthruState *tpm_pt,
+                                        uint8_t locty_number)
+{
+    int n;
+    ptmloc_t loc;
+
+    if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+        if (tpm_pt->cur_locty_number != locty_number) {
+            loc.u.req.loc = locty_number;
+            n = ioctl(tpm_pt->tpm_fd, PTM_SET_LOCALITY, &loc);
+            if (n < 0) {
+                error_report("tpm_cuse: could not set locality on "
+                             "CUSE TPM: %s (%i)",
+                             strerror(errno), errno);
+                return -1;
+            }
+            tpm_pt->cur_locty_number = locty_number;
+        }
+    }
+    return 0;
+}
+
 static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
+                                        uint8_t locality_number,
                                         const uint8_t *in, uint32_t in_len,
                                         uint8_t *out, uint32_t out_len,
                                         bool *selftest_done)
@@ -133,6 +164,11 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
     bool is_selftest;
     const struct tpm_resp_hdr *hdr;
 
+    ret = tpm_passthrough_set_locality(tpm_pt, locality_number);
+    if (ret < 0) {
+        goto err_exit;
+    }
+
     tpm_pt->tpm_op_canceled = false;
     tpm_pt->tpm_executing = true;
     *selftest_done = false;
@@ -183,10 +219,12 @@ err_exit:
 }
 
 static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt,
+                                         uint8_t locality_number,
                                          const TPMLocality *locty_data,
                                          bool *selftest_done)
 {
     return tpm_passthrough_unix_tx_bufs(tpm_pt,
+                                        locality_number,
                                         locty_data->w_buffer.buffer,
                                         locty_data->w_offset,
                                         locty_data->r_buffer.buffer,
@@ -207,6 +245,7 @@ static void tpm_passthrough_worker_thread(gpointer data,
     switch (cmd) {
     case TPM_BACKEND_CMD_PROCESS_CMD:
         tpm_passthrough_unix_transfer(tpm_pt,
+                                      thr_parms->tpm_state->locty_number,
                                       thr_parms->tpm_state->locty_data,
                                       &selftest_done);
 
@@ -223,6 +262,94 @@ static void tpm_passthrough_worker_thread(gpointer data,
 }
 
 /*
+ * Gracefully shut down the external CUSE TPM
+ */
+static void tpm_passthrough_shutdown(TPMPassthruState *tpm_pt)
+{
+    int n;
+    ptmres_t res;
+
+    if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+        n = ioctl(tpm_pt->tpm_fd, PTM_SHUTDOWN, &res);
+        if (n < 0) {
+            error_report("tpm_cuse: Could not cleanly shut down "
+                         "the CUSE TPM: %s (%i)",
+                         strerror(errno), errno);
+        }
+    }
+}
+
+/*
+ * Probe for the CUSE TPM by sending an ioctl() requesting its
+ * capability flags.
+ */
+static int tpm_passthrough_cuse_probe(TPMPassthruState *tpm_pt)
+{
+    int rc = 0;
+    int n;
+    ptmcap_t caps = 0;
+    const char *tpm = NULL;
+
+    n = ioctl(tpm_pt->tpm_fd, PTM_GET_CAPABILITY, &tpm_pt->cuse_cap);
+    if (n < 0) {
+        error_report("Error: CUSE TPM was requested, but probing failed.");
+        rc = -1;
+        goto err_exit;
+    }
+
+    /* check for min. required capabilities */
+    switch (tpm_pt->tpm_version) {
+    case TPM_VERSION_1_2:
+        caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
+               PTM_CAP_SET_LOCALITY;
+        tpm = "1.2";
+        break;
+    case TPM_VERSION_2_0:
+        caps = PTM_CAP_INIT | PTM_CAP_SHUTDOWN | PTM_CAP_GET_TPMESTABLISHED |
+               PTM_CAP_SET_LOCALITY | PTM_CAP_RESET_TPMESTABLISHED;
+        tpm = "2";
+        break;
+    case TPM_VERSION_UNSPEC:
+        error_report("tpm_cuse: %s: TPM version has not been set.",
+                     __func__);
+        return -1;
+    }
+
+    if (!TPM_CUSE_IMPLEMENTS(tpm_pt, caps)) {
+        error_report("tpm_cuse: TPM does not implement minimum set of required "
+                     "capabilities for TPM %s (0x%x).", tpm, (int)caps);
+        rc = -1;
+    }
+
+err_exit:
+    return rc;
+}
+
+/*
+ * Initialize the external CUSE TPM
+ */
+static int tpm_passthrough_cuse_init(TPMPassthruState *tpm_pt)
+{
+    int rc = 0;
+    int n;
+    ptminit_t init = {
+        .u.req.init_flags = INIT_FLAG_DELETE_VOLATILE,
+    };
+
+    if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+        n = ioctl(tpm_pt->tpm_fd, PTM_INIT, &init);
+        if (n < 0) {
+            error_report("tpm_cuse: Detected CUSE TPM but could not "
+                         "send INIT: %s (%i)",
+                         strerror(errno), errno);
+            rc = -1;
+        }
+    }
+
+    return rc;
+}
+
+/*
  * Start the TPM (thread). If it had been started before, then terminate
  * and start it again.
  */
@@ -237,6 +364,8 @@ static int tpm_passthrough_startup_tpm(TPMBackend *tb)
                               tpm_passthrough_worker_thread,
                               &tpm_pt->tpm_thread_params);
 
+    tpm_passthrough_cuse_init(tpm_pt);
+
     return 0;
 }
 
@@ -267,14 +396,46 @@ static int tpm_passthrough_init(TPMBackend *tb, TPMState *s,
 
 static bool tpm_passthrough_get_tpm_established_flag(TPMBackend *tb)
 {
+    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+    ptmest_t est;
+    int n;
+
+    if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+        n = ioctl(tpm_pt->tpm_fd, PTM_GET_TPMESTABLISHED, &est);
+        if (n < 0) {
+            error_report("tpm_cuse: Could not get the TPM established "
+                         "flag from the CUSE TPM: %s (%i)",
+                         strerror(errno), errno);
+            return false;
+        }
+        return (est.bit != 0);
+    }
     return false;
 }
 
 static int tpm_passthrough_reset_tpm_established_flag(TPMBackend *tb,
                                                       uint8_t locty)
 {
+    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+    int n;
+    int rc = 0;
+    ptmreset_est_t ptmreset_est;
+
     /* only a TPM 2.0 will support this */
-    return 0;
+    if (tpm_pt->tpm_version == TPM_VERSION_2_0) {
+        if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+            ptmreset_est.u.req.loc = tpm_pt->cur_locty_number;
+
+            n = ioctl(tpm_pt->tpm_fd, PTM_RESET_TPMESTABLISHED, &ptmreset_est);
+            if (n < 0) {
+                error_report("tpm_cuse: Could not reset the establishment bit "
+                             "failed: %s (%i)",
+                             strerror(errno), errno);
+                rc = -1;
+            }
+        }
+    }
+    return rc;
 }
 
 static bool tpm_passthrough_get_startup_error(TPMBackend *tb)
@@ -306,6 +467,8 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
     int n;
+    ptmres_t res;
+    static int error_printed;
 
     /*
      * As of Linux 3.7 the tpm_tis driver does not properly cancel
@@ -314,17 +477,36 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
      * command, e.g., a command executed on the host.
      */
     if (tpm_pt->tpm_executing) {
-        if (tpm_pt->cancel_fd >= 0) {
-            n = write(tpm_pt->cancel_fd, "-", 1);
-            if (n != 1) {
-                error_report("Canceling TPM command failed: %s",
-                             strerror(errno));
-            } else {
-                tpm_pt->tpm_op_canceled = true;
+        if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+            if (TPM_CUSE_IMPLEMENTS(tpm_pt, PTM_CAP_CANCEL_TPM_CMD)) {
+                n = ioctl(tpm_pt->tpm_fd, PTM_CANCEL_TPM_CMD, &res);
+                if (n < 0) {
+                    error_report("tpm_cuse: Could not cancel command on "
+                                 "CUSE TPM: %s (%i)",
+                                 strerror(errno), errno);
+                } else if (res != TPM_SUCCESS) {
+                    if (!error_printed) {
+                        error_report("TPM error code from command "
+                                     "cancellation of CUSE TPM: 0x%x", res);
+                        error_printed = true;
+                    }
+                } else {
+                    tpm_pt->tpm_op_canceled = true;
+                }
             }
         } else {
-            error_report("Cannot cancel TPM command due to missing "
-                         "TPM sysfs cancel entry");
+            if (tpm_pt->cancel_fd >= 0) {
+                n = write(tpm_pt->cancel_fd, "-", 1);
+                if (n != 1) {
+                    error_report("Canceling TPM command failed: %s",
+                                 strerror(errno));
+                } else {
+                    tpm_pt->tpm_op_canceled = true;
+                }
+            } else {
+                error_report("Cannot cancel TPM command due to missing "
+                             "TPM sysfs cancel entry");
+            }
         }
     }
 }
@@ -354,6 +536,11 @@ static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
     char *dev;
     char path[PATH_MAX];
 
+    if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+        /* not needed, but so we have a fd */
+        return qemu_open("/dev/null", O_WRONLY);
+    }
+
     if (tb->cancel_path) {
         fd = qemu_open(tb->cancel_path, O_WRONLY);
         if (fd < 0) {
@@ -388,12 +575,22 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
     const char *value;
+    bool have_cuse = false;
+
+    value = qemu_opt_get(opts, "type");
+    if (value != NULL && !strcmp("cuse-tpm", value)) {
+        have_cuse = true;
+    }
 
     value = qemu_opt_get(opts, "cancel-path");
     tb->cancel_path = g_strdup(value);
 
     value = qemu_opt_get(opts, "path");
     if (!value) {
+        if (have_cuse) {
+            error_report("Missing path to access CUSE TPM");
+            goto err_free_parameters;
+        }
         value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
     }
 
@@ -408,15 +605,26 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
         goto err_free_parameters;
     }
 
+    tpm_pt->cur_locty_number = ~0;
+
     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;
     }
 
+    if (have_cuse) {
+        if (tpm_passthrough_cuse_probe(tpm_pt)) {
+            goto err_close_tpmdev;
+        }
+    }
+
+
     return 0;
 
  err_close_tpmdev:
+    tpm_passthrough_shutdown(tpm_pt);
+
     qemu_close(tpm_pt->tpm_fd);
     tpm_pt->tpm_fd = -1;
 
@@ -467,6 +675,8 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
 
     tpm_backend_thread_end(&tpm_pt->tbt);
 
+    tpm_passthrough_shutdown(tpm_pt);
+
     qemu_close(tpm_pt->tpm_fd);
     qemu_close(tpm_pt->cancel_fd);
 
@@ -540,3 +750,44 @@ static void tpm_passthrough_register(void)
 }
 
 type_init(tpm_passthrough_register)
+
+/* CUSE TPM */
+static const char *tpm_passthrough_cuse_create_desc(void)
+{
+    return "CUSE TPM backend driver";
+}
+
+static const TPMDriverOps tpm_cuse_driver = {
+    .type                     = TPM_TYPE_CUSE_TPM,
+    .opts                     = tpm_passthrough_cmdline_opts,
+    .desc                     = tpm_passthrough_cuse_create_desc,
+    .create                   = tpm_passthrough_create,
+    .destroy                  = tpm_passthrough_destroy,
+    .init                     = tpm_passthrough_init,
+    .startup_tpm              = tpm_passthrough_startup_tpm,
+    .realloc_buffer           = tpm_passthrough_realloc_buffer,
+    .reset                    = tpm_passthrough_reset,
+    .had_startup_error        = tpm_passthrough_get_startup_error,
+    .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 const TypeInfo tpm_cuse_info = {
+    .name = TYPE_TPM_CUSE,
+    .parent = TYPE_TPM_BACKEND,
+    .instance_size = sizeof(TPMPassthruState),
+    .class_init = tpm_passthrough_class_init,
+    .instance_init = tpm_passthrough_inst_init,
+    .instance_finalize = tpm_passthrough_inst_finalize,
+};
+
+static void tpm_cuse_register(void)
+{
+    type_register_static(&tpm_cuse_info);
+    tpm_register_driver(&tpm_cuse_driver);
+}
+
+type_init(tpm_cuse_register)
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..0d7c7b6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2974,10 +2974,11 @@
 # An enumeration of TPM types
 #
 # @passthrough: TPM passthrough type
+# @cuse-tpm: CUSE TPM type
 #
 # Since: 1.5
 ##
-{ 'enum': 'TpmType', 'data': [ 'passthrough' ] }
+{ 'enum': 'TpmType', 'data': [ 'passthrough', 'cuse-tpm' ] }
 
 ##
 # @query-tpm-types:
@@ -3006,6 +3007,17 @@
                                              '*cancel-path' : 'str'} }
 
 ##
+# @TPMCuseOptions:
+#
+# Information about the CUSE TPM type
+#
+# @path: string describing the path used for accessing the TPM device
+#
+# Since: 2.4
+##
+{ 'type': 'TPMCuseOptions', 'data': { 'path' : 'str'}}
+
+##
 # @TpmTypeOptions:
 #
 # A union referencing different TPM backend types' configuration options
@@ -3015,7 +3027,8 @@
 # Since: 1.5
 ##
 { 'union': 'TpmTypeOptions',
-   'data': { 'passthrough' : 'TPMPassthroughOptions' } }
+   'data': { 'passthrough' : 'TPMPassthroughOptions',
+             'cuse-tpm' : 'TPMCuseOptions' } }
 
 ##
 # @TpmInfo:
diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..65d4ea0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2539,7 +2539,10 @@ DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
     "-tpmdev passthrough,id=id[,path=path][,cancel-path=path]\n"
     "                use path to provide path to a character device; default is /dev/tpm0\n"
     "                use cancel-path to provide path to TPM's cancel sysfs entry; if\n"
-    "                not provided it will be searched for in /sys/class/misc/tpm?/device\n",
+    "                not provided it will be searched for in /sys/class/misc/tpm?/device\n"
+    "-tpmdev cuse-tpm,id=id,path=path\n"
+    "                use path to provide path to a character device to talk to the\n"
+    "                TPM emulator providing a CUSE interface\n",
     QEMU_ARCH_ALL)
 STEXI
 
@@ -2548,8 +2551,8 @@ The general form of a TPM device option is:
 
 @item -tpmdev @var{backend} ,id=@var{id} [,@var{options}]
 @findex -tpmdev
-Backend type must be:
-@option{passthrough}.
+Backend type must be either one of the following:
+@option{passthrough}, @option{cuse-tpm}.
 
 The specific backend type will determine the applicable options.
 The @code{-tpmdev} option creates the TPM backend and requires a
@@ -2599,6 +2602,18 @@ To create a passthrough TPM use the following two options:
 Note that the @code{-tpmdev} id is @code{tpm0} and is referenced by
 @code{tpmdev=tpm0} in the device option.
 
+@item -tpmdev cuse-tpm, id=@var{id}, path=@var{path}
+
+(Linux-host only) Enable access to a TPM emulator with a CUSE interface.
+
+@option{path} specifies the path to the CUSE TPM character device.
+
+To create a backend device accessing the CUSE TPM emulator using /dev/vtpm
+use the following two options:
+@example
+-tpmdev cuse-tpm,id=tpm0,path=/dev/vtpm -device tpm-tis,tpmdev=tpm0
+@end example
+
 @end table
 
 ETEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3a42ad0..a1f45b1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3494,7 +3494,7 @@ Arguments: None
 Example:
 
 -> { "execute": "query-tpm-types" }
-<- { "return": [ "passthrough" ] }
+<- { "return": [ "passthrough", "cuse-tpm" ] }
 
 EQMP
 
diff --git a/tpm.c b/tpm.c
index 963b7ee..5443b7b 100644
--- a/tpm.c
+++ b/tpm.c
@@ -25,7 +25,7 @@ static QLIST_HEAD(, TPMBackend) tpm_backends =
 
 
 #define TPM_MAX_MODELS      1
-#define TPM_MAX_DRIVERS     1
+#define TPM_MAX_DRIVERS     2
 
 static TPMDriverOps const *be_drivers[TPM_MAX_DRIVERS] = {
     NULL,
@@ -273,6 +273,15 @@ static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv)
             tpo->has_cancel_path = true;
         }
         break;
+    case TPM_TYPE_CUSE_TPM:
+        res->options->kind = TPM_TYPE_OPTIONS_KIND_CUSE_TPM;
+        tpo = g_new0(TPMPassthroughOptions, 1);
+        res->options->passthrough = tpo;
+        if (drv->path) {
+            tpo->path = g_strdup(drv->path);
+            tpo->has_path = true;
+        }
+        break;
     case TPM_TYPE_MAX:
         break;
     }
-- 
1.9.3

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

* [Qemu-devel] [PATCH 2/5] Support Physical Presence Interface Spec
  2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 1/5] Provide support for the CUSE TPM Stefan Berger
@ 2015-04-15 22:38 ` Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 3/5] Introduce condition to notifiy waiters of completed command Stefan Berger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-15 22:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: safford, Kevin O'Connor, Stefan Berger, quan.xu, mst

For automated management of a TPM device, implement the TCG Physical Presence
Interface Specification that allows a root user on Linux (for example) to set
an opcode for a sequence of TPM operations that the BIOS is supposed to execute
upon reboot of the physical or virtual machine. A sequence of operations may for
example involve giving up ownership of the TPM and activating and enabling the
device.

The sequences of operations are defined in table 2 in the specs to be found
at the following link:

http://www.trustedcomputinggroup.org/resources/tcg_physical_presence_interface_specification

As an example, in recent versions of Linux the opcode (5) can be set as
follows:

cd /sys/devices/pnp0/00\:04/ppi

echo 5 > request

This ACPI implementation assumes that the underlying firmware (SeaBIOS)
has 'thrown an anchor' into the f-segment. The anchor is identified by
two signatures (TCG_MAGIC) surrounding a 64bit pointer. The structure
in the f-segment is write-protected and holds a pointer to a structure
in high memmory area where the ACPI code writes the opcode into and
where it can read the last response from the BIOS.

The supported opcodes are 1-11, 14, and 21-22. (see table 2 in spec)
Also '0' is supported to 'clear' an intention.


Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Michael Tsirkin <mst@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
---
 hw/i386/acpi-tpm-core.dsl | 277 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-tpm2.dsl     |  27 +++++
 hw/i386/q35-acpi-dsdt.dsl |   1 +
 hw/i386/ssdt-tpm.dsl      |  12 +-
 4 files changed, 306 insertions(+), 11 deletions(-)
 create mode 100644 hw/i386/acpi-tpm-core.dsl
 create mode 100644 hw/i386/acpi-tpm2.dsl

diff --git a/hw/i386/acpi-tpm-core.dsl b/hw/i386/acpi-tpm-core.dsl
new file mode 100644
index 0000000..20fdee4
--- /dev/null
+++ b/hw/i386/acpi-tpm-core.dsl
@@ -0,0 +1,277 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * Note: This implementation follows the TCG specification
+ * 'TCG Physical Presence Interface Specification' of which the latest
+ * version can be found here:
+ * http://www.trustedcomputinggroup.org/resources/tcg_physical_presence_interface_specification
+ */
+
+#include "hw/acpi/tpm.h"
+
+#define TCG_MAGIC 0x41504354
+
+/*
+ * common code for TPM SSDT and TPM2
+ */
+Name (_HID, EisaID ("PNP0C31"))
+Name (_CRS, ResourceTemplate ()
+{
+    Memory32Fixed (ReadWrite, TPM_TIS_ADDR_BASE, TPM_TIS_ADDR_SIZE)
+    // older Linux tpm_tis drivers do not work with IRQ
+    //IRQNoFlags () {TPM_TIS_IRQ}
+})
+
+Method (_STA, 0, NotSerialized) {
+    Return (0x0F)
+}
+
+OperationRegion (TTIS, SystemMemory,
+                 TPM_TIS_ADDR_BASE, TPM_TIS_ADDR_SIZE)
+
+// Define TPM Debug register
+Field(TTIS, AnyAcc, NoLock, Preserve) {
+    Offset (0xf90),
+    TDBG, 32        // QEMU TIS Debug
+}
+
+// Last accepted opcode
+NAME(OP, Zero)
+
+// Field to access f-segment
+OperationRegion (FSEG, SystemMemory, 0xf0000, 0x10000)
+Field(FSEG, WordAcc, NoLock, Preserve) {
+   FDAT, 0x80000
+}
+
+// Read a 32bit word from memory; this function
+// is used for accessing the bitfield in FDAT
+Method (RD32, 2, NotSerialized) {
+    Store(DerefOf(Index(Arg0, Arg1)), Local0)
+    Increment(Arg1)
+    Store(DerefOf(Index(Arg0, Arg1)), Local1)
+    Increment(Arg1)
+    Store(DerefOf(Index(Arg0, Arg1)), Local2)
+    Increment(Arg1)
+    Store(DerefOf(Index(Arg0, Arg1)), Local3)
+    Or(Local0, ShiftLeft(Local1,  8), Local0)
+    Or(Local0, ShiftLeft(Local2, 16), Local0)
+    Or(Local0, ShiftLeft(Local3, 24), Local0)
+    Return (Local0)
+}
+
+// The address in high memory where we exchange
+// info with the BIOS
+Name(ADDR, 0x0)
+
+// Scan the f-segment for our anchor with
+// signature TCG_MAGIC
+Method (SCAN, 0, NotSerialized) {
+    Store( Zero, Local1)
+    While (LLess(Local1, 0x10000)) {
+        Store(DerefOf(Index(FDAT, Local1)), Local0)
+        // first scan for single byte
+        If (LEqual(Local0, 0x54)) {
+           // Check two two 4-byte signatures
+            If (LAnd(
+                 LEqual(RD32(FDAT,     Local1    ), TCG_MAGIC),
+                 LEqual(RD32(FDAT, Add(Local1, 8)), TCG_MAGIC)
+               )
+            ) {
+               //Store ( RD32(FDAT, Add(Local1, 4)), TDBG)
+               Store ( RD32(FDAT, Add(Local1, 4)), ADDR)
+               Break
+            }
+        }
+        Add(Local1, 16, Local1)
+    }
+}
+
+// Write given opcode into RAM
+Method (WRAM, 1, NotSerialized) {
+    If (LEqual(ADDR, 0x0)) {
+        // find high memory reserved by BIOS; this will
+        // set ADDR
+        SCAN( )
+    }
+    If (LNotEqual(ADDR, 0x0)) {
+        // Write to high memory pointed to by ADDR
+        OperationRegion (HIGH, SystemMemory, ADDR, 0x10)
+        Field(HIGH, AnyAcc, NoLock, Preserve) {
+           SIG1, 32,
+           SIZE, 16,
+           CODE, 8
+        }
+        If (LAnd(
+            LEqual(SIG1, TCG_MAGIC),
+            LGreaterEqual(SIZE, 1))
+        ) {
+            // Remember last opcode in CODE
+            Store(Arg0, CODE)
+            // Write opcode for BIOS to find
+            Store(Arg0, OP)
+            Return ( 0 )
+        }
+    }
+    Return ( 1 )
+}
+
+
+Method (RRAM, 0, NotSerialized) {
+    Name (OPRE, Package(3) { 1, 0, 0})
+    If (LEqual(ADDR, 0x0)) {
+        // find high memory reserved by BIOS; this will
+        // set ADDR
+        SCAN( )
+    }
+    If (LNotEqual(ADDR, 0x0)) {
+        // Read from memory pointed to by ADDR
+        OperationRegion (HIGH, SystemMemory, ADDR, 0x10)
+        Field(HIGH, AnyAcc, NoLock, Preserve) {
+           SIG1, 32,
+           SIZE, 16,
+           CODE, 8,
+           SUCC, 8,
+           CODO, 8,
+           RESP, 32
+        }
+        // Check signature and sufficient space
+        If (LAnd(
+            LEqual(SIG1, TCG_MAGIC),
+            LGreaterEqual(SIZE, 7)
+        )) {
+            Store(SUCC, Index(OPRE, 0))
+            Store(CODO, Index(OPRE, 1))
+            Store(RESP, Index(OPRE, 2))
+        }
+    }
+    return (OPRE)
+}
+
+Method (_DSM, 4, NotSerialized) {
+    If (LEqual (Arg0, ToUUID("3DDDFAA6-361B-4EB4-A424-8D10089D1653"))) {
+
+        // only supporting API revision 1
+        If (LNotEqual (Arg1, 1)) {
+            Return (Buffer (1) {0})
+        }
+
+        Store(ToInteger(Arg2), Local0)
+        // standard DSM query function
+        If (LEqual (Local0, 0)) {
+            Return (Buffer () {0xFF, 0x01})
+        }
+
+        // interface version
+        If (LEqual (Local0, 1)) {
+            Return ("1.2")
+        }
+
+        // submit TPM operation
+        If (LEqual (Local0, 2)) {
+            // get opcode from package
+            Store(DerefOf(Index(Arg3, 0)), Local0)
+            // check for supported opcode
+            // supported opcodes: 0, 1-11, 14, 21-22
+            If (LOr(
+                  LOr(
+                    LAnd(LGreaterEqual(Local0, 0),
+                         LLessEqual(Local0, 11)),
+                    LEqual(Local0, 14)
+                  ),
+                    LAnd(LGreaterEqual(Local0, 21),
+                         LLessEqual(Local0, 22))
+                  )
+               ) {
+                // Write the OP into TPM NVRAM
+                Store(WRAM ( Local0 ), Local1)
+                return (Local1)
+            } else {
+                Return (1)
+            }
+        }
+
+        // get pending TPM operation
+        If (LEqual (Local0, 3)) {
+            NAME(PEOP, Package(2) { 0, 0 })
+
+            Store ( 0 , Index(PEOP, 0))
+            Store ( OP, Index(PEOP, 1))
+
+            Return (PEOP)
+        }
+
+        // action to transition to pre-OS env.
+        If (LEqual (Local0, 4)) {
+            return (2) // Requiring reboot
+        }
+
+        // get pre-OS TPM operation response
+        If (LEqual (Local0, 5)) {
+            Store (RRAM(), Local0)
+            return ( Local0 )
+        }
+
+        // preferred user language
+        If (LEqual (Local0, 6)) {
+            return (3) // Not implemented
+        }
+
+        // submit TPM operation v2
+        If (LEqual (Local0, 7)) {
+            Store(DerefOf(Index(Arg3, 0)), Local0)
+            // supported opcodes: 0, 1-11, 14, 21-22
+            If (LOr(
+                  LOr(
+                    LAnd(LGreaterEqual(Local0, 0),
+                         LLessEqual(Local0, 11)),
+                    LEqual(Local0, 14)
+                  ),
+                  LAnd(LGreaterEqual(Local0, 21),
+                  LLessEqual(Local0, 22))
+           )
+            ) {
+                // Write the OP into TPM NVRAM
+                Store(WRAM ( Local0 ), Local1)
+                return (Local1)
+            } else {
+                Return (1)
+            }
+        }
+
+        // get user confirmation status
+        If (LEqual (Local0, 8)) {
+            Store(DerefOf(Index(Arg3,0)), Local0)
+            // supported opcodes: 0, 1-11, 14, 21-22
+            If (LOr(
+                  LOr(
+                     LAnd(LGreaterEqual(Local0, 0),
+                          LLessEqual(Local0, 11)),
+                     LEqual(Local0, 14)
+                  ),
+                LAnd(LGreaterEqual(Local0, 21),
+                     LLessEqual(Local0, 22))
+                )
+            ) {
+                Return (4)  // allowed, no user required
+            } else {
+                Return (0)  // not implemented
+            }
+        }
+    }
+    return (Buffer() { 0x0 })
+}
+
diff --git a/hw/i386/acpi-tpm2.dsl b/hw/i386/acpi-tpm2.dsl
new file mode 100644
index 0000000..3d31125
--- /dev/null
+++ b/hw/i386/acpi-tpm2.dsl
@@ -0,0 +1,27 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * Note: This implementation follows the TCG specification
+ * 'TCG Physical Presence Interface Specification' of which the latest
+ * version can be found here:
+ * http://www.trustedcomputinggroup.org/resources/tcg_physical_presence_interface_specification
+ */
+
+Scope(\_SB) {
+    Device(TPM2) {
+#include "acpi-tpm-core.dsl"
+    }
+}
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index 16eaca3..a73035d 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -115,6 +115,7 @@ DefinitionBlock (
     }
 
 #include "acpi-dsdt-hpet.dsl"
+#include "acpi-tpm2.dsl"
 
 
 /****************************************************************
diff --git a/hw/i386/ssdt-tpm.dsl b/hw/i386/ssdt-tpm.dsl
index 75d9691..32d268c 100644
--- a/hw/i386/ssdt-tpm.dsl
+++ b/hw/i386/ssdt-tpm.dsl
@@ -12,7 +12,6 @@
  * 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_tpm_aml
 
@@ -28,16 +27,7 @@ DefinitionBlock (
     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)
-                // older Linux tpm_tis drivers do not work with IRQ
-                //IRQNoFlags () {TPM_TIS_IRQ}
-            })
-            Method (_STA, 0, NotSerialized) {
-                Return (0x0F)
-            }
+#include "acpi-tpm-core.dsl"
         }
     }
 }
-- 
1.9.3

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

* [Qemu-devel] [PATCH 3/5] Introduce condition to notifiy waiters of completed command
  2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 1/5] Provide support for the CUSE TPM Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 2/5] Support Physical Presence Interface Spec Stefan Berger
@ 2015-04-15 22:38 ` Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 4/5] Introduce condition in TPM backend for notification Stefan Berger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-15 22:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: safford, Stefan Berger, quan.xu, mst

Introduce a lock and a condition to notify anyone waiting for the completion
of the execution of a TPM command by the backend (thread). The backend
uses the condition to signal anyone waiting for command completion.
We need to place the condition in two locations: one is invoked by the
backend thread, the other by the bottom half thread.
We will use the signalling to wait for command completion before VM
suspend.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_int.h |  3 +++
 hw/tpm/tpm_tis.c | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index 6b2c9c9..70be1ad 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -30,6 +30,9 @@ struct TPMState {
     char *backend;
     TPMBackend *be_driver;
     TPMVersion be_tpm_version;
+
+    QemuMutex state_lock;
+    QemuCond cmd_complete;
 };
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index be7e61f..9a3a35b 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -366,6 +366,8 @@ static void tpm_tis_receive_bh(void *opaque)
     TPMTISEmuState *tis = &s->s.tis;
     uint8_t locty = s->locty_number;
 
+    qemu_mutex_lock(&s->state_lock);
+
     tpm_tis_sts_set(&tis->loc[locty],
                     TPM_TIS_STS_VALID | TPM_TIS_STS_DATA_AVAILABLE);
     tis->loc[locty].state = TPM_TIS_STATE_COMPLETION;
@@ -382,6 +384,10 @@ static void tpm_tis_receive_bh(void *opaque)
     tpm_tis_raise_irq(s, locty,
                       TPM_TIS_INT_DATA_AVAILABLE | TPM_TIS_INT_STS_VALID);
 #endif
+
+    /* notify of completed command */
+    qemu_cond_signal(&s->cmd_complete);
+    qemu_mutex_unlock(&s->state_lock);
 }
 
 /*
@@ -401,6 +407,11 @@ static void tpm_tis_receive_cb(TPMState *s, uint8_t locty,
         }
     }
 
+    qemu_mutex_lock(&s->state_lock);
+    /* notify of completed command */
+    qemu_cond_signal(&s->cmd_complete);
+    qemu_mutex_unlock(&s->state_lock);
+
     qemu_bh_schedule(tis->bh);
 }
 
@@ -1071,6 +1082,9 @@ static void tpm_tis_initfn(Object *obj)
     memory_region_init_io(&s->mmio, OBJECT(s), &tpm_tis_memory_ops,
                           s, "tpm-tis-mmio",
                           TPM_TIS_NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT);
+
+    qemu_mutex_init(&s->state_lock);
+    qemu_cond_init(&s->cmd_complete);
 }
 
 static void tpm_tis_class_init(ObjectClass *klass, void *data)
-- 
1.9.3

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

* [Qemu-devel] [PATCH 4/5] Introduce condition in TPM backend for notification
  2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
                   ` (2 preceding siblings ...)
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 3/5] Introduce condition to notifiy waiters of completed command Stefan Berger
@ 2015-04-15 22:38 ` Stefan Berger
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 5/5] Add support for VM suspend/resume for TPM TIS Stefan Berger
  2015-04-16 13:35 ` [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Igor Mammedov
  5 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-15 22:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: safford, Stefan Berger, quan.xu, mst

TPM backends will suspend independently of the frontends. Also
here we need to be able to wait for the TPM command to have been
completely processed.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_passthrough.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 26abfc9..bbc3325 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -76,6 +76,10 @@ struct TPMPassthruState {
     TPMVersion tpm_version;
     ptmcap_t cuse_cap; /* capabilties of the CUSE TPM */
     uint8_t cur_locty_number; /* last set locality */
+
+    QemuMutex state_lock;
+    QemuCond cmd_complete;  /* singnaled once tpm_busy is false */
+    bool tpm_busy;
 };
 
 typedef struct TPMPassthruState TPMPassthruState;
@@ -252,6 +256,11 @@ static void tpm_passthrough_worker_thread(gpointer data,
         thr_parms->recv_data_callback(thr_parms->tpm_state,
                                       thr_parms->tpm_state->locty_number,
                                       selftest_done);
+        /* result delivered */
+        qemu_mutex_lock(&tpm_pt->state_lock);
+        tpm_pt->tpm_busy = false;
+        qemu_cond_signal(&tpm_pt->cmd_complete);
+        qemu_mutex_unlock(&tpm_pt->state_lock);
         break;
     case TPM_BACKEND_CMD_INIT:
     case TPM_BACKEND_CMD_END:
@@ -380,6 +389,7 @@ static void tpm_passthrough_reset(TPMBackend *tb)
     tpm_backend_thread_end(&tpm_pt->tbt);
 
     tpm_pt->had_startup_error = false;
+    tpm_pt->tpm_busy = false;
 }
 
 static int tpm_passthrough_init(TPMBackend *tb, TPMState *s,
@@ -460,6 +470,11 @@ static void tpm_passthrough_deliver_request(TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
 
+    /* TPM considered busy once TPM Request scheduled for processing */
+    qemu_mutex_lock(&tpm_pt->state_lock);
+    tpm_pt->tpm_busy = true;
+    qemu_mutex_unlock(&tpm_pt->state_lock);
+
     tpm_backend_thread_deliver_request(&tpm_pt->tbt);
 }
 
@@ -721,6 +736,11 @@ static const TPMDriverOps tpm_passthrough_driver = {
 
 static void tpm_passthrough_inst_init(Object *obj)
 {
+    TPMBackend *tb = TPM_BACKEND(obj);
+    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+
+    qemu_mutex_init(&tpm_pt->state_lock);
+    qemu_cond_init(&tpm_pt->cmd_complete);
 }
 
 static void tpm_passthrough_inst_finalize(Object *obj)
-- 
1.9.3

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

* [Qemu-devel] [PATCH 5/5] Add support for VM suspend/resume for TPM TIS
  2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
                   ` (3 preceding siblings ...)
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 4/5] Introduce condition in TPM backend for notification Stefan Berger
@ 2015-04-15 22:38 ` Stefan Berger
  2015-04-16 13:35 ` [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Igor Mammedov
  5 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-15 22:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: safford, Stefan Berger, quan.xu, mst

Extend the TPM TIS code to support suspend/resume. In case a command
is being processed by the external TPM when suspending, wait for the command
to complete to catch the result. In case the bottom half did not run,
run the one function the bottom half is supposed to run. This then
makes the resume operation work.

The passthrough backend does not support suspend/resume operation
and is therefore blocked from suspend/resume and migration.

The CUSE TPM's supported capabilities are tested and if sufficient
capabilities are implemented, suspend/resume, snapshotting and
migration are supported by the CUSE TPM.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_passthrough.c     | 129 +++++++++++++++++++++++++--
 hw/tpm/tpm_tis.c             | 138 ++++++++++++++++++++++++++++-
 hw/tpm/tpm_tis.h             |   2 +
 hw/tpm/tpm_util.c            | 206 +++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_util.h            |   7 ++
 include/sysemu/tpm_backend.h |  12 +++
 6 files changed, 488 insertions(+), 6 deletions(-)

diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index bbc3325..2decf0d 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -35,6 +35,7 @@
 #include "tpm_tis.h"
 #include "tpm_util.h"
 #include "tpm_ioctl.h"
+#include "migration/migration.h"
 
 #define DEBUG_TPM 0
 
@@ -50,6 +51,7 @@
 #define TYPE_TPM_CUSE "tpm-cuse"
 
 static const TPMDriverOps tpm_passthrough_driver;
+static const VMStateDescription vmstate_tpm_cuse;
 
 /* data structures */
 typedef struct TPMPassthruThreadParams {
@@ -80,6 +82,10 @@ struct TPMPassthruState {
     QemuMutex state_lock;
     QemuCond cmd_complete;  /* singnaled once tpm_busy is false */
     bool tpm_busy;
+
+    Error *migration_blocker;
+
+    TPMBlobBuffers tpm_blobs;
 };
 
 typedef struct TPMPassthruState TPMPassthruState;
@@ -286,6 +292,10 @@ static void tpm_passthrough_shutdown(TPMPassthruState *tpm_pt)
                          strerror(errno), errno);
         }
     }
+    if (tpm_pt->migration_blocker) {
+        migrate_del_blocker(tpm_pt->migration_blocker);
+        error_free(tpm_pt->migration_blocker);
+    }
 }
 
 /*
@@ -337,13 +347,15 @@ err_exit:
 /*
  * Initialize the external CUSE TPM
  */
-static int tpm_passthrough_cuse_init(TPMPassthruState *tpm_pt)
+static int tpm_passthrough_cuse_init(TPMPassthruState *tpm_pt,
+                                     bool is_resume)
 {
     int rc = 0;
     int n;
-    ptminit_t init = {
-        .u.req.init_flags = INIT_FLAG_DELETE_VOLATILE,
-    };
+    ptminit_t init;
+    if (is_resume) {
+        init.u.req.init_flags = INIT_FLAG_DELETE_VOLATILE;
+    }
 
     if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
         n = ioctl(tpm_pt->tpm_fd, PTM_INIT, &init);
@@ -373,7 +385,7 @@ static int tpm_passthrough_startup_tpm(TPMBackend *tb)
                               tpm_passthrough_worker_thread,
                               &tpm_pt->tpm_thread_params);
 
-    tpm_passthrough_cuse_init(tpm_pt);
+    tpm_passthrough_cuse_init(tpm_pt, false);
 
     return 0;
 }
@@ -448,6 +460,34 @@ static int tpm_passthrough_reset_tpm_established_flag(TPMBackend *tb,
     return rc;
 }
 
+static int tpm_cuse_get_state_blobs(TPMBackend *tb,
+                                    bool decrypted_blobs,
+                                    TPMBlobBuffers *tpm_blobs)
+{
+    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+
+    assert(TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt));
+
+    return tpm_util_cuse_get_state_blobs(tpm_pt->tpm_fd, decrypted_blobs,
+                                         tpm_blobs);
+}
+
+static int tpm_cuse_set_state_blobs(TPMBackend *tb,
+                                    TPMBlobBuffers *tpm_blobs)
+{
+    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+    int n;
+
+    assert(TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt));
+
+    n = tpm_util_cuse_set_state_blobs(tpm_pt->tpm_fd, tpm_blobs);
+    if (n) {
+        return 1;
+    }
+
+    return tpm_passthrough_cuse_init(tpm_pt, true);
+}
+
 static bool tpm_passthrough_get_startup_error(TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -586,6 +626,25 @@ static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
     return fd;
 }
 
+static void tpm_passthrough_block_migration(TPMPassthruState *tpm_pt)
+{
+    ptmcap_t caps;
+
+    if (TPM_PASSTHROUGH_USES_CUSE_TPM(tpm_pt)) {
+        caps = PTM_CAP_GET_STATEBLOB | PTM_CAP_SET_STATEBLOB |
+               PTM_CAP_STOP;
+        if (!TPM_CUSE_IMPLEMENTS(tpm_pt, caps)) {
+            error_setg(&tpm_pt->migration_blocker,
+                       "Migration disabled: CUSE TPM lacks necessary capabilities.");
+            migrate_add_blocker(tpm_pt->migration_blocker);
+        }
+    } else {
+        error_setg(&tpm_pt->migration_blocker,
+                   "Migration disabled: Passthrough TPM does not support migration.");
+        migrate_add_blocker(tpm_pt->migration_blocker);
+    }
+}
+
 static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -634,6 +693,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
         }
     }
 
+    tpm_passthrough_block_migration(tpm_pt);
 
     return 0;
 
@@ -741,10 +801,13 @@ static void tpm_passthrough_inst_init(Object *obj)
 
     qemu_mutex_init(&tpm_pt->state_lock);
     qemu_cond_init(&tpm_pt->cmd_complete);
+
+    vmstate_register(NULL, -1, &vmstate_tpm_cuse, obj);
 }
 
 static void tpm_passthrough_inst_finalize(Object *obj)
 {
+    vmstate_unregister(NULL, &vmstate_tpm_cuse, obj);
 }
 
 static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
@@ -777,6 +840,62 @@ static const char *tpm_passthrough_cuse_create_desc(void)
     return "CUSE TPM backend driver";
 }
 
+static void tpm_cuse_pre_save(void *opaque)
+{
+    TPMPassthruState *tpm_pt = opaque;
+    TPMBackend *tb = &tpm_pt->parent;
+
+     qemu_mutex_lock(&tpm_pt->state_lock);
+     /* wait for TPM to finish processing */
+     if (tpm_pt->tpm_busy) {
+        qemu_cond_wait(&tpm_pt->cmd_complete, &tpm_pt->state_lock);
+     }
+     qemu_mutex_unlock(&tpm_pt->state_lock);
+
+fprintf(stderr, ">>>>> %s\n", __func__);
+    /* get the decrypted state blobs from the TPM */
+    tpm_cuse_get_state_blobs(tb, TRUE, &tpm_pt->tpm_blobs);
+}
+
+static int tpm_cuse_post_load(void *opaque,
+                              int version_id __attribute__((unused)))
+{
+    TPMPassthruState *tpm_pt = opaque;
+    TPMBackend *tb = &tpm_pt->parent;
+
+fprintf(stderr, ">>>>> %s\n", __func__);
+    return tpm_cuse_set_state_blobs(tb, &tpm_pt->tpm_blobs);
+}
+
+static const VMStateDescription vmstate_tpm_cuse = {
+    .name = "cuse-tpm",
+    .version_id = 1,
+    .minimum_version_id = 0,
+    .minimum_version_id_old = 0,
+    .pre_save  = tpm_cuse_pre_save,
+    .post_load = tpm_cuse_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(tpm_blobs.permanent_flags, TPMPassthruState),
+        VMSTATE_UINT32(tpm_blobs.permanent.size, TPMPassthruState),
+        VMSTATE_VBUFFER_ALLOC_UINT32(tpm_blobs.permanent.buffer,
+                                     TPMPassthruState, 1, NULL, 0,
+                                     tpm_blobs.permanent.size),
+
+        VMSTATE_UINT32(tpm_blobs.volatil_flags, TPMPassthruState),
+        VMSTATE_UINT32(tpm_blobs.volatil.size, TPMPassthruState),
+        VMSTATE_VBUFFER_ALLOC_UINT32(tpm_blobs.volatil.buffer,
+                                     TPMPassthruState, 1, NULL, 0,
+                                     tpm_blobs.volatil.size),
+
+        VMSTATE_UINT32(tpm_blobs.savestate_flags, TPMPassthruState),
+        VMSTATE_UINT32(tpm_blobs.savestate.size, TPMPassthruState),
+        VMSTATE_VBUFFER_ALLOC_UINT32(tpm_blobs.savestate.buffer,
+                                     TPMPassthruState, 1, NULL, 0,
+                                     tpm_blobs.savestate.size),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const TPMDriverOps tpm_cuse_driver = {
     .type                     = TPM_TYPE_CUSE_TPM,
     .opts                     = tpm_passthrough_cmdline_opts,
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 9a3a35b..e843a3b 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -366,6 +366,8 @@ static void tpm_tis_receive_bh(void *opaque)
     TPMTISEmuState *tis = &s->s.tis;
     uint8_t locty = s->locty_number;
 
+    tis->bh_scheduled = false;
+
     qemu_mutex_lock(&s->state_lock);
 
     tpm_tis_sts_set(&tis->loc[locty],
@@ -413,6 +415,8 @@ static void tpm_tis_receive_cb(TPMState *s, uint8_t locty,
     qemu_mutex_unlock(&s->state_lock);
 
     qemu_bh_schedule(tis->bh);
+
+    tis->bh_scheduled = true;
 }
 
 /*
@@ -1029,9 +1033,141 @@ static void tpm_tis_reset(DeviceState *dev)
     tpm_tis_do_startup_tpm(s);
 }
 
+
+/* persistent state handling */
+
+static void tpm_tis_pre_save(void *opaque)
+{
+    TPMState *s = opaque;
+    TPMTISEmuState *tis = &s->s.tis;
+    uint8_t locty = tis->active_locty;
+
+    DPRINTF("tpm_tis: suspend: locty = %d : r_offset = %d, w_offset = %d\n",
+            locty, tis->loc[0].r_offset, tis->loc[0].w_offset);
+#ifdef DEBUG_TIS
+    tpm_tis_dump_state(opaque, 0);
+#endif
+fprintf(stderr, ">>>>> %s\n", __func__);
+
+    qemu_mutex_lock(&s->state_lock);
+
+    /* wait for outstanding request to complete */
+    if (TPM_TIS_IS_VALID_LOCTY(locty) &&
+        tis->loc[locty].state == TPM_TIS_STATE_EXECUTION) {
+        /*
+         * If we get here when the bh is scheduled but did not run,
+         * we won't get notified...
+         */
+        if (!tis->bh_scheduled) {
+            /* backend thread to notify us */
+            qemu_cond_wait(&s->cmd_complete, &s->state_lock);
+        }
+        if (tis->loc[locty].state == TPM_TIS_STATE_EXECUTION) {
+            /* bottom half did not run - run its function */
+            qemu_mutex_unlock(&s->state_lock);
+            tpm_tis_receive_bh(opaque);
+            qemu_mutex_lock(&s->state_lock);
+        }
+    }
+
+    qemu_mutex_unlock(&s->state_lock);
+
+    /* copy current active read or write buffer into the buffer
+       written to disk */
+    if (TPM_TIS_IS_VALID_LOCTY(locty)) {
+        switch (tis->loc[locty].state) {
+        case TPM_TIS_STATE_RECEPTION:
+            memcpy(tis->buf,
+                   tis->loc[locty].w_buffer.buffer,
+                   MIN(sizeof(tis->buf),
+                       tis->loc[locty].w_buffer.size));
+            tis->offset = tis->loc[locty].w_offset;
+        break;
+        case TPM_TIS_STATE_COMPLETION:
+            memcpy(tis->buf,
+                   tis->loc[locty].r_buffer.buffer,
+                   MIN(sizeof(tis->buf),
+                       tis->loc[locty].r_buffer.size));
+            tis->offset = tis->loc[locty].r_offset;
+        break;
+        default:
+            /* leak nothing */
+            memset(tis->buf, 0x0, sizeof(tis->buf));
+        break;
+        }
+    }
+}
+
+static int tpm_tis_post_load(void *opaque,
+                             int version_id __attribute__((unused)))
+{
+    TPMState *s = opaque;
+    TPMTISEmuState *tis = &s->s.tis;
+
+    uint8_t locty = tis->active_locty;
+
+    if (TPM_TIS_IS_VALID_LOCTY(locty)) {
+        switch (tis->loc[locty].state) {
+        case TPM_TIS_STATE_RECEPTION:
+            memcpy(tis->loc[locty].w_buffer.buffer,
+                   tis->buf,
+                   MIN(sizeof(tis->buf),
+                       tis->loc[locty].w_buffer.size));
+            tis->loc[locty].w_offset = tis->offset;
+        break;
+        case TPM_TIS_STATE_COMPLETION:
+            memcpy(tis->loc[locty].r_buffer.buffer,
+                   tis->buf,
+                   MIN(sizeof(tis->buf),
+                       tis->loc[locty].r_buffer.size));
+            tis->loc[locty].r_offset = tis->offset;
+        break;
+        default:
+        break;
+        }
+    }
+
+    DPRINTF("tpm_tis: resume : locty = %d : r_offset = %d, w_offset = %d\n",
+            locty, tis->loc[0].r_offset, tis->loc[0].w_offset);
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_locty = {
+    .name = "loc",
+    .version_id = 1,
+    .minimum_version_id = 0,
+    .minimum_version_id_old = 0,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(state, TPMLocality),
+        VMSTATE_UINT32(inte, TPMLocality),
+        VMSTATE_UINT32(ints, TPMLocality),
+        VMSTATE_UINT8(access, TPMLocality),
+        VMSTATE_UINT32(sts, TPMLocality),
+        VMSTATE_UINT32(iface_id, TPMLocality),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
 static const VMStateDescription vmstate_tpm_tis = {
     .name = "tpm",
-    .unmigratable = 1,
+    .version_id = 1,
+    .minimum_version_id = 0,
+    .minimum_version_id_old = 0,
+    .pre_save  = tpm_tis_pre_save,
+    .post_load = tpm_tis_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(s.tis.offset, TPMState),
+        VMSTATE_BUFFER(s.tis.buf, TPMState),
+        VMSTATE_UINT8(s.tis.active_locty, TPMState),
+        VMSTATE_UINT8(s.tis.aborting_locty, TPMState),
+        VMSTATE_UINT8(s.tis.next_locty, TPMState),
+
+        VMSTATE_STRUCT_ARRAY(s.tis.loc, TPMState, TPM_TIS_NUM_LOCALITIES, 1,
+                             vmstate_locty, TPMLocality),
+
+        VMSTATE_END_OF_LIST()
+    }
 };
 
 static Property tpm_tis_properties[] = {
diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index a1df41f..b7fc0ea 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -54,6 +54,8 @@ typedef struct TPMLocality {
 
 typedef struct TPMTISEmuState {
     QEMUBH *bh;
+    bool bh_scheduled; /* bh scheduled but did not run yet */
+
     uint32_t offset;
     uint8_t buf[TPM_TIS_BUFFER_MAX];
 
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index 4ace585..0f63cfc 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -21,6 +21,19 @@
 
 #include "tpm_util.h"
 #include "tpm_int.h"
+#include "tpm_ioctl.h"
+#include "qemu/error-report.h"
+
+#define DEBUG_TPM 0
+
+#define DPRINTF(fmt, ...) do { \
+    if (DEBUG_TPM) { \
+        fprintf(stderr, fmt, ## __VA_ARGS__); \
+    } \
+} while (0)
+
+
+#define min(x, y) ((x) < (y) ? (x) : (y))
 
 /*
  * A basic test of a TPM device. We expect a well formatted response header
@@ -124,3 +137,196 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
 
     return 1;
 }
+
+static void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
+{
+    g_free(tsb->buffer);
+    tsb->buffer = NULL;
+    tsb->size = 0;
+}
+
+static int tpm_util_cuse_get_state_blob(int fd,
+                                        uint8_t type,
+                                        bool decrypted_blobs,
+                                        TPMSizedBuffer *tsb,
+                                        uint32_t *flags)
+{
+    ptm_getstate_t pgs;
+    uint16_t offset = 0;
+    int n;
+    ptmres_t res;
+
+    tpm_sized_buffer_reset(tsb);
+
+    while (true) {
+        pgs.u.req.state_flags = (decrypted_blobs) ? STATE_FLAG_DECRYPTED : 0;
+        pgs.u.req.tpm_number = 0;
+        pgs.u.req.type = type;
+        pgs.u.req.offset = offset;
+
+        n = ioctl(fd, PTM_GET_STATEBLOB, &pgs);
+        if (n < 0) {
+            error_report("CUSE TPM PTM_GET_STATEBLOB ioctl failed: %s",
+                         strerror(errno));
+            goto err_exit;
+        }
+        res = pgs.u.resp.tpm_result;
+        if (res != 0 && (res & 0x800) == 0) {
+            error_report("Getting the stateblob (type %d) failed with a TPM "
+                         "error 0x%x", type, res);
+            goto err_exit;
+        }
+
+        tsb->buffer = g_realloc(tsb->buffer, tsb->size + pgs.u.resp.length);
+        memcpy(&tsb->buffer[tsb->size], pgs.u.resp.data, pgs.u.resp.length);
+        tsb->size += pgs.u.resp.length;
+
+        if (pgs.u.resp.length != sizeof(pgs.u.resp.data)) {
+            *flags = pgs.u.resp.state_flags;
+            break;
+        }
+        offset += pgs.u.resp.length;
+    }
+
+    DPRINTF("tpm_util: got state blob type %d, %d bytes, flags 0x%08x, "
+            "decrypted=%d\n", type, tsb->size, *flags, decrypted_blobs);
+
+    return 0;
+
+err_exit:
+    return 1;
+}
+
+int tpm_util_cuse_get_state_blobs(int tpm_fd,
+                                  bool decrypted_blobs,
+                                  TPMBlobBuffers *tpm_blobs)
+{
+    ptmres_t res;
+    int n;
+
+    n = ioctl(tpm_fd, PTM_STORE_VOLATILE, &res);
+    if (n < 0) {
+        error_report("tpm_passthrough: Could not save the volatile "
+                     "state of the CUSE TPM: %s (%i)",
+                     strerror(errno), errno);
+        return 1;
+    } else if (res != TPM_SUCCESS) {
+        error_report("TPM error code from saving "
+                     "volatile data of CUSE TPM: 0x%x", res);
+        return 1;
+    }
+
+    n = tpm_util_cuse_get_state_blob(tpm_fd, PTM_BLOB_TYPE_PERMANENT,
+                                     decrypted_blobs,
+                                     &tpm_blobs->permanent,
+                                     &tpm_blobs->permanent_flags);
+    if (n) {
+        return 1;
+    }
+    n = tpm_util_cuse_get_state_blob(tpm_fd, PTM_BLOB_TYPE_VOLATILE,
+                                     decrypted_blobs,
+                                     &tpm_blobs->volatil,
+                                     &tpm_blobs->volatil_flags);
+    if (n) {
+        goto exit_free_permanent;
+    }
+    n = tpm_util_cuse_get_state_blob(tpm_fd, PTM_BLOB_TYPE_SAVESTATE,
+                                     decrypted_blobs,
+                                     &tpm_blobs->savestate,
+                                     &tpm_blobs->savestate_flags);
+    if (n) {
+        goto exit_free_volatile;
+    }
+
+    return 0;
+
+exit_free_volatile:
+    tpm_sized_buffer_reset(&tpm_blobs->volatil);
+
+exit_free_permanent:
+    tpm_sized_buffer_reset(&tpm_blobs->permanent);
+
+    return 1;
+}
+
+static int tpm_util_cuse_set_state_blob(int fd,
+                                        uint8_t type,
+                                        TPMSizedBuffer *tsb,
+                                        uint32 flags)
+{
+    ptm_setstate_t pss;
+    ptmres_t res;
+    off_t offset = 0;
+    size_t to_copy;
+    int n;
+
+    while (tsb->size) {
+        pss.u.req.state_flags = flags;
+        pss.u.req.type = type;
+        pss.u.req.tpm_number = 0;
+        to_copy = min(tsb->size - offset, sizeof(pss.u.req.data));
+        memcpy(pss.u.req.data, &tsb->buffer[offset], to_copy);
+        offset += to_copy;
+        pss.u.req.length = to_copy;
+
+        n = ioctl(fd, PTM_SET_STATEBLOB, &pss);
+        if (n < 0) {
+            error_report("CUSE TPM PTM_SET_STATEBLOB ioctl failed: %s",
+                         strerror(errno));
+            goto err_exit;
+        }
+        res = pss.u.resp.tpm_result;
+        if (res != 0) {
+            error_report("Setting the stateblob (type %d) failed with a TPM "
+                         "error 0x%x", type, res);
+            goto err_exit;
+        }
+        if (to_copy < sizeof(pss.u.req.data)) {
+            break;
+        }
+    }
+
+    DPRINTF("tpm_util: set the state blob type %d, %d bytes, flags 0x%08x\n",
+            type, tsb->size, flags);
+
+    return 0;
+
+err_exit:
+    return 1;
+}
+
+int tpm_util_cuse_set_state_blobs(int tpm_fd,
+                                  TPMBlobBuffers *tpm_blobs)
+{
+    int n;
+    ptmres_t res;
+
+    n = ioctl(tpm_fd, PTM_STOP, &res);
+    if (n < 0) {
+        error_report("tpm_passthrough: Could not stop "
+                     "the CUSE TPM: %s (%i)",
+                     strerror(errno), errno);
+        return 1;
+    }
+
+    n = tpm_util_cuse_set_state_blob(tpm_fd, PTM_BLOB_TYPE_PERMANENT,
+                                     &tpm_blobs->permanent,
+                                     tpm_blobs->permanent_flags);
+    if (n) {
+        return 1;
+    }
+    n = tpm_util_cuse_set_state_blob(tpm_fd, PTM_BLOB_TYPE_VOLATILE,
+                                     &tpm_blobs->volatil,
+                                     tpm_blobs->volatil_flags);
+    if (n) {
+        return 1;
+    }
+    n = tpm_util_cuse_set_state_blob(tpm_fd, PTM_BLOB_TYPE_SAVESTATE,
+                                     &tpm_blobs->savestate,
+                                     tpm_blobs->savestate_flags);
+    if (n) {
+        return 1;
+    }
+
+    return 0;
+}
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index e7f354a..04f5afd 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -25,4 +25,11 @@
 
 int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
 
+int tpm_util_cuse_get_state_blobs(int tpm_fd,
+                                  bool decrypted_blobs,
+                                  TPMBlobBuffers *tpm_blobs);
+
+int tpm_util_cuse_set_state_blobs(int tpm_fd,
+                                  TPMBlobBuffers *tpm_blobs);
+
 #endif /* TPM_TPM_UTILS_H */
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 0a366be..92bc3e4 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -63,6 +63,18 @@ typedef struct TPMSizedBuffer {
     uint8_t  *buffer;
 } TPMSizedBuffer;
 
+/* blobs from the TPM; part of VM state when migrating */
+typedef struct TPMBlobBuffers {
+    uint32_t permanent_flags;
+    TPMSizedBuffer permanent;
+
+    uint32_t volatil_flags;
+    TPMSizedBuffer volatil;
+
+    uint32_t savestate_flags;
+    TPMSizedBuffer savestate;
+} TPMBlobBuffers;
+
 struct TPMDriverOps {
     enum TpmType type;
     const QemuOptDesc *opts;
-- 
1.9.3

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
                   ` (4 preceding siblings ...)
  2015-04-15 22:38 ` [Qemu-devel] [PATCH 5/5] Add support for VM suspend/resume for TPM TIS Stefan Berger
@ 2015-04-16 13:35 ` Igor Mammedov
  2015-04-16 14:05   ` Stefan Berger
  2015-04-16 18:55   ` Michael S. Tsirkin
  5 siblings, 2 replies; 21+ messages in thread
From: Igor Mammedov @ 2015-04-16 13:35 UTC (permalink / raw)
  To: Stefan Berger; +Cc: safford, qemu-devel, quan.xu, mst

On Wed, 15 Apr 2015 18:38:43 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> The following series of patches extends TPM support with an
> external TPM that offers a Linux CUSE (character device in userspace)
> interface. This TPM lets each VM access its own private vTPM.
> The CUSE TPM supports suspend/resume and migration. Much
> out-of-band functionality necessary to control the CUSE TPM is
> implemented using ioctl's.
> 
> The series extends the TPM support so far that most functionality of
> TPM support on a physical platform is now available to each x86 VM,
> this includes the Physical Presence Interface support that has
> its counter-part in the SeaBIOS and is implemented using ACPI.
> 
> http://www.seabios.org/pipermail/seabios/2015-March/008978.html
is it already merged?

Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
and tpm_ppi in BIOS memory?
That would simplify BIOS part a bit and significantly simplify ACPI code
as most of it is dealing with figuring out address of tpm_ppi.

> 
> 
> Stefan Berger (5):
>   Provide support for the CUSE TPM
>   Support Physical Presence Interface Spec
>   Introduce condition to notifiy waiters of completed command
>   Introduce condition in TPM backend for notification
>   Add support for VM suspend/resume for TPM TIS
> 
>  hmp.c                        |   6 +
>  hw/i386/acpi-tpm-core.dsl    | 277 +++++++++++++++++++++++++++++
>  hw/i386/acpi-tpm2.dsl        |  27 +++
>  hw/i386/q35-acpi-dsdt.dsl    |   1 +
>  hw/i386/ssdt-tpm.dsl         |  12 +-
>  hw/tpm/tpm_int.h             |   4 +
>  hw/tpm/tpm_ioctl.h           | 178 +++++++++++++++++++
>  hw/tpm/tpm_passthrough.c     | 410 +++++++++++++++++++++++++++++++++++++++++--
>  hw/tpm/tpm_tis.c             | 152 +++++++++++++++-
>  hw/tpm/tpm_tis.h             |   2 +
>  hw/tpm/tpm_util.c            | 206 ++++++++++++++++++++++
>  hw/tpm/tpm_util.h            |   7 +
>  include/sysemu/tpm_backend.h |  12 ++
>  qapi-schema.json             |  17 +-
>  qemu-options.hx              |  21 ++-
>  qmp-commands.hx              |   2 +-
>  tpm.c                        |  11 +-
>  17 files changed, 1316 insertions(+), 29 deletions(-)
>  create mode 100644 hw/i386/acpi-tpm-core.dsl
>  create mode 100644 hw/i386/acpi-tpm2.dsl
>  create mode 100644 hw/tpm/tpm_ioctl.h
> 

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-16 13:35 ` [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Igor Mammedov
@ 2015-04-16 14:05   ` Stefan Berger
  2015-04-22  7:00     ` Igor Mammedov
  2015-04-16 18:55   ` Michael S. Tsirkin
  1 sibling, 1 reply; 21+ messages in thread
From: Stefan Berger @ 2015-04-16 14:05 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On 04/16/2015 09:35 AM, Igor Mammedov wrote:
> On Wed, 15 Apr 2015 18:38:43 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> The following series of patches extends TPM support with an
>> external TPM that offers a Linux CUSE (character device in userspace)
>> interface. This TPM lets each VM access its own private vTPM.
>> The CUSE TPM supports suspend/resume and migration. Much
>> out-of-band functionality necessary to control the CUSE TPM is
>> implemented using ioctl's.
>>
>> The series extends the TPM support so far that most functionality of
>> TPM support on a physical platform is now available to each x86 VM,
>> this includes the Physical Presence Interface support that has
>> its counter-part in the SeaBIOS and is implemented using ACPI.
>>
>> http://www.seabios.org/pipermail/seabios/2015-March/008978.html
> is it already merged?

No, not yet. :-(

>
> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
> and tpm_ppi in BIOS memory?

MMIO region of what? Of the TIS? The TIS doesn't have memory locations 
'just to keep bytes' and they would be cleared upon machine reset / reboot.

The purpose of the PPI interface is to leave an opcode for the BIOS to 
act upon after a reset. So we have to write it into memory that doesn't 
get cleared upon reboot. Also, the BIOS leaves a result in memory so we 
can read the result code in the OS via sysfs entry.

I had previously tried using NVRAM of the TPM to leave that opcode (and 
result) , but this doesn't work well due to protection restrictions of 
the TPM's NVRAM locations and using the Linux TSS for example non-root 
users could then write an opcode into the NVRAM of the TPM (there are 
TPM commands to write to the TPM's NVRAM locations and tpm-tools has 
tools to write to these locations) that the machine then ends up acting 
upon without the admin of the machine wanting that. So, that's not a 
choice, either.


> That would simplify BIOS part a bit and significantly simplify ACPI code
> as most of it is dealing with figuring out address of tpm_ppi.

Wished it would, but I don't see a way to make it easier.

So the first time one looks into the sysfs ppi entries [on Linux] it may 
take a few seconds until the anchor is found. Subsequently the memory 
location is cached and operations  go a lot faster.

    Stefan

>>
>> Stefan Berger (5):
>>    Provide support for the CUSE TPM
>>    Support Physical Presence Interface Spec
>>    Introduce condition to notifiy waiters of completed command
>>    Introduce condition in TPM backend for notification
>>    Add support for VM suspend/resume for TPM TIS
>>
>>   hmp.c                        |   6 +
>>   hw/i386/acpi-tpm-core.dsl    | 277 +++++++++++++++++++++++++++++
>>   hw/i386/acpi-tpm2.dsl        |  27 +++
>>   hw/i386/q35-acpi-dsdt.dsl    |   1 +
>>   hw/i386/ssdt-tpm.dsl         |  12 +-
>>   hw/tpm/tpm_int.h             |   4 +
>>   hw/tpm/tpm_ioctl.h           | 178 +++++++++++++++++++
>>   hw/tpm/tpm_passthrough.c     | 410 +++++++++++++++++++++++++++++++++++++++++--
>>   hw/tpm/tpm_tis.c             | 152 +++++++++++++++-
>>   hw/tpm/tpm_tis.h             |   2 +
>>   hw/tpm/tpm_util.c            | 206 ++++++++++++++++++++++
>>   hw/tpm/tpm_util.h            |   7 +
>>   include/sysemu/tpm_backend.h |  12 ++
>>   qapi-schema.json             |  17 +-
>>   qemu-options.hx              |  21 ++-
>>   qmp-commands.hx              |   2 +-
>>   tpm.c                        |  11 +-
>>   17 files changed, 1316 insertions(+), 29 deletions(-)
>>   create mode 100644 hw/i386/acpi-tpm-core.dsl
>>   create mode 100644 hw/i386/acpi-tpm2.dsl
>>   create mode 100644 hw/tpm/tpm_ioctl.h
>>

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-16 13:35 ` [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Igor Mammedov
  2015-04-16 14:05   ` Stefan Berger
@ 2015-04-16 18:55   ` Michael S. Tsirkin
  2015-04-16 19:21     ` Stefan Berger
  1 sibling, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-04-16 18:55 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: safford, qemu-devel, quan.xu, Stefan Berger

On Thu, Apr 16, 2015 at 03:35:06PM +0200, Igor Mammedov wrote:
> On Wed, 15 Apr 2015 18:38:43 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> 
> > The following series of patches extends TPM support with an
> > external TPM that offers a Linux CUSE (character device in userspace)
> > interface. This TPM lets each VM access its own private vTPM.
> > The CUSE TPM supports suspend/resume and migration. Much
> > out-of-band functionality necessary to control the CUSE TPM is
> > implemented using ioctl's.
> > 
> > The series extends the TPM support so far that most functionality of
> > TPM support on a physical platform is now available to each x86 VM,
> > this includes the Physical Presence Interface support that has
> > its counter-part in the SeaBIOS and is implemented using ACPI.
> > 
> > http://www.seabios.org/pipermail/seabios/2015-March/008978.html
> is it already merged?
> 
> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
> and tpm_ppi in BIOS memory?
> That would simplify BIOS part a bit and significantly simplify ACPI code
> as most of it is dealing with figuring out address of tpm_ppi.

Which (if it works) I guess brings us back to the idea of using
a pci device with a bar where we can stick tpm+vm id+whatever?


> > 
> > 
> > Stefan Berger (5):
> >   Provide support for the CUSE TPM
> >   Support Physical Presence Interface Spec
> >   Introduce condition to notifiy waiters of completed command
> >   Introduce condition in TPM backend for notification
> >   Add support for VM suspend/resume for TPM TIS
> > 
> >  hmp.c                        |   6 +
> >  hw/i386/acpi-tpm-core.dsl    | 277 +++++++++++++++++++++++++++++
> >  hw/i386/acpi-tpm2.dsl        |  27 +++
> >  hw/i386/q35-acpi-dsdt.dsl    |   1 +
> >  hw/i386/ssdt-tpm.dsl         |  12 +-
> >  hw/tpm/tpm_int.h             |   4 +
> >  hw/tpm/tpm_ioctl.h           | 178 +++++++++++++++++++
> >  hw/tpm/tpm_passthrough.c     | 410 +++++++++++++++++++++++++++++++++++++++++--
> >  hw/tpm/tpm_tis.c             | 152 +++++++++++++++-
> >  hw/tpm/tpm_tis.h             |   2 +
> >  hw/tpm/tpm_util.c            | 206 ++++++++++++++++++++++
> >  hw/tpm/tpm_util.h            |   7 +
> >  include/sysemu/tpm_backend.h |  12 ++
> >  qapi-schema.json             |  17 +-
> >  qemu-options.hx              |  21 ++-
> >  qmp-commands.hx              |   2 +-
> >  tpm.c                        |  11 +-
> >  17 files changed, 1316 insertions(+), 29 deletions(-)
> >  create mode 100644 hw/i386/acpi-tpm-core.dsl
> >  create mode 100644 hw/i386/acpi-tpm2.dsl
> >  create mode 100644 hw/tpm/tpm_ioctl.h
> > 

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-16 18:55   ` Michael S. Tsirkin
@ 2015-04-16 19:21     ` Stefan Berger
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-04-16 19:21 UTC (permalink / raw)
  To: Michael S. Tsirkin, Igor Mammedov; +Cc: safford, qemu-devel, quan.xu

On 04/16/2015 02:55 PM, Michael S. Tsirkin wrote:
> On Thu, Apr 16, 2015 at 03:35:06PM +0200, Igor Mammedov wrote:
>> On Wed, 15 Apr 2015 18:38:43 -0400
>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>
>>> The following series of patches extends TPM support with an
>>> external TPM that offers a Linux CUSE (character device in userspace)
>>> interface. This TPM lets each VM access its own private vTPM.
>>> The CUSE TPM supports suspend/resume and migration. Much
>>> out-of-band functionality necessary to control the CUSE TPM is
>>> implemented using ioctl's.
>>>
>>> The series extends the TPM support so far that most functionality of
>>> TPM support on a physical platform is now available to each x86 VM,
>>> this includes the Physical Presence Interface support that has
>>> its counter-part in the SeaBIOS and is implemented using ACPI.
>>>
>>> http://www.seabios.org/pipermail/seabios/2015-March/008978.html
>> is it already merged?
>>
>> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
>> and tpm_ppi in BIOS memory?
>> That would simplify BIOS part a bit and significantly simplify ACPI code
>> as most of it is dealing with figuring out address of tpm_ppi.
> Which (if it works) I guess brings us back to the idea of using
> a pci device with a bar where we can stick tpm+vm id+whatever?

Well, at least the current implementation works with these patches + 
CUSE TPM + patched SeaBIOS .

So the PCI bar does not get reset during a machine reboot and thus 
preserves values? I did not model the TPM TIS as a PCI device, since it 
typically is not such a device, but a LPC devices (close to ISA type of 
device).

If we wanted to achieve that this method also works on real hardware, 
with SeaBIOS running piggy-backed on coreboot, then we shouldn't assume 
a PCI device, since it won't be. Otherwise, what are we trying to 
achieve? Is the ACPI code the problem?

     Stefan

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-16 14:05   ` Stefan Berger
@ 2015-04-22  7:00     ` Igor Mammedov
  2015-04-22 18:18       ` Stefan Berger
  0 siblings, 1 reply; 21+ messages in thread
From: Igor Mammedov @ 2015-04-22  7:00 UTC (permalink / raw)
  To: Stefan Berger; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On Thu, 16 Apr 2015 10:05:35 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 04/16/2015 09:35 AM, Igor Mammedov wrote:
> > On Wed, 15 Apr 2015 18:38:43 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >
> >> The following series of patches extends TPM support with an
> >> external TPM that offers a Linux CUSE (character device in userspace)
> >> interface. This TPM lets each VM access its own private vTPM.
> >> The CUSE TPM supports suspend/resume and migration. Much
> >> out-of-band functionality necessary to control the CUSE TPM is
> >> implemented using ioctl's.
> >>
> >> The series extends the TPM support so far that most functionality of
> >> TPM support on a physical platform is now available to each x86 VM,
> >> this includes the Physical Presence Interface support that has
> >> its counter-part in the SeaBIOS and is implemented using ACPI.
> >>
> >> http://www.seabios.org/pipermail/seabios/2015-March/008978.html
> > is it already merged?
> 
> No, not yet. :-(
> 
> >
> > Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
> > and tpm_ppi in BIOS memory?
> 
> MMIO region of what? Of the TIS? The TIS doesn't have memory locations 
> 'just to keep bytes' and they would be cleared upon machine reset / reboot.
> 
> The purpose of the PPI interface is to leave an opcode for the BIOS to 
> act upon after a reset. So we have to write it into memory that doesn't 
> get cleared upon reboot. Also, the BIOS leaves a result in memory so we 
> can read the result code in the OS via sysfs entry.
it doesn't matter where opcodes are stored though, they could be stored
on QEMU's TPM device (i.e. inside TPM owned MMIO region). That way QEMU
will know in advance where opcodes are stored and build ACPI tables
with correct address without any need for scanning memory.

Although it's just another workaround around split brain problem where
QEMU owned ACPI tables have to know about guest (BIOS) provided address.


> 
> I had previously tried using NVRAM of the TPM to leave that opcode (and 
> result) , but this doesn't work well due to protection restrictions of 
> the TPM's NVRAM locations and using the Linux TSS for example non-root 
> users could then write an opcode into the NVRAM of the TPM (there are 
> TPM commands to write to the TPM's NVRAM locations and tpm-tools has 
> tools to write to these locations) that the machine then ends up acting 
> upon without the admin of the machine wanting that. So, that's not a 
> choice, either.
> 
> 
> > That would simplify BIOS part a bit and significantly simplify ACPI code
> > as most of it is dealing with figuring out address of tpm_ppi.
> 
> Wished it would, but I don't see a way to make it easier.
Since ACPI implementation for handling opcodes doesn't interface/depend
on TPM device and interfaces only with BIOS it should also be BIOS owned.
Cleaner way could be installing an additional BIOS generated table
(with correct ADDR) to QEMU provided ACPI tables set.

That also would be reusable with coreboot since you will have shared
ACPI impl. in SeaBIOS without need to duplicate it in QEMU/coreboot.

PS:
Is it possible to shovel BIOS side TPM code into option rom
so that it will be loaded only when TPM is present and keep
BIOS small in case TPM is not required?

> So the first time one looks into the sysfs ppi entries [on Linux] it may 
> take a few seconds until the anchor is found. Subsequently the memory 
> location is cached and operations  go a lot faster.
> 
>     Stefan
> 
> >>
> >> Stefan Berger (5):
> >>    Provide support for the CUSE TPM
> >>    Support Physical Presence Interface Spec
> >>    Introduce condition to notifiy waiters of completed command
> >>    Introduce condition in TPM backend for notification
> >>    Add support for VM suspend/resume for TPM TIS
> >>
> >>   hmp.c                        |   6 +
> >>   hw/i386/acpi-tpm-core.dsl    | 277 +++++++++++++++++++++++++++++
> >>   hw/i386/acpi-tpm2.dsl        |  27 +++
> >>   hw/i386/q35-acpi-dsdt.dsl    |   1 +
> >>   hw/i386/ssdt-tpm.dsl         |  12 +-
> >>   hw/tpm/tpm_int.h             |   4 +
> >>   hw/tpm/tpm_ioctl.h           | 178 +++++++++++++++++++
> >>   hw/tpm/tpm_passthrough.c     | 410 +++++++++++++++++++++++++++++++++++++++++--
> >>   hw/tpm/tpm_tis.c             | 152 +++++++++++++++-
> >>   hw/tpm/tpm_tis.h             |   2 +
> >>   hw/tpm/tpm_util.c            | 206 ++++++++++++++++++++++
> >>   hw/tpm/tpm_util.h            |   7 +
> >>   include/sysemu/tpm_backend.h |  12 ++
> >>   qapi-schema.json             |  17 +-
> >>   qemu-options.hx              |  21 ++-
> >>   qmp-commands.hx              |   2 +-
> >>   tpm.c                        |  11 +-
> >>   17 files changed, 1316 insertions(+), 29 deletions(-)
> >>   create mode 100644 hw/i386/acpi-tpm-core.dsl
> >>   create mode 100644 hw/i386/acpi-tpm2.dsl
> >>   create mode 100644 hw/tpm/tpm_ioctl.h
> >>
> 

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-22  7:00     ` Igor Mammedov
@ 2015-04-22 18:18       ` Stefan Berger
  2015-04-29  9:06         ` Igor Mammedov
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Berger @ 2015-04-22 18:18 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On 04/22/2015 03:00 AM, Igor Mammedov wrote:
> On Thu, 16 Apr 2015 10:05:35 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 04/16/2015 09:35 AM, Igor Mammedov wrote:
>>> On Wed, 15 Apr 2015 18:38:43 -0400
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>>
>>>> The following series of patches extends TPM support with an
>>>> external TPM that offers a Linux CUSE (character device in userspace)
>>>> interface. This TPM lets each VM access its own private vTPM.
>>>> The CUSE TPM supports suspend/resume and migration. Much
>>>> out-of-band functionality necessary to control the CUSE TPM is
>>>> implemented using ioctl's.
>>>>
>>>> The series extends the TPM support so far that most functionality of
>>>> TPM support on a physical platform is now available to each x86 VM,
>>>> this includes the Physical Presence Interface support that has
>>>> its counter-part in the SeaBIOS and is implemented using ACPI.
>>>>
>>>> http://www.seabios.org/pipermail/seabios/2015-March/008978.html
>>> is it already merged?
>> No, not yet. :-(
>>
>>> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
>>> and tpm_ppi in BIOS memory?
>> MMIO region of what? Of the TIS? The TIS doesn't have memory locations
>> 'just to keep bytes' and they would be cleared upon machine reset / reboot.
>>
>> The purpose of the PPI interface is to leave an opcode for the BIOS to
>> act upon after a reset. So we have to write it into memory that doesn't
>> get cleared upon reboot. Also, the BIOS leaves a result in memory so we
>> can read the result code in the OS via sysfs entry.
> it doesn't matter where opcodes are stored though, they could be stored
> on QEMU's TPM device (i.e. inside TPM owned MMIO region). That way QEMU
> will know in advance where opcodes are stored and build ACPI tables
> with correct address without any need for scanning memory.

It only matters that these opcodes survive a machine reboot. Some 
devices get reset on the way
and the choices of suitable NVRAM are limited.

Ok, so we could extend the TPM TIS model with a buffer that can hold 
these opcodes.
At the moment I think I need 3 bytes. Future specifications may require 
more. We can
make room for this in the TIS from offset 0xf90-0xfff where we can put 
vendor
specific extensions. Maybe we just stick it into locality 4 -> 0xfed4 
4f90 to 0xfed4 4fff
and don't reset that memory area ?

The only thing that speaks against this is that this would not work if 
SeaBIOS was not
running on QEMU but on bare metal -- if that was to be a concern at all. 
The current
solution tried to address this as well, but it would require coreboot 
support and the
last time I tested this on my Chromebook it looks like an anchor created 
but SeaBIOS
is not found after a reboot anymore, so it would require some form of 
cooperation
from coreboot to enable this. So if we ditch this, we can go with a 
buffer in the MMIO.



>
> Although it's just another workaround around split brain problem where
> QEMU owned ACPI tables have to know about guest (BIOS) provided address.
>
>
>> I had previously tried using NVRAM of the TPM to leave that opcode (and
>> result) , but this doesn't work well due to protection restrictions of
>> the TPM's NVRAM locations and using the Linux TSS for example non-root
>> users could then write an opcode into the NVRAM of the TPM (there are
>> TPM commands to write to the TPM's NVRAM locations and tpm-tools has
>> tools to write to these locations) that the machine then ends up acting
>> upon without the admin of the machine wanting that. So, that's not a
>> choice, either.
>>
>>
>>> That would simplify BIOS part a bit and significantly simplify ACPI code
>>> as most of it is dealing with figuring out address of tpm_ppi.
>> Wished it would, but I don't see a way to make it easier.
> Since ACPI implementation for handling opcodes doesn't interface/depend
> on TPM device and interfaces only with BIOS it should also be BIOS owned.
> Cleaner way could be installing an additional BIOS generated table
> (with correct ADDR) to QEMU provided ACPI tables set.

Would that be a custom table? Do we need changes for this in the OS
or can that table be found via ACPI?

>
> That also would be reusable with coreboot since you will have shared
> ACPI impl. in SeaBIOS without need to duplicate it in QEMU/coreboot.

Can you sketch the ACPI code necessary to convey the SeaBIOS / coreboot
allocated memory address ? How do we write the SeaBIOS allocated
address into the ACPI code?

>
> PS:
> Is it possible to shovel BIOS side TPM code into option rom
> so that it will be loaded only when TPM is present and keep
> BIOS small in case TPM is not required?

There are several places in core SeaBIOS where we jump into TPM related 
code
for taking measurements. See this patch. TPM support is isolated with a 
config
option and does not take much space if the user chooses not to compile 
it in.

http://www.seabios.org/pipermail/seabios/2015-March/008979.html

     Stefan

>> So the first time one looks into the sysfs ppi entries [on Linux] it may
>> take a few seconds until the anchor is found. Subsequently the memory
>> location is cached and operations  go a lot faster.
>>
>>      Stefan
>>
>>>> Stefan Berger (5):
>>>>     Provide support for the CUSE TPM
>>>>     Support Physical Presence Interface Spec
>>>>     Introduce condition to notifiy waiters of completed command
>>>>     Introduce condition in TPM backend for notification
>>>>     Add support for VM suspend/resume for TPM TIS
>>>>
>>>>    hmp.c                        |   6 +
>>>>    hw/i386/acpi-tpm-core.dsl    | 277 +++++++++++++++++++++++++++++
>>>>    hw/i386/acpi-tpm2.dsl        |  27 +++
>>>>    hw/i386/q35-acpi-dsdt.dsl    |   1 +
>>>>    hw/i386/ssdt-tpm.dsl         |  12 +-
>>>>    hw/tpm/tpm_int.h             |   4 +
>>>>    hw/tpm/tpm_ioctl.h           | 178 +++++++++++++++++++
>>>>    hw/tpm/tpm_passthrough.c     | 410 +++++++++++++++++++++++++++++++++++++++++--
>>>>    hw/tpm/tpm_tis.c             | 152 +++++++++++++++-
>>>>    hw/tpm/tpm_tis.h             |   2 +
>>>>    hw/tpm/tpm_util.c            | 206 ++++++++++++++++++++++
>>>>    hw/tpm/tpm_util.h            |   7 +
>>>>    include/sysemu/tpm_backend.h |  12 ++
>>>>    qapi-schema.json             |  17 +-
>>>>    qemu-options.hx              |  21 ++-
>>>>    qmp-commands.hx              |   2 +-
>>>>    tpm.c                        |  11 +-
>>>>    17 files changed, 1316 insertions(+), 29 deletions(-)
>>>>    create mode 100644 hw/i386/acpi-tpm-core.dsl
>>>>    create mode 100644 hw/i386/acpi-tpm2.dsl
>>>>    create mode 100644 hw/tpm/tpm_ioctl.h
>>>>
>

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-22 18:18       ` Stefan Berger
@ 2015-04-29  9:06         ` Igor Mammedov
  2015-04-29 16:42           ` Stefan Berger
  0 siblings, 1 reply; 21+ messages in thread
From: Igor Mammedov @ 2015-04-29  9:06 UTC (permalink / raw)
  To: Stefan Berger; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On Wed, 22 Apr 2015 14:18:55 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 04/22/2015 03:00 AM, Igor Mammedov wrote:
> > On Thu, 16 Apr 2015 10:05:35 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >
> >> On 04/16/2015 09:35 AM, Igor Mammedov wrote:
> >>> On Wed, 15 Apr 2015 18:38:43 -0400
[...]
> >>> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
> >>> and tpm_ppi in BIOS memory?
> >> MMIO region of what? Of the TIS? The TIS doesn't have memory locations
> >> 'just to keep bytes' and they would be cleared upon machine reset / reboot.
> >>
> >> The purpose of the PPI interface is to leave an opcode for the BIOS to
> >> act upon after a reset. So we have to write it into memory that doesn't
> >> get cleared upon reboot. Also, the BIOS leaves a result in memory so we
> >> can read the result code in the OS via sysfs entry.
> > it doesn't matter where opcodes are stored though, they could be stored
> > on QEMU's TPM device (i.e. inside TPM owned MMIO region). That way QEMU
> > will know in advance where opcodes are stored and build ACPI tables
> > with correct address without any need for scanning memory.
> 
> It only matters that these opcodes survive a machine reboot. Some 
> devices get reset on the way
> and the choices of suitable NVRAM are limited.
> 
> Ok, so we could extend the TPM TIS model with a buffer that can hold 
> these opcodes.
> At the moment I think I need 3 bytes. Future specifications may require 
> more. We can
> make room for this in the TIS from offset 0xf90-0xfff where we can put 
> vendor
> specific extensions. Maybe we just stick it into locality 4 -> 0xfed4 
> 4f90 to 0xfed4 4fff
> and don't reset that memory area ?
yep

> 
> The only thing that speaks against this is that this would not work if 
> SeaBIOS was not
> running on QEMU but on bare metal -- if that was to be a concern at all. 
> The current
> solution tried to address this as well, but it would require coreboot 
> support and the
> last time I tested this on my Chromebook it looks like an anchor created 
> but SeaBIOS
> is not found after a reboot anymore, so it would require some form of 
> cooperation
> from coreboot to enable this. So if we ditch this, we can go with a 
> buffer in the MMIO.
Coreboot would probably require different ACPI buffer read/write functions,
the rest could stay the same.

> 
> 
> 
> >
> > Although it's just another workaround around split brain problem where
> > QEMU owned ACPI tables have to know about guest (BIOS) provided address.
> >
> >
> >> I had previously tried using NVRAM of the TPM to leave that opcode (and
> >> result) , but this doesn't work well due to protection restrictions of
> >> the TPM's NVRAM locations and using the Linux TSS for example non-root
> >> users could then write an opcode into the NVRAM of the TPM (there are
> >> TPM commands to write to the TPM's NVRAM locations and tpm-tools has
> >> tools to write to these locations) that the machine then ends up acting
> >> upon without the admin of the machine wanting that. So, that's not a
> >> choice, either.
> >>
> >>
> >>> That would simplify BIOS part a bit and significantly simplify ACPI code
> >>> as most of it is dealing with figuring out address of tpm_ppi.
> >> Wished it would, but I don't see a way to make it easier.
> > Since ACPI implementation for handling opcodes doesn't interface/depend
> > on TPM device and interfaces only with BIOS it should also be BIOS owned.
> > Cleaner way could be installing an additional BIOS generated table
> > (with correct ADDR) to QEMU provided ACPI tables set.
> 
> Would that be a custom table? Do we need changes for this in the OS
> or can that table be found via ACPI?
It would be just additional SSDT, no OS changes are required.
what you'll need to do is to extend QEMU supplied RSDT to include
pointer to additional SSDT.
I'm not sure where ACPI tables come from in case of coreboot though.

> 
> >
> > That also would be reusable with coreboot since you will have shared
> > ACPI impl. in SeaBIOS without need to duplicate it in QEMU/coreboot.
> 
> Can you sketch the ACPI code necessary to convey the SeaBIOS / coreboot
> allocated memory address ? How do we write the SeaBIOS allocated
> address into the ACPI code?
grep for acpi_pci64_start in SeaBIOS code,
it should give rough idea how to do it.

[...]

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-29  9:06         ` Igor Mammedov
@ 2015-04-29 16:42           ` Stefan Berger
  2015-05-04  9:16             ` Igor Mammedov
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Berger @ 2015-04-29 16:42 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On 04/29/2015 05:06 AM, Igor Mammedov wrote:
> On Wed, 22 Apr 2015 14:18:55 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 04/22/2015 03:00 AM, Igor Mammedov wrote:
>>> On Thu, 16 Apr 2015 10:05:35 -0400
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>>
>>>> On 04/16/2015 09:35 AM, Igor Mammedov wrote:
>>>>> On Wed, 15 Apr 2015 18:38:43 -0400
> [...]
>>>>> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
>>>>> and tpm_ppi in BIOS memory?
>>>> MMIO region of what? Of the TIS? The TIS doesn't have memory locations
>>>> 'just to keep bytes' and they would be cleared upon machine reset / reboot.
>>>>
>>>> The purpose of the PPI interface is to leave an opcode for the BIOS to
>>>> act upon after a reset. So we have to write it into memory that doesn't
>>>> get cleared upon reboot. Also, the BIOS leaves a result in memory so we
>>>> can read the result code in the OS via sysfs entry.
>>> it doesn't matter where opcodes are stored though, they could be stored
>>> on QEMU's TPM device (i.e. inside TPM owned MMIO region). That way QEMU
>>> will know in advance where opcodes are stored and build ACPI tables
>>> with correct address without any need for scanning memory.
>> It only matters that these opcodes survive a machine reboot. Some
>> devices get reset on the way
>> and the choices of suitable NVRAM are limited.
>>
>> Ok, so we could extend the TPM TIS model with a buffer that can hold
>> these opcodes.
>> At the moment I think I need 3 bytes. Future specifications may require
>> more. We can
>> make room for this in the TIS from offset 0xf90-0xfff where we can put
>> vendor
>> specific extensions. Maybe we just stick it into locality 4 -> 0xfed4
>> 4f90 to 0xfed4 4fff
>> and don't reset that memory area ?
> yep


So we can do it this way ...


>
>> The only thing that speaks against this is that this would not work if
>> SeaBIOS was not
>> running on QEMU but on bare metal -- if that was to be a concern at all.
>> The current
>> solution tried to address this as well, but it would require coreboot
>> support and the
>> last time I tested this on my Chromebook it looks like an anchor created
>> but SeaBIOS
>> is not found after a reboot anymore, so it would require some form of
>> cooperation
>> from coreboot to enable this. So if we ditch this, we can go with a
>> buffer in the MMIO.
> Coreboot would probably require different ACPI buffer read/write functions,
> the rest could stay the same.

Yes, understood.

>>
>>
>>> Although it's just another workaround around split brain problem where
>>> QEMU owned ACPI tables have to know about guest (BIOS) provided address.
>>>
>>>
>>>> I had previously tried using NVRAM of the TPM to leave that opcode (and
>>>> result) , but this doesn't work well due to protection restrictions of
>>>> the TPM's NVRAM locations and using the Linux TSS for example non-root
>>>> users could then write an opcode into the NVRAM of the TPM (there are
>>>> TPM commands to write to the TPM's NVRAM locations and tpm-tools has
>>>> tools to write to these locations) that the machine then ends up acting
>>>> upon without the admin of the machine wanting that. So, that's not a
>>>> choice, either.
>>>>
>>>>
>>>>> That would simplify BIOS part a bit and significantly simplify ACPI code
>>>>> as most of it is dealing with figuring out address of tpm_ppi.
>>>> Wished it would, but I don't see a way to make it easier.
>>> Since ACPI implementation for handling opcodes doesn't interface/depend
>>> on TPM device and interfaces only with BIOS it should also be BIOS owned.
>>> Cleaner way could be installing an additional BIOS generated table
>>> (with correct ADDR) to QEMU provided ACPI tables set.
>> Would that be a custom table? Do we need changes for this in the OS
>> or can that table be found via ACPI?
> It would be just additional SSDT, no OS changes are required.
> what you'll need to do is to extend QEMU supplied RSDT to include
> pointer to additional SSDT.
> I'm not sure where ACPI tables come from in case of coreboot though.
>
>>> That also would be reusable with coreboot since you will have shared
>>> ACPI impl. in SeaBIOS without need to duplicate it in QEMU/coreboot.
>> Can you sketch the ACPI code necessary to convey the SeaBIOS / coreboot
>> allocated memory address ? How do we write the SeaBIOS allocated
>> address into the ACPI code?
> grep for acpi_pci64_start in SeaBIOS code,
> it should give rough idea how to do it.
>
> [...]
>

... or we can do it this way. Which way now ? My preference is the TIS 
path, because it seems easier.


Now what I am seeing in SeaBIOS is that another SSDT is being built. 
Would this then be an
additional SSDT or would it replace the TPM's SSDT that we're now 
supplying from QEMU.

2 choices now -- which one to take?

     Stefan

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-04-29 16:42           ` Stefan Berger
@ 2015-05-04  9:16             ` Igor Mammedov
  2015-05-04 15:22               ` Stefan Berger
  0 siblings, 1 reply; 21+ messages in thread
From: Igor Mammedov @ 2015-05-04  9:16 UTC (permalink / raw)
  To: Stefan Berger; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On Wed, 29 Apr 2015 12:42:21 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 04/29/2015 05:06 AM, Igor Mammedov wrote:
> > On Wed, 22 Apr 2015 14:18:55 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >
> >> On 04/22/2015 03:00 AM, Igor Mammedov wrote:
> >>> On Thu, 16 Apr 2015 10:05:35 -0400
> >>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >>>
> >>>> On 04/16/2015 09:35 AM, Igor Mammedov wrote:
> >>>>> On Wed, 15 Apr 2015 18:38:43 -0400
> > [...]
> >>>>> Is it possible to use MMIO region instead of allocating tpm_ppi_anchor
> >>>>> and tpm_ppi in BIOS memory?
> >>>> MMIO region of what? Of the TIS? The TIS doesn't have memory locations
> >>>> 'just to keep bytes' and they would be cleared upon machine reset / reboot.
> >>>>
> >>>> The purpose of the PPI interface is to leave an opcode for the BIOS to
> >>>> act upon after a reset. So we have to write it into memory that doesn't
> >>>> get cleared upon reboot. Also, the BIOS leaves a result in memory so we
> >>>> can read the result code in the OS via sysfs entry.
> >>> it doesn't matter where opcodes are stored though, they could be stored
> >>> on QEMU's TPM device (i.e. inside TPM owned MMIO region). That way QEMU
> >>> will know in advance where opcodes are stored and build ACPI tables
> >>> with correct address without any need for scanning memory.
> >> It only matters that these opcodes survive a machine reboot. Some
> >> devices get reset on the way
> >> and the choices of suitable NVRAM are limited.
> >>
> >> Ok, so we could extend the TPM TIS model with a buffer that can hold
> >> these opcodes.
> >> At the moment I think I need 3 bytes. Future specifications may require
> >> more. We can
> >> make room for this in the TIS from offset 0xf90-0xfff where we can put
> >> vendor
> >> specific extensions. Maybe we just stick it into locality 4 -> 0xfed4
> >> 4f90 to 0xfed4 4fff
> >> and don't reset that memory area ?
> > yep
> 
> 
> So we can do it this way ...
> 
> 
> >
> >> The only thing that speaks against this is that this would not work if
> >> SeaBIOS was not
> >> running on QEMU but on bare metal -- if that was to be a concern at all.
> >> The current
> >> solution tried to address this as well, but it would require coreboot
> >> support and the
> >> last time I tested this on my Chromebook it looks like an anchor created
> >> but SeaBIOS
> >> is not found after a reboot anymore, so it would require some form of
> >> cooperation
> >> from coreboot to enable this. So if we ditch this, we can go with a
> >> buffer in the MMIO.
> > Coreboot would probably require different ACPI buffer read/write functions,
> > the rest could stay the same.
> 
> Yes, understood.
> 
> >>
> >>
> >>> Although it's just another workaround around split brain problem where
> >>> QEMU owned ACPI tables have to know about guest (BIOS) provided address.
> >>>
> >>>
> >>>> I had previously tried using NVRAM of the TPM to leave that opcode (and
> >>>> result) , but this doesn't work well due to protection restrictions of
> >>>> the TPM's NVRAM locations and using the Linux TSS for example non-root
> >>>> users could then write an opcode into the NVRAM of the TPM (there are
> >>>> TPM commands to write to the TPM's NVRAM locations and tpm-tools has
> >>>> tools to write to these locations) that the machine then ends up acting
> >>>> upon without the admin of the machine wanting that. So, that's not a
> >>>> choice, either.
> >>>>
> >>>>
> >>>>> That would simplify BIOS part a bit and significantly simplify ACPI code
> >>>>> as most of it is dealing with figuring out address of tpm_ppi.
> >>>> Wished it would, but I don't see a way to make it easier.
> >>> Since ACPI implementation for handling opcodes doesn't interface/depend
> >>> on TPM device and interfaces only with BIOS it should also be BIOS owned.
> >>> Cleaner way could be installing an additional BIOS generated table
> >>> (with correct ADDR) to QEMU provided ACPI tables set.
> >> Would that be a custom table? Do we need changes for this in the OS
> >> or can that table be found via ACPI?
> > It would be just additional SSDT, no OS changes are required.
> > what you'll need to do is to extend QEMU supplied RSDT to include
> > pointer to additional SSDT.
> > I'm not sure where ACPI tables come from in case of coreboot though.
> >
> >>> That also would be reusable with coreboot since you will have shared
> >>> ACPI impl. in SeaBIOS without need to duplicate it in QEMU/coreboot.
> >> Can you sketch the ACPI code necessary to convey the SeaBIOS / coreboot
> >> allocated memory address ? How do we write the SeaBIOS allocated
> >> address into the ACPI code?
> > grep for acpi_pci64_start in SeaBIOS code,
> > it should give rough idea how to do it.
> >
> > [...]
> >
> 
> ... or we can do it this way. Which way now ? My preference is the TIS 
> path, because it seems easier.
> 
> 
> Now what I am seeing in SeaBIOS is that another SSDT is being built. 
> Would this then be an
> additional SSDT or would it replace the TPM's SSDT that we're now 
> supplying from QEMU.
perhaps an additional SSDT that has only Seabios specific AML which
doesn't interact with TIS device.

> 
> 2 choices now -- which one to take?
I'd try installing extra SSDT table first as a cleanest way (seabios only)
and if it fails fallback to TIS path.

> 
>      Stefan
> 

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-05-04  9:16             ` Igor Mammedov
@ 2015-05-04 15:22               ` Stefan Berger
  2015-05-04 16:16                 ` Kevin O'Connor
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Berger @ 2015-05-04 15:22 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On 05/04/2015 05:16 AM, Igor Mammedov wrote:
> On Wed, 29 Apr 2015 12:42:21 -0400
>
>
>> 2 choices now -- which one to take?
> I'd try installing extra SSDT table first as a cleanest way (seabios only)
> and if it fails fallback to TIS path.

I did some experiment where I tried to use AML's Store() call to see 
whether it can read from some entry in another scope, similar to what 
the specs show on page 487, so that one could read the base address that 
is written elsewhere. The problem seems to be that there is no actual 
searching going on for accessing a scope during runtime, but this seems 
to be done during compile time. So something like Store(\_SB.I2C1.Y, 
BUFF) from the specs needs to have a SCOPE(\_SB.I2C1...) available 
during compile time.

Otherwise I am not sure how to go about this. The only way I could think 
of would be to write a unique and 'recognizeable' address into the AML, 
something like 0x12345678, try to find it in SeaBIOS and replace it with 
the actual address. It doesn't seem right, tough. The next best choice 
would be to paravirtualize the TIS with bytes that survive a reset.

   Regards,
       Stefan

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-05-04 15:22               ` Stefan Berger
@ 2015-05-04 16:16                 ` Kevin O'Connor
  2015-05-04 18:39                   ` Stefan Berger
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin O'Connor @ 2015-05-04 16:16 UTC (permalink / raw)
  To: Stefan Berger; +Cc: safford, Igor Mammedov, qemu-devel, quan.xu, mst

On Mon, May 04, 2015 at 11:22:25AM -0400, Stefan Berger wrote:
> On 05/04/2015 05:16 AM, Igor Mammedov wrote:
> >On Wed, 29 Apr 2015 12:42:21 -0400
> >>2 choices now -- which one to take?
> >I'd try installing extra SSDT table first as a cleanest way (seabios only)
> >and if it fails fallback to TIS path.
> 
> I did some experiment where I tried to use AML's Store() call to see whether
> it can read from some entry in another scope, similar to what the specs show
> on page 487, so that one could read the base address that is written
> elsewhere. The problem seems to be that there is no actual searching going
> on for accessing a scope during runtime, but this seems to be done during
> compile time. So something like Store(\_SB.I2C1.Y, BUFF) from the specs
> needs to have a SCOPE(\_SB.I2C1...) available during compile time.

You can reference scopes provided by other SSDTs/DSDT using the
"External()" acpi command.

> Otherwise I am not sure how to go about this. The only way I could think of
> would be to write a unique and 'recognizeable' address into the AML,
> something like 0x12345678, try to find it in SeaBIOS and replace it with the
> actual address. It doesn't seem right, tough. The next best choice would be
> to paravirtualize the TIS with bytes that survive a reset.

I don't think it's a good idea to have SeaBIOS modify the QEMU
provided ACPI tables.  It also shouldn't be necessary.  If there's
anything that QEMU needs in the guest memory, then QEMU should be able
to build it and tell SeaBIOS to allocate it via the existing
"romfile_loader" mechanism.

-Kevin

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-05-04 16:16                 ` Kevin O'Connor
@ 2015-05-04 18:39                   ` Stefan Berger
  2015-05-04 21:41                     ` Igor Mammedov
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Berger @ 2015-05-04 18:39 UTC (permalink / raw)
  To: Kevin O'Connor; +Cc: safford, Igor Mammedov, qemu-devel, quan.xu, mst

On 05/04/2015 12:16 PM, Kevin O'Connor wrote:
> On Mon, May 04, 2015 at 11:22:25AM -0400, Stefan Berger wrote:
>> On 05/04/2015 05:16 AM, Igor Mammedov wrote:
>>> On Wed, 29 Apr 2015 12:42:21 -0400
>>>> 2 choices now -- which one to take?
>>> I'd try installing extra SSDT table first as a cleanest way (seabios only)
>>> and if it fails fallback to TIS path.
>> I did some experiment where I tried to use AML's Store() call to see whether
>> it can read from some entry in another scope, similar to what the specs show
>> on page 487, so that one could read the base address that is written
>> elsewhere. The problem seems to be that there is no actual searching going
>> on for accessing a scope during runtime, but this seems to be done during
>> compile time. So something like Store(\_SB.I2C1.Y, BUFF) from the specs
>> needs to have a SCOPE(\_SB.I2C1...) available during compile time.
> You can reference scopes provided by other SSDTs/DSDT using the
> "External()" acpi command.

Tried it. Works with the \DBUG function for example.

As an example, External(\BASE) would give allow us to get the base address.
The AML code for BASE, something like this

     Scope(\) {
         Name(BASE, 0x12345678)
     }


Access to it would be like this:

Store(\BASE, Local0)

The above first snippet would have to be compiled by SeaBIOS. 0x12345678 
would in this case be the address allocated by SeaBIOS where the AML 
code for PPI could read/write the PPID data to/from. Kevin and I had an 
offline conversation about this and Kevin doesn't seem to like the idea 
of a split implementation of ACPI table generation. I would then extend 
the TIS with a couple of bytes that retain the data across a reset.

    Stefan

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-05-04 18:39                   ` Stefan Berger
@ 2015-05-04 21:41                     ` Igor Mammedov
  2015-05-05  2:50                       ` Kevin O'Connor
  0 siblings, 1 reply; 21+ messages in thread
From: Igor Mammedov @ 2015-05-04 21:41 UTC (permalink / raw)
  To: Stefan Berger; +Cc: safford, Kevin O'Connor, qemu-devel, quan.xu, mst

On Mon, 04 May 2015 14:39:32 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 05/04/2015 12:16 PM, Kevin O'Connor wrote:
> > On Mon, May 04, 2015 at 11:22:25AM -0400, Stefan Berger wrote:
> >> On 05/04/2015 05:16 AM, Igor Mammedov wrote:
> >>> On Wed, 29 Apr 2015 12:42:21 -0400
> >>>> 2 choices now -- which one to take?
> >>> I'd try installing extra SSDT table first as a cleanest way
> >>> (seabios only) and if it fails fallback to TIS path.
> >> I did some experiment where I tried to use AML's Store() call to
> >> see whether it can read from some entry in another scope, similar
> >> to what the specs show on page 487, so that one could read the
> >> base address that is written elsewhere. The problem seems to be
> >> that there is no actual searching going on for accessing a scope
> >> during runtime, but this seems to be done during compile time. So
> >> something like Store(\_SB.I2C1.Y, BUFF) from the specs needs to
> >> have a SCOPE(\_SB.I2C1...) available during compile time.
> > You can reference scopes provided by other SSDTs/DSDT using the
> > "External()" acpi command.
> 
> Tried it. Works with the \DBUG function for example.
> 
> As an example, External(\BASE) would give allow us to get the base
> address. The AML code for BASE, something like this
> 
>      Scope(\) {
>          Name(BASE, 0x12345678)
>      }
> 
> 
> Access to it would be like this:
> 
> Store(\BASE, Local0)
> 
> The above first snippet would have to be compiled by SeaBIOS.
> 0x12345678 would in this case be the address allocated by SeaBIOS
> where the AML code for PPI could read/write the PPID data to/from.
> Kevin and I had an offline conversation about this and Kevin doesn't
> seem to like the idea of a split implementation of ACPI table
> generation. I would then extend the TIS with a couple of bytes that
> retain the data across a reset.
It's not really split implementation, it's installing an additional
table which is purely SeaBIOS specific since QEMU doesn't care about
PPID data at all nor does it need to do so.

For example of AML template patching in SeaBIOS you can look at
build_pcihp(), so you can dynamically patch AML variable in template
when SeaBIOS (re)boots.

What's missing in SeaBIOS is a mechanism to add extra table to QEMU
supplied RSDT.

But I'm fine with paravirt implementation if above is considered too
hard to implement. If you go this route look at build_ssdt() in QEMU
and a new aml_*() API that helps to create AML objects dynamically.

> 
>     Stefan
> 
> 

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-05-04 21:41                     ` Igor Mammedov
@ 2015-05-05  2:50                       ` Kevin O'Connor
  2015-05-05 17:42                         ` Stefan Berger
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin O'Connor @ 2015-05-05  2:50 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: safford, mst, qemu-devel, quan.xu, Stefan Berger

On Mon, May 04, 2015 at 11:41:54PM +0200, Igor Mammedov wrote:
> It's not really split implementation, it's installing an additional
> table which is purely SeaBIOS specific since QEMU doesn't care about
> PPID data at all nor does it need to do so.

If both SeaBIOS and QEMU provide acpi tables, it can lead to
confusion.  For example, if seabios adds tables, what acpi version
should go into the acpi table header, if Windows BSODs what project
should the bug report go to, how do we ensure no namespace collisions
between SSDTs, etc.

So, it's technically not difficult, but the support is painful.

> But I'm fine with paravirt implementation if above is considered too
> hard to implement. If you go this route look at build_ssdt() in QEMU
> and a new aml_*() API that helps to create AML objects dynamically.

I think this approach is simpler all around.

-Kevin

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

* Re: [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM
  2015-05-05  2:50                       ` Kevin O'Connor
@ 2015-05-05 17:42                         ` Stefan Berger
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Berger @ 2015-05-05 17:42 UTC (permalink / raw)
  To: Kevin O'Connor, Igor Mammedov; +Cc: safford, qemu-devel, quan.xu, mst

On 05/04/2015 10:50 PM, Kevin O'Connor wrote:
> On Mon, May 04, 2015 at 11:41:54PM +0200, Igor Mammedov wrote:
>> It's not really split implementation, it's installing an additional
>> table which is purely SeaBIOS specific since QEMU doesn't care about
>> PPID data at all nor does it need to do so.
> If both SeaBIOS and QEMU provide acpi tables, it can lead to
> confusion.  For example, if seabios adds tables, what acpi version
> should go into the acpi table header, if Windows BSODs what project
> should the bug report go to, how do we ensure no namespace collisions
> between SSDTs, etc.
>
> So, it's technically not difficult, but the support is painful.
>
>> But I'm fine with paravirt implementation if above is considered too
>> hard to implement. If you go this route look at build_ssdt() in QEMU
>> and a new aml_*() API that helps to create AML objects dynamically.
> I think this approach is simpler all around.

I did that now and will repost the series here and those for SeaBIOS.

    Stefan

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

end of thread, other threads:[~2015-05-05 17:42 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15 22:38 [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Stefan Berger
2015-04-15 22:38 ` [Qemu-devel] [PATCH 1/5] Provide support for the CUSE TPM Stefan Berger
2015-04-15 22:38 ` [Qemu-devel] [PATCH 2/5] Support Physical Presence Interface Spec Stefan Berger
2015-04-15 22:38 ` [Qemu-devel] [PATCH 3/5] Introduce condition to notifiy waiters of completed command Stefan Berger
2015-04-15 22:38 ` [Qemu-devel] [PATCH 4/5] Introduce condition in TPM backend for notification Stefan Berger
2015-04-15 22:38 ` [Qemu-devel] [PATCH 5/5] Add support for VM suspend/resume for TPM TIS Stefan Berger
2015-04-16 13:35 ` [Qemu-devel] [PATCH 0/5] Extend TPM support with a QEMU-external TPM Igor Mammedov
2015-04-16 14:05   ` Stefan Berger
2015-04-22  7:00     ` Igor Mammedov
2015-04-22 18:18       ` Stefan Berger
2015-04-29  9:06         ` Igor Mammedov
2015-04-29 16:42           ` Stefan Berger
2015-05-04  9:16             ` Igor Mammedov
2015-05-04 15:22               ` Stefan Berger
2015-05-04 16:16                 ` Kevin O'Connor
2015-05-04 18:39                   ` Stefan Berger
2015-05-04 21:41                     ` Igor Mammedov
2015-05-05  2:50                       ` Kevin O'Connor
2015-05-05 17:42                         ` Stefan Berger
2015-04-16 18:55   ` Michael S. Tsirkin
2015-04-16 19:21     ` Stefan Berger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.