All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform
@ 2019-12-19 14:05 Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c Stefan Berger
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:05 UTC (permalink / raw)
  To: qemu-ppc; +Cc: marcandre.lureau, Stefan Berger, qemu-devel, david

The following series of patches adds vTPM emulator support for the
ppc64 platform (pSeries). 

It can be tested as follows with swtpm/libtpms:

mkdir /tmp/mytpm1
swtpm socket --tpmstate dir=/tmp/mytpm1 \
  --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \
  --log level=20

If TPM 2 is desired, add --tpm2 as parameter to the above.

In another terminal start QEMU:

sudo ./ppc64-softmmu/qemu-system-ppc64 -display sdl \
	-machine pseries,accel=kvm \
	-m 1024 -bios slof.bin -boot menu=on \
	-nodefaults -device VGA -device pci-ohci -device usb-kbd \
	-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
	-tpmdev emulator,id=tpm0,chardev=chrtpm \
	-device tpm-spapr,tpmdev=tpm0 \
	-device spapr-vscsi,id=scsi0,reg=0x00002000 \
	-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 \
	-drive file=test.img,format=raw,if=none,id=drive-virtio-disk0

Links:
 - libtpms: https://github.com/stefanberger/libtpms/wiki
 - swtpm: https://github.com/stefanberger/swtpm/wiki

Changes:
 v6->v7:
  - Implemented get_dt_compatible() and using it
  - Moved tpm_this_show_buffer to tpm_util.c

 v5->v6:
  - adjusted names of structures and simplified
  - only transmitting min. necessary bytes to pass to VM after resume
  - addressed other issues pointed out by D. Gibson

 v4->v5:
  - use runstate_check(RUN_STATE_FINISH_MIGRATE) to check whether devices
    are suspending; ditch 3 patches in this series that tried to do similar

 v3->v4:
  - addressed comments to v3
  - reworked suspend/resume support that requires extensions to backends

 v2->v3:
  - patch 1: a TPM 2 is identified by IBM,vtpm20 in the compatible node
  - patch 1: convert to tracing to display Tx and Rx buffers
  - added documentation patch
  - added patch to enable TPM device as part of pSeries

 v1->v2:
  - followed Cedric Le Goater's suggestions to patch 1
  - send appropriate CRQ error responses if DMA read or write fails
  - renamed tpm_spapr_got_payload to tpm_spapr_process_cmd and
    pass endianess-adjusted data pointer from CRQ to it

Regards,
    Stefan



Stefan Berger (6):
  tpm: Move tpm_tis_show_buffer to tpm_util.c
  spapr: Implement get_dt_compatible() callback
  tpm_spapr: Support TPM for ppc64 using CRQ based interface
  tpm_spapr: Support suspend and resume
  hw/ppc/Kconfig: Enable TPM_SPAPR as part of PSERIES config
  docs: tpm: Add example command line for ppc64 and tpm-spapr

 docs/specs/tpm.txt         |  20 +-
 hw/ppc/Kconfig             |   1 +
 hw/ppc/spapr_vio.c         |  11 +-
 hw/tpm/Kconfig             |   6 +
 hw/tpm/Makefile.objs       |   1 +
 hw/tpm/tpm_spapr.c         | 443 +++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_tis.c           |  32 +--
 hw/tpm/tpm_util.c          |  25 +++
 hw/tpm/tpm_util.h          |   3 +
 hw/tpm/trace-events        |  16 +-
 include/hw/ppc/spapr_vio.h |   1 +
 include/sysemu/tpm.h       |   3 +
 qapi/tpm.json              |   6 +-
 13 files changed, 533 insertions(+), 35 deletions(-)
 create mode 100644 hw/tpm/tpm_spapr.c

-- 
2.21.0



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

* [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c
  2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
@ 2019-12-19 14:06 ` Stefan Berger
  2019-12-19 14:29   ` Philippe Mathieu-Daudé
  2019-12-20  8:23   ` David Gibson
  2019-12-19 14:06 ` [PATCH v7 2/6] spapr: Implement get_dt_compatible() callback Stefan Berger
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:06 UTC (permalink / raw)
  To: qemu-ppc
  Cc: marcandre.lureau, Stefan Berger, Stefan Berger, qemu-devel, david

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis.c    | 32 ++++----------------------------
 hw/tpm/tpm_util.c   | 25 +++++++++++++++++++++++++
 hw/tpm/tpm_util.h   |  3 +++
 hw/tpm/trace-events |  2 +-
 4 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 7aaf9b946d..5b17c88a7d 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -107,30 +107,6 @@ static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
     return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
 }
 
-static void tpm_tis_show_buffer(const unsigned char *buffer,
-                                size_t buffer_size, const char *string)
-{
-    size_t len, i;
-    char *line_buffer, *p;
-
-    len = MIN(tpm_cmd_get_size(buffer), buffer_size);
-
-    /*
-     * allocate enough room for 3 chars per buffer entry plus a
-     * newline after every 16 chars and a final null terminator.
-     */
-    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
-
-    for (i = 0, p = line_buffer; i < len; i++) {
-        if (i && !(i % 16)) {
-            p += sprintf(p, "\n");
-        }
-        p += sprintf(p, "%.2X ", buffer[i]);
-    }
-    trace_tpm_tis_show_buffer(string, len, line_buffer);
-
-    g_free(line_buffer);
-}
 
 /*
  * Set the given flags in the STS register by clearing the register but
@@ -156,8 +132,8 @@ static void tpm_tis_sts_set(TPMLocality *l, uint32_t flags)
  */
 static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
 {
-    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
-        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
+    if (trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
+        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
     }
 
     /*
@@ -325,8 +301,8 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
     s->loc[locty].state = TPM_TIS_STATE_COMPLETION;
     s->rw_offset = 0;
 
-    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
-        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
+    if (trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
+        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
     }
 
     if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index 62b091f0c0..c0a0f3d71f 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -350,3 +350,28 @@ void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
     tsb->buffer = NULL;
     tsb->size = 0;
 }
+
+void tpm_util_show_buffer(const unsigned char *buffer,
+                          size_t buffer_size, const char *string)
+{
+    size_t len, i;
+    char *line_buffer, *p;
+
+    len = MIN(tpm_cmd_get_size(buffer), buffer_size);
+
+    /*
+     * allocate enough room for 3 chars per buffer entry plus a
+     * newline after every 16 chars and a final null terminator.
+     */
+    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
+
+    for (i = 0, p = line_buffer; i < len; i++) {
+        if (i && !(i % 16)) {
+            p += sprintf(p, "\n");
+        }
+        p += sprintf(p, "%.2X ", buffer[i]);
+    }
+    trace_tpm_util_show_buffer(string, len, line_buffer);
+
+    g_free(line_buffer);
+}
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index f397ac21b8..7889081fba 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -79,4 +79,7 @@ typedef struct TPMSizedBuffer {
 
 void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
 
+void tpm_util_show_buffer(const unsigned char *buffer,
+                          size_t buffer_size, const char *string);
+
 #endif /* TPM_TPM_UTIL_H */
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 89804bcd64..357c9e9a84 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -14,6 +14,7 @@ tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len = %u,
 tpm_util_get_buffer_size_hdr_len2(uint32_t len, size_t expected) "tpm2_resp->hdr.len = %u, expected = %zu"
 tpm_util_get_buffer_size_len2(uint32_t len, size_t expected) "tpm2_resp->len = %u, expected = %zu"
 tpm_util_get_buffer_size(size_t len) "buffersize of device: %zu"
+tpm_util_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"
 
 # tpm_emulator.c
 tpm_emulator_set_locality(uint8_t locty) "setting locality to %d"
@@ -36,7 +37,6 @@ tpm_emulator_pre_save(void) ""
 tpm_emulator_inst_init(void) ""
 
 # tpm_tis.c
-tpm_tis_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\nbuf: %s"
 tpm_tis_raise_irq(uint32_t irqmask) "Raising IRQ for flag 0x%08x"
 tpm_tis_new_active_locality(uint8_t locty) "Active locality is now %d"
 tpm_tis_abort(uint8_t locty) "New active locality is %d"
-- 
2.21.0



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

* [PATCH v7 2/6] spapr: Implement get_dt_compatible() callback
  2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c Stefan Berger
@ 2019-12-19 14:06 ` Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 3/6] tpm_spapr: Support TPM for ppc64 using CRQ based interface Stefan Berger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:06 UTC (permalink / raw)
  To: qemu-ppc
  Cc: marcandre.lureau, Stefan Berger, Stefan Berger, qemu-devel, david

For devices that cannot be statically initialized, implement a
get_dt_compatible() callback that allows us to ask the device for
the 'compatible' value.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/ppc/spapr_vio.c         | 11 +++++++++--
 include/hw/ppc/spapr_vio.h |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 554de9930d..4b24b81797 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -87,6 +87,7 @@ static int vio_make_devnode(SpaprVioDevice *dev,
     SpaprVioDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
     int vdevice_off, node_off, ret;
     char *dt_name;
+    const char *dt_compatible;
 
     vdevice_off = fdt_path_offset(fdt, "/vdevice");
     if (vdevice_off < 0) {
@@ -113,9 +114,15 @@ static int vio_make_devnode(SpaprVioDevice *dev,
         }
     }
 
-    if (pc->dt_compatible) {
+    if (pc->get_dt_compatible) {
+        dt_compatible = pc->get_dt_compatible(dev);
+    } else {
+        dt_compatible = pc->dt_compatible;
+    }
+
+    if (dt_compatible) {
         ret = fdt_setprop_string(fdt, node_off, "compatible",
-                                 pc->dt_compatible);
+                                 dt_compatible);
         if (ret < 0) {
             return ret;
         }
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index 72762ed16b..67f58b7f98 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -58,6 +58,7 @@ typedef struct SpaprVioDeviceClass {
     void (*realize)(SpaprVioDevice *dev, Error **errp);
     void (*reset)(SpaprVioDevice *dev);
     int (*devnode)(SpaprVioDevice *dev, void *fdt, int node_off);
+    const char *(*get_dt_compatible)(SpaprVioDevice *dev);
 } SpaprVioDeviceClass;
 
 struct SpaprVioDevice {
-- 
2.21.0



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

* [PATCH v7 3/6] tpm_spapr: Support TPM for ppc64 using CRQ based interface
  2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 2/6] spapr: Implement get_dt_compatible() callback Stefan Berger
@ 2019-12-19 14:06 ` Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 4/6] tpm_spapr: Support suspend and resume Stefan Berger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:06 UTC (permalink / raw)
  To: qemu-ppc; +Cc: marcandre.lureau, Stefan Berger, qemu-devel, david

Implement support for TPM on ppc64 by implementing the vTPM CRQ interface
as a frontend. It can use the tpm_emulator driver backend with the external
swtpm.

The Linux vTPM driver for ppc64 works with this emulation.

This TPM emulator also handles the TPM 2 case.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/tpm/Kconfig       |   6 +
 hw/tpm/Makefile.objs |   1 +
 hw/tpm/tpm_spapr.c   | 378 +++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/trace-events  |  12 ++
 include/sysemu/tpm.h |   3 +
 qapi/tpm.json        |   6 +-
 6 files changed, 403 insertions(+), 3 deletions(-)
 create mode 100644 hw/tpm/tpm_spapr.c

diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 4c8ee87d67..66a570aac1 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -22,3 +22,9 @@ config TPM_EMULATOR
     bool
     default y
     depends on TPMDEV
+
+config TPM_SPAPR
+    bool
+    default n
+    select TPMDEV
+    depends on PSERIES
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index de0b85d02a..85eb99ae05 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
 common-obj-$(CONFIG_TPM_CRB) += tpm_crb.o
 common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
 common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o
+obj-$(CONFIG_TPM_SPAPR) += tpm_spapr.o
diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
new file mode 100644
index 0000000000..ab184fbb82
--- /dev/null
+++ b/hw/tpm/tpm_spapr.c
@@ -0,0 +1,378 @@
+/*
+ * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
+ *
+ * PAPR Virtual TPM
+ *
+ * Copyright (c) 2015, 2017, 2019 IBM Corporation.
+ *
+ * Authors:
+ *    Stefan Berger <stefanb@linux.vnet.ibm.com>
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+
+#include "sysemu/tpm_backend.h"
+#include "tpm_int.h"
+#include "tpm_util.h"
+
+#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_vio.h"
+#include "trace.h"
+
+#define DEBUG_SPAPR 0
+
+#define VIO_SPAPR_VTPM(obj) \
+     OBJECT_CHECK(SpaprTpmState, (obj), TYPE_TPM_SPAPR)
+
+typedef struct TpmCrq {
+    uint8_t valid;  /* 0x80: cmd; 0xc0: init crq */
+                    /* 0x81-0x83: CRQ message response */
+    uint8_t msg;    /* see below */
+    uint16_t len;   /* len of TPM request; len of TPM response */
+    uint32_t data;  /* rtce_dma_handle when sending TPM request */
+    uint64_t reserved;
+} TpmCrq;
+
+#define SPAPR_VTPM_VALID_INIT_CRQ_COMMAND  0xC0
+#define SPAPR_VTPM_VALID_COMMAND           0x80
+#define SPAPR_VTPM_MSG_RESULT              0x80
+
+/* msg types for valid = SPAPR_VTPM_VALID_INIT_CRQ */
+#define SPAPR_VTPM_INIT_CRQ_RESULT           0x1
+#define SPAPR_VTPM_INIT_CRQ_COMPLETE_RESULT  0x2
+
+/* msg types for valid = SPAPR_VTPM_VALID_CMD */
+#define SPAPR_VTPM_GET_VERSION               0x1
+#define SPAPR_VTPM_TPM_COMMAND               0x2
+#define SPAPR_VTPM_GET_RTCE_BUFFER_SIZE      0x3
+#define SPAPR_VTPM_PREPARE_TO_SUSPEND        0x4
+
+/* response error messages */
+#define SPAPR_VTPM_VTPM_ERROR                0xff
+
+/* error codes */
+#define SPAPR_VTPM_ERR_COPY_IN_FAILED        0x3
+#define SPAPR_VTPM_ERR_COPY_OUT_FAILED       0x4
+
+#define TPM_SPAPR_BUFFER_MAX                 4096
+
+typedef struct {
+    SpaprVioDevice vdev;
+
+    TpmCrq crq; /* track single TPM command */
+
+    uint8_t state;
+#define SPAPR_VTPM_STATE_NONE         0
+#define SPAPR_VTPM_STATE_EXECUTION    1
+#define SPAPR_VTPM_STATE_COMPLETION   2
+
+    unsigned char buffer[TPM_SPAPR_BUFFER_MAX];
+
+    TPMBackendCmd cmd;
+
+    TPMBackend *be_driver;
+    TPMVersion be_tpm_version;
+
+    size_t be_buffer_size;
+} SpaprTpmState;
+
+/*
+ * Send a request to the TPM.
+ */
+static void tpm_spapr_tpm_send(SpaprTpmState *s)
+{
+    if (trace_event_get_state_backends(TRACE_TPM_SPAPR_SHOW_BUFFER)) {
+        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
+    }
+
+    s->state = SPAPR_VTPM_STATE_EXECUTION;
+    s->cmd = (TPMBackendCmd) {
+        .locty = 0,
+        .in = s->buffer,
+        .in_len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size),
+        .out = s->buffer,
+        .out_len = s->be_buffer_size,
+    };
+
+    tpm_backend_deliver_request(s->be_driver, &s->cmd);
+}
+
+static int tpm_spapr_process_cmd(SpaprTpmState *s, uint64_t dataptr)
+{
+    long rc;
+
+    /* a max. of be_buffer_size bytes can be transported */
+    rc = spapr_vio_dma_read(&s->vdev, dataptr,
+                            s->buffer, s->be_buffer_size);
+    if (rc) {
+        error_report("tpm_spapr_got_payload: DMA read failure");
+    }
+    /* let vTPM handle any malformed request */
+    tpm_spapr_tpm_send(s);
+
+    return rc;
+}
+
+static inline int spapr_tpm_send_crq(struct SpaprVioDevice *dev, TpmCrq *crq)
+{
+    return spapr_vio_send_crq(dev, (uint8_t *)crq);
+}
+
+static int tpm_spapr_do_crq(struct SpaprVioDevice *dev, uint8_t *crq_data)
+{
+    SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
+    TpmCrq local_crq;
+    TpmCrq *crq = &s->crq; /* requests only */
+    int rc;
+    uint8_t valid = crq_data[0];
+    uint8_t msg = crq_data[1];
+
+    trace_tpm_spapr_do_crq(valid, msg);
+
+    switch (valid) {
+    case SPAPR_VTPM_VALID_INIT_CRQ_COMMAND: /* Init command/response */
+
+        /* Respond to initialization request */
+        switch (msg) {
+        case SPAPR_VTPM_INIT_CRQ_RESULT:
+            trace_tpm_spapr_do_crq_crq_result();
+            memset(&local_crq, 0, sizeof(local_crq));
+            local_crq.valid = SPAPR_VTPM_VALID_INIT_CRQ_COMMAND;
+            local_crq.msg = SPAPR_VTPM_INIT_CRQ_RESULT;
+            spapr_tpm_send_crq(dev, &local_crq);
+            break;
+
+        case SPAPR_VTPM_INIT_CRQ_COMPLETE_RESULT:
+            trace_tpm_spapr_do_crq_crq_complete_result();
+            memset(&local_crq, 0, sizeof(local_crq));
+            local_crq.valid = SPAPR_VTPM_VALID_INIT_CRQ_COMMAND;
+            local_crq.msg = SPAPR_VTPM_INIT_CRQ_COMPLETE_RESULT;
+            spapr_tpm_send_crq(dev, &local_crq);
+            break;
+        }
+
+        break;
+    case SPAPR_VTPM_VALID_COMMAND: /* Payloads */
+        switch (msg) {
+        case SPAPR_VTPM_TPM_COMMAND:
+            trace_tpm_spapr_do_crq_tpm_command();
+            if (s->state == SPAPR_VTPM_STATE_EXECUTION) {
+                return H_BUSY;
+            }
+            memcpy(crq, crq_data, sizeof(*crq));
+
+            rc = tpm_spapr_process_cmd(s, be32_to_cpu(crq->data));
+
+            if (rc == H_SUCCESS) {
+                crq->valid = be16_to_cpu(0);
+            } else {
+                local_crq.valid = SPAPR_VTPM_MSG_RESULT;
+                local_crq.msg = SPAPR_VTPM_VTPM_ERROR;
+                local_crq.len = cpu_to_be16(0);
+                local_crq.data = cpu_to_be32(SPAPR_VTPM_ERR_COPY_IN_FAILED);
+                spapr_tpm_send_crq(dev, &local_crq);
+            }
+            break;
+
+        case SPAPR_VTPM_GET_RTCE_BUFFER_SIZE:
+            trace_tpm_spapr_do_crq_tpm_get_rtce_buffer_size(s->be_buffer_size);
+            local_crq.valid = SPAPR_VTPM_VALID_COMMAND;
+            local_crq.msg = SPAPR_VTPM_GET_RTCE_BUFFER_SIZE |
+                            SPAPR_VTPM_MSG_RESULT;
+            local_crq.len = cpu_to_be16(s->be_buffer_size);
+            spapr_tpm_send_crq(dev, &local_crq);
+            break;
+
+        case SPAPR_VTPM_GET_VERSION:
+            local_crq.valid = SPAPR_VTPM_VALID_COMMAND;
+            local_crq.msg = SPAPR_VTPM_GET_VERSION | SPAPR_VTPM_MSG_RESULT;
+            local_crq.len = cpu_to_be16(0);
+            switch (s->be_tpm_version) {
+            case TPM_VERSION_1_2:
+                local_crq.data = cpu_to_be32(1);
+                break;
+            case TPM_VERSION_2_0:
+                local_crq.data = cpu_to_be32(2);
+                break;
+            default:
+                g_assert_not_reached();
+                break;
+            }
+            trace_tpm_spapr_do_crq_get_version(be32_to_cpu(local_crq.data));
+            spapr_tpm_send_crq(dev, &local_crq);
+            break;
+
+        case SPAPR_VTPM_PREPARE_TO_SUSPEND:
+            trace_tpm_spapr_do_crq_prepare_to_suspend();
+            local_crq.valid = SPAPR_VTPM_VALID_COMMAND;
+            local_crq.msg = SPAPR_VTPM_PREPARE_TO_SUSPEND |
+                            SPAPR_VTPM_MSG_RESULT;
+            spapr_tpm_send_crq(dev, &local_crq);
+            break;
+
+        default:
+            trace_tpm_spapr_do_crq_unknown_msg_type(crq->msg);
+        }
+        break;
+    default:
+        trace_tpm_spapr_do_crq_unknown_crq(valid, msg);
+    };
+
+    return H_SUCCESS;
+}
+
+static void tpm_spapr_request_completed(TPMIf *ti, int ret)
+{
+    SpaprTpmState *s = VIO_SPAPR_VTPM(ti);
+    TpmCrq *crq = &s->crq;
+    uint32_t len;
+    int rc;
+
+    s->state = SPAPR_VTPM_STATE_COMPLETION;
+
+    /* a max. of be_buffer_size bytes can be transported */
+    len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size);
+    rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data),
+                             s->buffer, len);
+
+    if (trace_event_get_state_backends(TRACE_TPM_SPAPR_SHOW_BUFFER)) {
+        tpm_util_show_buffer(s->buffer, len, "From TPM");
+    }
+
+    crq->valid = SPAPR_VTPM_MSG_RESULT;
+    if (rc == H_SUCCESS) {
+        crq->msg = SPAPR_VTPM_TPM_COMMAND | SPAPR_VTPM_MSG_RESULT;
+        crq->len = cpu_to_be16(len);
+    } else {
+        error_report("%s: DMA write failure", __func__);
+        crq->msg = SPAPR_VTPM_VTPM_ERROR;
+        crq->len = cpu_to_be16(0);
+        crq->data = cpu_to_be32(SPAPR_VTPM_ERR_COPY_OUT_FAILED);
+    }
+
+    rc = spapr_tpm_send_crq(&s->vdev, crq);
+    if (rc) {
+        error_report("%s: Error sending response", __func__);
+    }
+}
+
+static int tpm_spapr_do_startup_tpm(SpaprTpmState *s, size_t buffersize)
+{
+    return tpm_backend_startup_tpm(s->be_driver, buffersize);
+}
+
+static const char *tpm_spapr_get_dt_compatible(SpaprVioDevice *dev)
+{
+    SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
+
+    switch (s->be_tpm_version) {
+    case TPM_VERSION_1_2:
+        return "IBM,vtpm";
+    case TPM_VERSION_2_0:
+        return "IBM,vtpm20";
+    default:
+        g_assert_not_reached();
+    }
+}
+
+static void tpm_spapr_reset(SpaprVioDevice *dev)
+{
+    SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
+
+    s->state = SPAPR_VTPM_STATE_NONE;
+
+    s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
+
+    s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
+                            TPM_SPAPR_BUFFER_MAX);
+
+    tpm_backend_reset(s->be_driver);
+    tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
+}
+
+static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
+{
+    SpaprTpmState *s = VIO_SPAPR_VTPM(ti);
+
+    if (tpm_backend_had_startup_error(s->be_driver)) {
+        return TPM_VERSION_UNSPEC;
+    }
+
+    return tpm_backend_get_tpm_version(s->be_driver);
+}
+
+static const VMStateDescription vmstate_spapr_vtpm = {
+    .name = "tpm-spapr",
+    .unmigratable = 1,
+};
+
+static Property tpm_spapr_properties[] = {
+    DEFINE_SPAPR_PROPERTIES(SpaprTpmState, vdev),
+    DEFINE_PROP_TPMBE("tpmdev", SpaprTpmState, be_driver),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void tpm_spapr_realizefn(SpaprVioDevice *dev, Error **errp)
+{
+    SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
+
+    if (!tpm_find()) {
+        error_setg(errp, "at most one TPM device is permitted");
+        return;
+    }
+
+    dev->crq.SendFunc = tpm_spapr_do_crq;
+
+    if (!s->be_driver) {
+        error_setg(errp, "'tpmdev' property is required");
+        return;
+    }
+}
+
+static void tpm_spapr_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SpaprVioDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
+    TPMIfClass *tc = TPM_IF_CLASS(klass);
+
+    k->realize = tpm_spapr_realizefn;
+    k->reset = tpm_spapr_reset;
+    k->dt_name = "vtpm";
+    k->dt_type = "IBM,vtpm";
+    k->get_dt_compatible = tpm_spapr_get_dt_compatible;
+    k->signal_mask = 0x00000001;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    dc->props = tpm_spapr_properties;
+    k->rtce_window_size = 0x10000000;
+    dc->vmsd = &vmstate_spapr_vtpm;
+
+    tc->model = TPM_MODEL_TPM_SPAPR;
+    tc->get_version = tpm_spapr_get_version;
+    tc->request_completed = tpm_spapr_request_completed;
+}
+
+static const TypeInfo tpm_spapr_info = {
+    .name          = TYPE_TPM_SPAPR,
+    .parent        = TYPE_VIO_SPAPR_DEVICE,
+    .instance_size = sizeof(SpaprTpmState),
+    .class_init    = tpm_spapr_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_TPM_IF },
+        { }
+    }
+};
+
+static void tpm_spapr_register_types(void)
+{
+    type_register_static(&tpm_spapr_info);
+}
+
+type_init(tpm_spapr_register_types)
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 357c9e9a84..9143a8eaa3 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -55,3 +55,15 @@ tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
 
 # tpm_ppi.c
 tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
+
+# hw/tpm/tpm_spapr.c
+tpm_spapr_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"
+tpm_spapr_do_crq(uint8_t raw1, uint8_t raw2) "1st 2 bytes in CRQ: 0x%02x 0x%02x"
+tpm_spapr_do_crq_crq_result(void) "SPAPR_VTPM_INIT_CRQ_RESULT"
+tpm_spapr_do_crq_crq_complete_result(void) "SPAPR_VTPM_INIT_CRQ_COMP_RESULT"
+tpm_spapr_do_crq_tpm_command(void) "got TPM command payload"
+tpm_spapr_do_crq_tpm_get_rtce_buffer_size(size_t buffersize) "response: buffer size is %zu"
+tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u"
+tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend"
+tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x"
+tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..."
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 5b541a71c8..15979a3647 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -45,11 +45,14 @@ typedef struct TPMIfClass {
 
 #define TYPE_TPM_TIS                "tpm-tis"
 #define TYPE_TPM_CRB                "tpm-crb"
+#define TYPE_TPM_SPAPR              "tpm-spapr"
 
 #define TPM_IS_TIS(chr)                             \
     object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS)
 #define TPM_IS_CRB(chr)                             \
     object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
+#define TPM_IS_SPAPR(chr)                           \
+    object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
 
 /* returns NULL unless there is exactly one TPM device */
 static inline TPMIf *tpm_find(void)
diff --git a/qapi/tpm.json b/qapi/tpm.json
index b30323bb6b..63878aa0f4 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -12,11 +12,11 @@
 #
 # @tpm-tis: TPM TIS model
 # @tpm-crb: TPM CRB model (since 2.12)
+# @tpm-spapr: TPM SPAPR model (since 5.0)
 #
 # Since: 1.5
 ##
-{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] }
-
+{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb', 'tpm-spapr' ] }
 ##
 # @query-tpm-models:
 #
@@ -29,7 +29,7 @@
 # Example:
 #
 # -> { "execute": "query-tpm-models" }
-# <- { "return": [ "tpm-tis", "tpm-crb" ] }
+# <- { "return": [ "tpm-tis", "tpm-crb", "tpm-spapr" ] }
 #
 ##
 { 'command': 'query-tpm-models', 'returns': ['TpmModel'] }
-- 
2.21.0



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

* [PATCH v7 4/6] tpm_spapr: Support suspend and resume
  2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
                   ` (2 preceding siblings ...)
  2019-12-19 14:06 ` [PATCH v7 3/6] tpm_spapr: Support TPM for ppc64 using CRQ based interface Stefan Berger
@ 2019-12-19 14:06 ` Stefan Berger
  2020-01-03  0:19   ` David Gibson
  2019-12-19 14:06 ` [PATCH v7 5/6] hw/ppc/Kconfig: Enable TPM_SPAPR as part of PSERIES config Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 6/6] docs: tpm: Add example command line for ppc64 and tpm-spapr Stefan Berger
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:06 UTC (permalink / raw)
  To: qemu-ppc; +Cc: marcandre.lureau, Stefan Berger, qemu-devel, david

Extend the tpm_spapr frontend with VM suspend and resume support.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_spapr.c  | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 hw/tpm/trace-events |  2 ++
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
index ab184fbb82..cf5c7851e7 100644
--- a/hw/tpm/tpm_spapr.c
+++ b/hw/tpm/tpm_spapr.c
@@ -76,6 +76,9 @@ typedef struct {
 
     unsigned char buffer[TPM_SPAPR_BUFFER_MAX];
 
+    uint32_t numbytes; /* number of bytes in suspend_buffer */
+    unsigned char *suspend_buffer;
+
     TPMBackendCmd cmd;
 
     TPMBackend *be_driver;
@@ -240,6 +243,13 @@ static void tpm_spapr_request_completed(TPMIf *ti, int ret)
 
     /* a max. of be_buffer_size bytes can be transported */
     len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size);
+
+    if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
+        /* defer delivery of response until .post_load */
+        s->numbytes = len;
+        return;
+    }
+
     rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data),
                              s->buffer, len);
 
@@ -288,11 +298,13 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
     SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
 
     s->state = SPAPR_VTPM_STATE_NONE;
+    s->numbytes = 0;
 
     s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
 
     s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
                             TPM_SPAPR_BUFFER_MAX);
+    s->suspend_buffer = g_realloc(s->suspend_buffer, s->be_buffer_size);
 
     tpm_backend_reset(s->be_driver);
     tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
@@ -309,9 +321,62 @@ static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
     return tpm_backend_get_tpm_version(s->be_driver);
 }
 
+/* persistent state handling */
+
+static int tpm_spapr_pre_save(void *opaque)
+{
+    SpaprTpmState *s = opaque;
+
+    tpm_backend_finish_sync(s->be_driver);
+
+    if (s->numbytes) {
+        memcpy(s->suspend_buffer, s->buffer, s->numbytes);
+    }
+
+    trace_tpm_spapr_pre_save(s->numbytes);
+    /*
+     * we cannot deliver the results to the VM since DMA would touch VM memory
+     */
+
+    return 0;
+}
+
+static int tpm_spapr_post_load(void *opaque, int version_id)
+{
+    SpaprTpmState *s = opaque;
+
+    if (s->numbytes) {
+        trace_tpm_spapr_post_load();
+
+        memcpy(s->buffer, s->suspend_buffer,
+               MIN(s->numbytes, s->be_buffer_size));
+        /* deliver the results to the VM via DMA */
+        tpm_spapr_request_completed(TPM_IF(s), 0);
+        s->numbytes = 0;
+    }
+
+    return 0;
+}
+
 static const VMStateDescription vmstate_spapr_vtpm = {
     .name = "tpm-spapr",
-    .unmigratable = 1,
+    .version_id = 1,
+    .minimum_version_id = 0,
+    .minimum_version_id_old = 0,
+    .pre_save = tpm_spapr_pre_save,
+    .post_load = tpm_spapr_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_SPAPR_VIO(vdev, SpaprTpmState),
+
+        VMSTATE_UINT8(state, SpaprTpmState),
+        VMSTATE_UINT32(numbytes, SpaprTpmState),
+        VMSTATE_VBUFFER_ALLOC_UINT32(suspend_buffer,
+                                     SpaprTpmState, 0, NULL,
+                                     numbytes),
+        /* remember DMA address */
+        VMSTATE_UINT32(crq.data, SpaprTpmState),
+        VMSTATE_END_OF_LIST(),
+    }
 };
 
 static Property tpm_spapr_properties[] = {
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 9143a8eaa3..5592cec7de 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -67,3 +67,5 @@ tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u"
 tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend"
 tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x"
 tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..."
+tpm_spapr_pre_save(uint32_t v) "Number of TPM response bytes to deliver after resume: %u"
+tpm_spapr_post_load(void) "Delivering TPM response after resume"
-- 
2.21.0



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

* [PATCH v7 5/6] hw/ppc/Kconfig: Enable TPM_SPAPR as part of PSERIES config
  2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
                   ` (3 preceding siblings ...)
  2019-12-19 14:06 ` [PATCH v7 4/6] tpm_spapr: Support suspend and resume Stefan Berger
@ 2019-12-19 14:06 ` Stefan Berger
  2019-12-19 14:06 ` [PATCH v7 6/6] docs: tpm: Add example command line for ppc64 and tpm-spapr Stefan Berger
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:06 UTC (permalink / raw)
  To: qemu-ppc
  Cc: marcandre.lureau, Stefan Berger, Stefan Berger, qemu-devel, david

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/ppc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index f927ec9c74..b5b3519158 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -10,6 +10,7 @@ config PSERIES
     select XICS_SPAPR
     select XIVE_SPAPR
     select MSI_NONBROKEN
+    select TPM_SPAPR
 
 config SPAPR_RNG
     bool
-- 
2.21.0



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

* [PATCH v7 6/6] docs: tpm: Add example command line for ppc64 and tpm-spapr
  2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
                   ` (4 preceding siblings ...)
  2019-12-19 14:06 ` [PATCH v7 5/6] hw/ppc/Kconfig: Enable TPM_SPAPR as part of PSERIES config Stefan Berger
@ 2019-12-19 14:06 ` Stefan Berger
  2020-01-03  0:20   ` David Gibson
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Berger @ 2019-12-19 14:06 UTC (permalink / raw)
  To: qemu-ppc
  Cc: marcandre.lureau, Stefan Berger, Stefan Berger, qemu-devel, david

Add an example to the TPM docs for how to add a TPM SPAPR
device model to a QEMU VM emulating a pSeries machine.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 docs/specs/tpm.txt | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 9c8cca042d..9c3e67d8a7 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -34,6 +34,12 @@ The CRB interface makes a memory mapped IO region in the area 0xfed40000 -
 QEMU files related to TPM CRB interface:
  - hw/tpm/tpm_crb.c
 
+
+pSeries (ppc64) machines offer a tpm-spapr device model.
+
+QEMU files related to the SPAPR interface:
+ - hw/tpm/tpm_spapr.c
+
 = fw_cfg interface =
 
 The bios/firmware may read the "etc/tpm/config" fw_cfg entry for
@@ -281,7 +287,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
   --log level=20
 
 Command line to start QEMU with the TPM emulator device communicating with
-the swtpm:
+the swtpm (x86):
 
 qemu-system-x86_64 -display sdl -accel kvm \
   -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
@@ -289,6 +295,18 @@ qemu-system-x86_64 -display sdl -accel kvm \
   -tpmdev emulator,id=tpm0,chardev=chrtpm \
   -device tpm-tis,tpmdev=tpm0 test.img
 
+In case a pSeries machine is emulated, use the following command line:
+
+qemu-system-ppc64 -display sdl -machine pseries,accel=kvm \
+  -m 1024 -bios slof.bin -boot menu=on \
+  -nodefaults -device VGA -device pci-ohci -device usb-kbd \
+  -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+  -tpmdev emulator,id=tpm0,chardev=chrtpm \
+  -device tpm-spapr,tpmdev=tpm0 \
+  -device spapr-vscsi,id=scsi0,reg=0x00002000 \
+  -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 \
+  -drive file=test.img,format=raw,if=none,id=drive-virtio-disk0
+
 
 In case SeaBIOS is used as firmware, it should show the TPM menu item
 after entering the menu with 'ESC'.
-- 
2.21.0



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

* Re: [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c
  2019-12-19 14:06 ` [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c Stefan Berger
@ 2019-12-19 14:29   ` Philippe Mathieu-Daudé
  2019-12-20  8:23   ` David Gibson
  1 sibling, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-19 14:29 UTC (permalink / raw)
  To: Stefan Berger, qemu-ppc
  Cc: marcandre.lureau, david, qemu-devel, Stefan Berger

On 12/19/19 3:06 PM, Stefan Berger wrote:
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   hw/tpm/tpm_tis.c    | 32 ++++----------------------------
>   hw/tpm/tpm_util.c   | 25 +++++++++++++++++++++++++
>   hw/tpm/tpm_util.h   |  3 +++
>   hw/tpm/trace-events |  2 +-
>   4 files changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 7aaf9b946d..5b17c88a7d 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -107,30 +107,6 @@ static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
>       return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
>   }
>   
> -static void tpm_tis_show_buffer(const unsigned char *buffer,
> -                                size_t buffer_size, const char *string)
> -{
> -    size_t len, i;
> -    char *line_buffer, *p;
> -
> -    len = MIN(tpm_cmd_get_size(buffer), buffer_size);
> -
> -    /*
> -     * allocate enough room for 3 chars per buffer entry plus a
> -     * newline after every 16 chars and a final null terminator.
> -     */
> -    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
> -
> -    for (i = 0, p = line_buffer; i < len; i++) {
> -        if (i && !(i % 16)) {
> -            p += sprintf(p, "\n");
> -        }
> -        p += sprintf(p, "%.2X ", buffer[i]);
> -    }
> -    trace_tpm_tis_show_buffer(string, len, line_buffer);
> -
> -    g_free(line_buffer);
> -}
>   
>   /*
>    * Set the given flags in the STS register by clearing the register but
> @@ -156,8 +132,8 @@ static void tpm_tis_sts_set(TPMLocality *l, uint32_t flags)
>    */
>   static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
>   {
> -    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
> -        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
> +    if (trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
> +        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
>       }
>   
>       /*
> @@ -325,8 +301,8 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
>       s->loc[locty].state = TPM_TIS_STATE_COMPLETION;
>       s->rw_offset = 0;
>   
> -    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
> -        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
> +    if (trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
> +        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
>       }
>   
>       if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 62b091f0c0..c0a0f3d71f 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -350,3 +350,28 @@ void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
>       tsb->buffer = NULL;
>       tsb->size = 0;
>   }
> +
> +void tpm_util_show_buffer(const unsigned char *buffer,
> +                          size_t buffer_size, const char *string)
> +{
> +    size_t len, i;
> +    char *line_buffer, *p;
> +
> +    len = MIN(tpm_cmd_get_size(buffer), buffer_size);
> +
> +    /*
> +     * allocate enough room for 3 chars per buffer entry plus a
> +     * newline after every 16 chars and a final null terminator.
> +     */
> +    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
> +
> +    for (i = 0, p = line_buffer; i < len; i++) {
> +        if (i && !(i % 16)) {
> +            p += sprintf(p, "\n");
> +        }
> +        p += sprintf(p, "%.2X ", buffer[i]);
> +    }
> +    trace_tpm_util_show_buffer(string, len, line_buffer);
> +
> +    g_free(line_buffer);
> +}
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index f397ac21b8..7889081fba 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -79,4 +79,7 @@ typedef struct TPMSizedBuffer {
>   
>   void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
>   
> +void tpm_util_show_buffer(const unsigned char *buffer,
> +                          size_t buffer_size, const char *string);
> +
>   #endif /* TPM_TPM_UTIL_H */
> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> index 89804bcd64..357c9e9a84 100644
> --- a/hw/tpm/trace-events
> +++ b/hw/tpm/trace-events
> @@ -14,6 +14,7 @@ tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len = %u,
>   tpm_util_get_buffer_size_hdr_len2(uint32_t len, size_t expected) "tpm2_resp->hdr.len = %u, expected = %zu"
>   tpm_util_get_buffer_size_len2(uint32_t len, size_t expected) "tpm2_resp->len = %u, expected = %zu"
>   tpm_util_get_buffer_size(size_t len) "buffersize of device: %zu"
> +tpm_util_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"

Please avoid multi-line trace formats if possible.
Since this is a pre-existing issue:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>   
>   # tpm_emulator.c
>   tpm_emulator_set_locality(uint8_t locty) "setting locality to %d"
> @@ -36,7 +37,6 @@ tpm_emulator_pre_save(void) ""
>   tpm_emulator_inst_init(void) ""
>   
>   # tpm_tis.c
> -tpm_tis_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\nbuf: %s"
>   tpm_tis_raise_irq(uint32_t irqmask) "Raising IRQ for flag 0x%08x"
>   tpm_tis_new_active_locality(uint8_t locty) "Active locality is now %d"
>   tpm_tis_abort(uint8_t locty) "New active locality is %d"
> 



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

* Re: [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c
  2019-12-19 14:06 ` [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c Stefan Berger
  2019-12-19 14:29   ` Philippe Mathieu-Daudé
@ 2019-12-20  8:23   ` David Gibson
  2019-12-20 12:42     ` Stefan Berger
  1 sibling, 1 reply; 13+ messages in thread
From: David Gibson @ 2019-12-20  8:23 UTC (permalink / raw)
  To: Stefan Berger; +Cc: marcandre.lureau, qemu-ppc, qemu-devel, Stefan Berger

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

On Thu, Dec 19, 2019 at 09:06:00AM -0500, Stefan Berger wrote:
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Do you want me to queue this in my ppc tree?

> ---
>  hw/tpm/tpm_tis.c    | 32 ++++----------------------------
>  hw/tpm/tpm_util.c   | 25 +++++++++++++++++++++++++
>  hw/tpm/tpm_util.h   |  3 +++
>  hw/tpm/trace-events |  2 +-
>  4 files changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 7aaf9b946d..5b17c88a7d 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -107,30 +107,6 @@ static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
>      return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
>  }
>  
> -static void tpm_tis_show_buffer(const unsigned char *buffer,
> -                                size_t buffer_size, const char *string)
> -{
> -    size_t len, i;
> -    char *line_buffer, *p;
> -
> -    len = MIN(tpm_cmd_get_size(buffer), buffer_size);
> -
> -    /*
> -     * allocate enough room for 3 chars per buffer entry plus a
> -     * newline after every 16 chars and a final null terminator.
> -     */
> -    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
> -
> -    for (i = 0, p = line_buffer; i < len; i++) {
> -        if (i && !(i % 16)) {
> -            p += sprintf(p, "\n");
> -        }
> -        p += sprintf(p, "%.2X ", buffer[i]);
> -    }
> -    trace_tpm_tis_show_buffer(string, len, line_buffer);
> -
> -    g_free(line_buffer);
> -}
>  
>  /*
>   * Set the given flags in the STS register by clearing the register but
> @@ -156,8 +132,8 @@ static void tpm_tis_sts_set(TPMLocality *l, uint32_t flags)
>   */
>  static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
>  {
> -    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
> -        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
> +    if (trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
> +        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
>      }
>  
>      /*
> @@ -325,8 +301,8 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
>      s->loc[locty].state = TPM_TIS_STATE_COMPLETION;
>      s->rw_offset = 0;
>  
> -    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
> -        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
> +    if (trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
> +        tpm_util_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
>      }
>  
>      if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 62b091f0c0..c0a0f3d71f 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -350,3 +350,28 @@ void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
>      tsb->buffer = NULL;
>      tsb->size = 0;
>  }
> +
> +void tpm_util_show_buffer(const unsigned char *buffer,
> +                          size_t buffer_size, const char *string)
> +{
> +    size_t len, i;
> +    char *line_buffer, *p;
> +
> +    len = MIN(tpm_cmd_get_size(buffer), buffer_size);
> +
> +    /*
> +     * allocate enough room for 3 chars per buffer entry plus a
> +     * newline after every 16 chars and a final null terminator.
> +     */
> +    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
> +
> +    for (i = 0, p = line_buffer; i < len; i++) {
> +        if (i && !(i % 16)) {
> +            p += sprintf(p, "\n");
> +        }
> +        p += sprintf(p, "%.2X ", buffer[i]);
> +    }
> +    trace_tpm_util_show_buffer(string, len, line_buffer);
> +
> +    g_free(line_buffer);
> +}
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index f397ac21b8..7889081fba 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -79,4 +79,7 @@ typedef struct TPMSizedBuffer {
>  
>  void tpm_sized_buffer_reset(TPMSizedBuffer *tsb);
>  
> +void tpm_util_show_buffer(const unsigned char *buffer,
> +                          size_t buffer_size, const char *string);
> +
>  #endif /* TPM_TPM_UTIL_H */
> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> index 89804bcd64..357c9e9a84 100644
> --- a/hw/tpm/trace-events
> +++ b/hw/tpm/trace-events
> @@ -14,6 +14,7 @@ tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len = %u,
>  tpm_util_get_buffer_size_hdr_len2(uint32_t len, size_t expected) "tpm2_resp->hdr.len = %u, expected = %zu"
>  tpm_util_get_buffer_size_len2(uint32_t len, size_t expected) "tpm2_resp->len = %u, expected = %zu"
>  tpm_util_get_buffer_size(size_t len) "buffersize of device: %zu"
> +tpm_util_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\n%s"
>  
>  # tpm_emulator.c
>  tpm_emulator_set_locality(uint8_t locty) "setting locality to %d"
> @@ -36,7 +37,6 @@ tpm_emulator_pre_save(void) ""
>  tpm_emulator_inst_init(void) ""
>  
>  # tpm_tis.c
> -tpm_tis_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\nbuf: %s"
>  tpm_tis_raise_irq(uint32_t irqmask) "Raising IRQ for flag 0x%08x"
>  tpm_tis_new_active_locality(uint8_t locty) "Active locality is now %d"
>  tpm_tis_abort(uint8_t locty) "New active locality is %d"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c
  2019-12-20  8:23   ` David Gibson
@ 2019-12-20 12:42     ` Stefan Berger
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Berger @ 2019-12-20 12:42 UTC (permalink / raw)
  To: David Gibson, Stefan Berger; +Cc: marcandre.lureau, qemu-ppc, qemu-devel

On 12/20/19 3:23 AM, David Gibson wrote:
> On Thu, Dec 19, 2019 at 09:06:00AM -0500, Stefan Berger wrote:
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> Do you want me to queue this in my ppc tree?


Either you or me. It's fine if you queue it. Thanks.


    Stefan



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

* Re: [PATCH v7 4/6] tpm_spapr: Support suspend and resume
  2019-12-19 14:06 ` [PATCH v7 4/6] tpm_spapr: Support suspend and resume Stefan Berger
@ 2020-01-03  0:19   ` David Gibson
  2020-01-03 15:45     ` Stefan Berger
  0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2020-01-03  0:19 UTC (permalink / raw)
  To: Stefan Berger; +Cc: marcandre.lureau, qemu-ppc, qemu-devel

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

On Thu, Dec 19, 2019 at 09:06:03AM -0500, Stefan Berger wrote:
> Extend the tpm_spapr frontend with VM suspend and resume support.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/tpm/tpm_spapr.c  | 67 ++++++++++++++++++++++++++++++++++++++++++++-
>  hw/tpm/trace-events |  2 ++
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
> index ab184fbb82..cf5c7851e7 100644
> --- a/hw/tpm/tpm_spapr.c
> +++ b/hw/tpm/tpm_spapr.c
> @@ -76,6 +76,9 @@ typedef struct {
>  
>      unsigned char buffer[TPM_SPAPR_BUFFER_MAX];
>  
> +    uint32_t numbytes; /* number of bytes in suspend_buffer */
> +    unsigned char *suspend_buffer;
> +
>      TPMBackendCmd cmd;
>  
>      TPMBackend *be_driver;
> @@ -240,6 +243,13 @@ static void tpm_spapr_request_completed(TPMIf *ti, int ret)
>  
>      /* a max. of be_buffer_size bytes can be transported */
>      len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size);
> +
> +    if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
> +        /* defer delivery of response until .post_load */
> +        s->numbytes = len;
> +        return;
> +    }

I'm not understanding the basics of what's going on here.  IIUC, the
backend TPM can complete a request after we've entered migration
downtime.  But if that's the case, I can't see any guarantee that we
won't have already transmitted the TPM device state, so updating it
here might never reach the destination.  In that case we'd still lose
the request, so I'm not sure what we're accomplishing here.

>      rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data),
>                               s->buffer, len);
>  
> @@ -288,11 +298,13 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
>      SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
>  
>      s->state = SPAPR_VTPM_STATE_NONE;
> +    s->numbytes = 0;
>  
>      s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
>  
>      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
>                              TPM_SPAPR_BUFFER_MAX);
> +    s->suspend_buffer = g_realloc(s->suspend_buffer, s->be_buffer_size);
>  
>      tpm_backend_reset(s->be_driver);
>      tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
> @@ -309,9 +321,62 @@ static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
>      return tpm_backend_get_tpm_version(s->be_driver);
>  }
>  
> +/* persistent state handling */
> +
> +static int tpm_spapr_pre_save(void *opaque)
> +{
> +    SpaprTpmState *s = opaque;
> +
> +    tpm_backend_finish_sync(s->be_driver);
> +
> +    if (s->numbytes) {
> +        memcpy(s->suspend_buffer, s->buffer, s->numbytes);
> +    }
> +
> +    trace_tpm_spapr_pre_save(s->numbytes);
> +    /*
> +     * we cannot deliver the results to the VM since DMA would touch VM memory
> +     */
> +
> +    return 0;
> +}
> +
> +static int tpm_spapr_post_load(void *opaque, int version_id)
> +{
> +    SpaprTpmState *s = opaque;
> +
> +    if (s->numbytes) {
> +        trace_tpm_spapr_post_load();
> +
> +        memcpy(s->buffer, s->suspend_buffer,
> +               MIN(s->numbytes, s->be_buffer_size));
> +        /* deliver the results to the VM via DMA */
> +        tpm_spapr_request_completed(TPM_IF(s), 0);
> +        s->numbytes = 0;
> +    }
> +
> +    return 0;
> +}
> +
>  static const VMStateDescription vmstate_spapr_vtpm = {
>      .name = "tpm-spapr",
> -    .unmigratable = 1,
> +    .version_id = 1,
> +    .minimum_version_id = 0,
> +    .minimum_version_id_old = 0,
> +    .pre_save = tpm_spapr_pre_save,
> +    .post_load = tpm_spapr_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_SPAPR_VIO(vdev, SpaprTpmState),
> +
> +        VMSTATE_UINT8(state, SpaprTpmState),
> +        VMSTATE_UINT32(numbytes, SpaprTpmState),
> +        VMSTATE_VBUFFER_ALLOC_UINT32(suspend_buffer,
> +                                     SpaprTpmState, 0, NULL,
> +                                     numbytes),
> +        /* remember DMA address */
> +        VMSTATE_UINT32(crq.data, SpaprTpmState),
> +        VMSTATE_END_OF_LIST(),
> +    }
>  };
>  
>  static Property tpm_spapr_properties[] = {
> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> index 9143a8eaa3..5592cec7de 100644
> --- a/hw/tpm/trace-events
> +++ b/hw/tpm/trace-events
> @@ -67,3 +67,5 @@ tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u"
>  tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend"
>  tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x"
>  tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..."
> +tpm_spapr_pre_save(uint32_t v) "Number of TPM response bytes to deliver after resume: %u"
> +tpm_spapr_post_load(void) "Delivering TPM response after resume"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v7 6/6] docs: tpm: Add example command line for ppc64 and tpm-spapr
  2019-12-19 14:06 ` [PATCH v7 6/6] docs: tpm: Add example command line for ppc64 and tpm-spapr Stefan Berger
@ 2020-01-03  0:20   ` David Gibson
  0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2020-01-03  0:20 UTC (permalink / raw)
  To: Stefan Berger; +Cc: marcandre.lureau, qemu-ppc, qemu-devel, Stefan Berger

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

On Thu, Dec 19, 2019 at 09:06:05AM -0500, Stefan Berger wrote:
> Add an example to the TPM docs for how to add a TPM SPAPR
> device model to a QEMU VM emulating a pSeries machine.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

I don't see any advantage to splitting this out - it can be merged
into 3/6.

> ---
>  docs/specs/tpm.txt | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 9c8cca042d..9c3e67d8a7 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -34,6 +34,12 @@ The CRB interface makes a memory mapped IO region in the area 0xfed40000 -
>  QEMU files related to TPM CRB interface:
>   - hw/tpm/tpm_crb.c
>  
> +
> +pSeries (ppc64) machines offer a tpm-spapr device model.
> +
> +QEMU files related to the SPAPR interface:
> + - hw/tpm/tpm_spapr.c
> +
>  = fw_cfg interface =
>  
>  The bios/firmware may read the "etc/tpm/config" fw_cfg entry for
> @@ -281,7 +287,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
>    --log level=20
>  
>  Command line to start QEMU with the TPM emulator device communicating with
> -the swtpm:
> +the swtpm (x86):
>  
>  qemu-system-x86_64 -display sdl -accel kvm \
>    -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
> @@ -289,6 +295,18 @@ qemu-system-x86_64 -display sdl -accel kvm \
>    -tpmdev emulator,id=tpm0,chardev=chrtpm \
>    -device tpm-tis,tpmdev=tpm0 test.img
>  
> +In case a pSeries machine is emulated, use the following command line:
> +
> +qemu-system-ppc64 -display sdl -machine pseries,accel=kvm \
> +  -m 1024 -bios slof.bin -boot menu=on \
> +  -nodefaults -device VGA -device pci-ohci -device usb-kbd \
> +  -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
> +  -tpmdev emulator,id=tpm0,chardev=chrtpm \
> +  -device tpm-spapr,tpmdev=tpm0 \
> +  -device spapr-vscsi,id=scsi0,reg=0x00002000 \
> +  -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 \
> +  -drive file=test.img,format=raw,if=none,id=drive-virtio-disk0
> +
>  
>  In case SeaBIOS is used as firmware, it should show the TPM menu item
>  after entering the menu with 'ESC'.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v7 4/6] tpm_spapr: Support suspend and resume
  2020-01-03  0:19   ` David Gibson
@ 2020-01-03 15:45     ` Stefan Berger
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Berger @ 2020-01-03 15:45 UTC (permalink / raw)
  To: David Gibson, Stefan Berger; +Cc: marcandre.lureau, qemu-ppc, qemu-devel

On 1/2/20 7:19 PM, David Gibson wrote:
> On Thu, Dec 19, 2019 at 09:06:03AM -0500, Stefan Berger wrote:
>> Extend the tpm_spapr frontend with VM suspend and resume support.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   hw/tpm/tpm_spapr.c  | 67 ++++++++++++++++++++++++++++++++++++++++++++-
>>   hw/tpm/trace-events |  2 ++
>>   2 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
>> index ab184fbb82..cf5c7851e7 100644
>> --- a/hw/tpm/tpm_spapr.c
>> +++ b/hw/tpm/tpm_spapr.c
>> @@ -76,6 +76,9 @@ typedef struct {
>>   
>>       unsigned char buffer[TPM_SPAPR_BUFFER_MAX];
>>   
>> +    uint32_t numbytes; /* number of bytes in suspend_buffer */
>> +    unsigned char *suspend_buffer;
>> +
>>       TPMBackendCmd cmd;
>>   
>>       TPMBackend *be_driver;
>> @@ -240,6 +243,13 @@ static void tpm_spapr_request_completed(TPMIf *ti, int ret)
>>   
>>       /* a max. of be_buffer_size bytes can be transported */
>>       len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size);
>> +
>> +    if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
>> +        /* defer delivery of response until .post_load */
>> +        s->numbytes = len;
>> +        return;
>> +    }
> I'm not understanding the basics of what's going on here.  IIUC, the
> backend TPM can complete a request after we've entered migration


The tpm_backend_finish_sync() in the .pre_save function will delay the 
writing of the device state until all outstanding responses have been 
captured. In case a response was outstanding, the above s->numbytes will 
then be set and looked at after return from tpm_backend_finish_sync() -- 
see below.


> downtime.  But if that's the case, I can't see any guarantee that we
> won't have already transmitted the TPM device state, so updating it
> here might never reach the destination.  In that case we'd still lose
> the request, so I'm not sure what we're accomplishing here.
>
>>       rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data),
>>                                s->buffer, len);
>>   
>> @@ -288,11 +298,13 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
>>       SpaprTpmState *s = VIO_SPAPR_VTPM(dev);
>>   
>>       s->state = SPAPR_VTPM_STATE_NONE;
>> +    s->numbytes = 0;
>>   
>>       s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver);
>>   
>>       s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
>>                               TPM_SPAPR_BUFFER_MAX);
>> +    s->suspend_buffer = g_realloc(s->suspend_buffer, s->be_buffer_size);
>>   
>>       tpm_backend_reset(s->be_driver);
>>       tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
>> @@ -309,9 +321,62 @@ static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
>>       return tpm_backend_get_tpm_version(s->be_driver);
>>   }
>>   
>> +/* persistent state handling */
>> +
>> +static int tpm_spapr_pre_save(void *opaque)
>> +{
>> +    SpaprTpmState *s = opaque;
>> +
>> +    tpm_backend_finish_sync(s->be_driver);
>> +
>> +    if (s->numbytes) {
>> +        memcpy(s->suspend_buffer, s->buffer, s->numbytes);
>> +    }
>> +
>> +    trace_tpm_spapr_pre_save(s->numbytes);
>> +    /*
>> +     * we cannot deliver the results to the VM since DMA would touch VM memory
>> +     */
>> +
>> +    return 0;
>> +}
>> +
>> +static int tpm_spapr_post_load(void *opaque, int version_id)
>> +{
>> +    SpaprTpmState *s = opaque;
>> +
>> +    if (s->numbytes) {
>> +        trace_tpm_spapr_post_load();
>> +
>> +        memcpy(s->buffer, s->suspend_buffer,
>> +               MIN(s->numbytes, s->be_buffer_size));
>> +        /* deliver the results to the VM via DMA */
>> +        tpm_spapr_request_completed(TPM_IF(s), 0);
>> +        s->numbytes = 0;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static const VMStateDescription vmstate_spapr_vtpm = {
>>       .name = "tpm-spapr",
>> -    .unmigratable = 1,
>> +    .version_id = 1,
>> +    .minimum_version_id = 0,
>> +    .minimum_version_id_old = 0,
>> +    .pre_save = tpm_spapr_pre_save,
>> +    .post_load = tpm_spapr_post_load,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_SPAPR_VIO(vdev, SpaprTpmState),
>> +
>> +        VMSTATE_UINT8(state, SpaprTpmState),
>> +        VMSTATE_UINT32(numbytes, SpaprTpmState),
>> +        VMSTATE_VBUFFER_ALLOC_UINT32(suspend_buffer,
>> +                                     SpaprTpmState, 0, NULL,
>> +                                     numbytes),
>> +        /* remember DMA address */
>> +        VMSTATE_UINT32(crq.data, SpaprTpmState),
>> +        VMSTATE_END_OF_LIST(),
>> +    }
>>   };
>>   
>>   static Property tpm_spapr_properties[] = {
>> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
>> index 9143a8eaa3..5592cec7de 100644
>> --- a/hw/tpm/trace-events
>> +++ b/hw/tpm/trace-events
>> @@ -67,3 +67,5 @@ tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u"
>>   tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend"
>>   tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x"
>>   tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..."
>> +tpm_spapr_pre_save(uint32_t v) "Number of TPM response bytes to deliver after resume: %u"
>> +tpm_spapr_post_load(void) "Delivering TPM response after resume"




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

end of thread, other threads:[~2020-01-03 15:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 14:05 [PATCH v7 0/6] Add vTPM emulator support for ppc64 platform Stefan Berger
2019-12-19 14:06 ` [PATCH v7 1/6] tpm: Move tpm_tis_show_buffer to tpm_util.c Stefan Berger
2019-12-19 14:29   ` Philippe Mathieu-Daudé
2019-12-20  8:23   ` David Gibson
2019-12-20 12:42     ` Stefan Berger
2019-12-19 14:06 ` [PATCH v7 2/6] spapr: Implement get_dt_compatible() callback Stefan Berger
2019-12-19 14:06 ` [PATCH v7 3/6] tpm_spapr: Support TPM for ppc64 using CRQ based interface Stefan Berger
2019-12-19 14:06 ` [PATCH v7 4/6] tpm_spapr: Support suspend and resume Stefan Berger
2020-01-03  0:19   ` David Gibson
2020-01-03 15:45     ` Stefan Berger
2019-12-19 14:06 ` [PATCH v7 5/6] hw/ppc/Kconfig: Enable TPM_SPAPR as part of PSERIES config Stefan Berger
2019-12-19 14:06 ` [PATCH v7 6/6] docs: tpm: Add example command line for ppc64 and tpm-spapr Stefan Berger
2020-01-03  0:20   ` David Gibson

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.