* [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM
@ 2017-04-07 14:30 Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class Amarnath Valluri
` (9 more replies)
0 siblings, 10 replies; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
Briefly, Theses set of patches introduces:
- new TPM backend driver to support software TPM emulators(swtpm(1)).
- and few supported fixes/enhancements/cleanup to existing tpm backend code.
The similar idea was initiated earliar(2) by Stefan Berger(CCed) with slightly
different approach, using CUSE. As swtpm has excellent support for unix domain
sockets, hence this implementation uses unix domain sockets to communicate
with
swtpm.
When Qemu is configured with 'emulator' tpm backend, it spawns 'swtpm' and
communicates its via Unix domain sockets.
1) https://github.com/stefanberger/swtpm
2) https://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg00089.html
** Changes in V2:
- Made spawnning swtpm optional
- used QIOChannel instead of plain unix sockets
- incorporated other fixes pointed in v1 review
Amarnath Valluri (9):
tpm-backend: Remove unneeded member variable from backend class
tpm-backend: Move thread handling inside TPMBackend
tpm-backend: Initialize and free data members in it's own methods
tpm-backend: Made few interface methods optional
tmp backend: Add new api to read backend TpmInfo
tpm-backend: Remove unneeded destroy() method from TpmDriverOps
interface
tpm-backend: Move realloc_buffer() implementation to base class
tpm-passthrough: move reusable code to utils
tpm: Added support for TPM emulator
backends/tpm.c | 121 +++--
configure | 15 +-
hmp.c | 31 +-
hw/tpm/Makefile.objs | 1 +
hw/tpm/tpm_emulator.c | 927 +++++++++++++++++++++++++++++++++++++++
hw/tpm/tpm_ioctl.h | 243 ++++++++++
hw/tpm/tpm_passthrough.c | 221 +++-------
hw/tpm/tpm_util.c | 25 ++
hw/tpm/tpm_util.h | 4 +
include/sysemu/tpm_backend.h | 65 ++-
include/sysemu/tpm_backend_int.h | 41 --
qapi-schema.json | 67 ++-
qemu-options.hx | 53 ++-
tpm.c | 36 +-
14 files changed, 1529 insertions(+), 321 deletions(-)
create mode 100644 hw/tpm/tpm_emulator.c
create mode 100644 hw/tpm/tpm_ioctl.h
delete mode 100644 include/sysemu/tpm_backend_int.h
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 18:19 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 2/9] tpm-backend: Move thread handling inside TPMBackend Amarnath Valluri
` (8 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
TPMDriverOps inside TPMBackend is not required, as it is supposed to be a class
member. The only possible reason for keeping in TPMBackend was, to get the
backend type in tpm.c where dedicated backend api, tpm_backend_get_type() is
present.
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
hw/tpm/tpm_passthrough.c | 4 ----
include/sysemu/tpm_backend.h | 1 -
tpm.c | 2 +-
3 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 9234eb3..a0baf5f 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -46,8 +46,6 @@
#define TPM_PASSTHROUGH(obj) \
OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH)
-static const TPMDriverOps tpm_passthrough_driver;
-
/* data structures */
typedef struct TPMPassthruThreadParams {
TPMState *tpm_state;
@@ -462,8 +460,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
/* let frontend set the fe_model to proper value */
tb->fe_model = -1;
- tb->ops = &tpm_passthrough_driver;
-
if (tpm_passthrough_handle_device_opts(opts, tb)) {
goto err_exit;
}
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index b58f52d..e7f590d 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -50,7 +50,6 @@ struct TPMBackend {
enum TpmModel fe_model;
char *path;
char *cancel_path;
- const TPMDriverOps *ops;
QLIST_ENTRY(TPMBackend) list;
};
diff --git a/tpm.c b/tpm.c
index 9a7c711..0ee021a 100644
--- a/tpm.c
+++ b/tpm.c
@@ -258,7 +258,7 @@ static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv)
res->model = drv->fe_model;
res->options = g_new0(TpmTypeOptions, 1);
- switch (drv->ops->type) {
+ switch (tpm_backend_get_type(drv)) {
case TPM_TYPE_PASSTHROUGH:
res->options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
tpo = g_new0(TPMPassthroughOptions, 1);
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 2/9] tpm-backend: Move thread handling inside TPMBackend
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 18:21 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods Amarnath Valluri
` (7 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
Move thread handling inside TPMBackend, this way backend implementations need
not to maintain their own thread life cycle, instead they needs to implement
'handle_request()' class method that always been called from a thread.
This change made tpm_backend_int.h kind of useless, hence removed it.
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
backends/tpm.c | 63 +++++++++++++++++++++++++---------------
hw/tpm/tpm_passthrough.c | 58 +++++-------------------------------
include/sysemu/tpm_backend.h | 32 ++++++++++++--------
include/sysemu/tpm_backend_int.h | 41 --------------------------
4 files changed, 68 insertions(+), 126 deletions(-)
delete mode 100644 include/sysemu/tpm_backend_int.h
diff --git a/backends/tpm.c b/backends/tpm.c
index 536f262..0ec0c67 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -18,7 +18,24 @@
#include "qapi/qmp/qerror.h"
#include "sysemu/tpm.h"
#include "qemu/thread.h"
-#include "sysemu/tpm_backend_int.h"
+
+static void tpm_backend_worker_thread(gpointer data, gpointer user_data)
+{
+ TPMBackend *s = TPM_BACKEND(user_data);
+ TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+
+ assert (k->handle_request != NULL);
+ k->handle_request(s, (TPMBackendCmd)data);
+}
+
+static void tpm_backend_thread_end(TPMBackend *s)
+{
+ if (s->thread_pool) {
+ g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_END, NULL);
+ g_thread_pool_free(s->thread_pool, FALSE, TRUE);
+ s->thread_pool = NULL;
+ }
+}
enum TpmType tpm_backend_get_type(TPMBackend *s)
{
@@ -39,6 +56,8 @@ void tpm_backend_destroy(TPMBackend *s)
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
k->ops->destroy(s);
+
+ tpm_backend_thread_end(s);
}
int tpm_backend_init(TPMBackend *s, TPMState *state,
@@ -46,13 +65,23 @@ int tpm_backend_init(TPMBackend *s, TPMState *state,
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
- return k->ops->init(s, state, datacb);
+ s->tpm_state = state;
+ s->recv_data_callback = datacb;
+
+ return k->ops->init(s);
}
int tpm_backend_startup_tpm(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ /* terminate a running TPM */
+ tpm_backend_thread_end(s);
+
+ s->thread_pool = g_thread_pool_new(tpm_backend_worker_thread, s, 1, TRUE,
+ NULL);
+ g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
+
return k->ops->startup_tpm(s);
}
@@ -72,9 +101,8 @@ size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
void tpm_backend_deliver_request(TPMBackend *s)
{
- TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
-
- k->ops->deliver_request(s);
+ g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD,
+ NULL);
}
void tpm_backend_reset(TPMBackend *s)
@@ -82,6 +110,8 @@ void tpm_backend_reset(TPMBackend *s)
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
k->ops->reset(s);
+
+ tpm_backend_thread_end(s);
}
void tpm_backend_cancel_cmd(TPMBackend *s)
@@ -156,29 +186,15 @@ static void tpm_backend_instance_init(Object *obj)
tpm_backend_prop_get_opened,
tpm_backend_prop_set_opened,
NULL);
-}
-void tpm_backend_thread_deliver_request(TPMBackendThread *tbt)
-{
- g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD, NULL);
}
-void tpm_backend_thread_create(TPMBackendThread *tbt,
- GFunc func, gpointer user_data)
+static void tpm_backend_instance_finalize(Object *obj)
{
- if (!tbt->pool) {
- tbt->pool = g_thread_pool_new(func, user_data, 1, TRUE, NULL);
- g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
- }
-}
+ TPMBackend *s = TPM_BACKEND(obj);
-void tpm_backend_thread_end(TPMBackendThread *tbt)
-{
- if (tbt->pool) {
- g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_END, NULL);
- g_thread_pool_free(tbt->pool, FALSE, TRUE);
- tbt->pool = NULL;
- }
+ g_free(s->id);
+ tpm_backend_thread_end(s);
}
static const TypeInfo tpm_backend_info = {
@@ -186,6 +202,7 @@ static const TypeInfo tpm_backend_info = {
.parent = TYPE_OBJECT,
.instance_size = sizeof(TPMBackend),
.instance_init = tpm_backend_instance_init,
+ .instance_finalize = tpm_backend_instance_finalize,
.class_size = sizeof(TPMBackendClass),
.abstract = true,
};
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index a0baf5f..f50d9cf 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -30,7 +30,6 @@
#include "tpm_int.h"
#include "hw/hw.h"
#include "hw/i386/pc.h"
-#include "sysemu/tpm_backend_int.h"
#include "tpm_tis.h"
#include "tpm_util.h"
@@ -47,20 +46,9 @@
OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH)
/* data structures */
-typedef struct TPMPassthruThreadParams {
- TPMState *tpm_state;
-
- TPMRecvDataCB *recv_data_callback;
- TPMBackend *tb;
-} TPMPassthruThreadParams;
-
struct TPMPassthruState {
TPMBackend parent;
- TPMBackendThread tbt;
-
- TPMPassthruThreadParams tpm_thread_params;
-
char *tpm_dev;
int tpm_fd;
bool tpm_executing;
@@ -214,12 +202,9 @@ static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt,
selftest_done);
}
-static void tpm_passthrough_worker_thread(gpointer data,
- gpointer user_data)
+static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd cmd)
{
- TPMPassthruThreadParams *thr_parms = user_data;
- TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(thr_parms->tb);
- TPMBackendCmd cmd = (TPMBackendCmd)data;
+ TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
bool selftest_done = false;
DPRINTF("tpm_passthrough: processing command type %d\n", cmd);
@@ -227,12 +212,12 @@ 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_data,
+ tb->tpm_state->locty_data,
&selftest_done);
- thr_parms->recv_data_callback(thr_parms->tpm_state,
- thr_parms->tpm_state->locty_number,
- selftest_done);
+ tb->recv_data_callback(tb->tpm_state,
+ tb->tpm_state->locty_number,
+ selftest_done);
break;
case TPM_BACKEND_CMD_INIT:
case TPM_BACKEND_CMD_END:
@@ -248,15 +233,6 @@ static void tpm_passthrough_worker_thread(gpointer data,
*/
static int tpm_passthrough_startup_tpm(TPMBackend *tb)
{
- TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
-
- /* terminate a running TPM */
- tpm_backend_thread_end(&tpm_pt->tbt);
-
- tpm_backend_thread_create(&tpm_pt->tbt,
- tpm_passthrough_worker_thread,
- &tpm_pt->tpm_thread_params);
-
return 0;
}
@@ -268,20 +244,11 @@ static void tpm_passthrough_reset(TPMBackend *tb)
tpm_passthrough_cancel_cmd(tb);
- tpm_backend_thread_end(&tpm_pt->tbt);
-
tpm_pt->had_startup_error = false;
}
-static int tpm_passthrough_init(TPMBackend *tb, TPMState *s,
- TPMRecvDataCB *recv_data_cb)
+static int tpm_passthrough_init(TPMBackend *tb)
{
- TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
-
- tpm_pt->tpm_thread_params.tpm_state = s;
- tpm_pt->tpm_thread_params.recv_data_callback = recv_data_cb;
- tpm_pt->tpm_thread_params.tb = tb;
-
return 0;
}
@@ -315,13 +282,6 @@ static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb)
return sb->size;
}
-static void tpm_passthrough_deliver_request(TPMBackend *tb)
-{
- TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
-
- tpm_backend_thread_deliver_request(&tpm_pt->tbt);
-}
-
static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
{
TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -483,8 +443,6 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
tpm_passthrough_cancel_cmd(tb);
- tpm_backend_thread_end(&tpm_pt->tbt);
-
qemu_close(tpm_pt->tpm_fd);
qemu_close(tpm_pt->cancel_fd);
@@ -520,7 +478,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
.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,
@@ -540,6 +497,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
TPMBackendClass *tbc = TPM_BACKEND_CLASS(klass);
tbc->ops = &tpm_passthrough_driver;
+ tbc->handle_request = tpm_passthrough_handle_request;
}
static const TypeInfo tpm_passthrough_info = {
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index e7f590d..a538a7f 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -29,22 +29,24 @@
typedef struct TPMBackendClass TPMBackendClass;
typedef struct TPMBackend TPMBackend;
-
typedef struct TPMDriverOps TPMDriverOps;
+typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty, bool selftest_done);
-struct TPMBackendClass {
- ObjectClass parent_class;
-
- const TPMDriverOps *ops;
-
- void (*opened)(TPMBackend *s, Error **errp);
-};
+typedef enum TPMBackendCmd {
+ TPM_BACKEND_CMD_INIT = 1,
+ TPM_BACKEND_CMD_PROCESS_CMD,
+ TPM_BACKEND_CMD_END,
+ TPM_BACKEND_CMD_TPM_RESET,
+} TPMBackendCmd;
struct TPMBackend {
Object parent;
/*< protected >*/
bool opened;
+ TPMState *tpm_state;
+ GThreadPool *thread_pool;
+ TPMRecvDataCB *recv_data_callback;
char *id;
enum TpmModel fe_model;
@@ -54,7 +56,15 @@ struct TPMBackend {
QLIST_ENTRY(TPMBackend) list;
};
-typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty, bool selftest_done);
+struct TPMBackendClass {
+ ObjectClass parent_class;
+
+ const TPMDriverOps *ops;
+
+ void (*opened)(TPMBackend *s, Error **errp);
+
+ void (*handle_request)(TPMBackend *s, TPMBackendCmd cmd);
+};
typedef struct TPMSizedBuffer {
uint32_t size;
@@ -71,7 +81,7 @@ struct TPMDriverOps {
void (*destroy)(TPMBackend *t);
/* initialize the backend */
- int (*init)(TPMBackend *t, TPMState *s, TPMRecvDataCB *datacb);
+ int (*init)(TPMBackend *t);
/* start up the TPM on the backend */
int (*startup_tpm)(TPMBackend *t);
/* returns true if nothing will ever answer TPM requests */
@@ -79,8 +89,6 @@ struct TPMDriverOps {
size_t (*realloc_buffer)(TPMSizedBuffer *sb);
- void (*deliver_request)(TPMBackend *t);
-
void (*reset)(TPMBackend *t);
void (*cancel_cmd)(TPMBackend *t);
diff --git a/include/sysemu/tpm_backend_int.h b/include/sysemu/tpm_backend_int.h
deleted file mode 100644
index 00639dd..0000000
--- a/include/sysemu/tpm_backend_int.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * common TPM backend driver functions
- *
- * Copyright (c) 2012-2013 IBM Corporation
- * Authors:
- * Stefan Berger <stefanb@us.ibm.com>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>
- */
-
-#ifndef TPM_BACKEND_INT_H
-#define TPM_BACKEND_INT_H
-
-typedef struct TPMBackendThread {
- GThreadPool *pool;
-} TPMBackendThread;
-
-void tpm_backend_thread_deliver_request(TPMBackendThread *tbt);
-void tpm_backend_thread_create(TPMBackendThread *tbt,
- GFunc func, gpointer user_data);
-void tpm_backend_thread_end(TPMBackendThread *tbt);
-
-typedef enum TPMBackendCmd {
- TPM_BACKEND_CMD_INIT = 1,
- TPM_BACKEND_CMD_PROCESS_CMD,
- TPM_BACKEND_CMD_END,
- TPM_BACKEND_CMD_TPM_RESET,
-} TPMBackendCmd;
-
-#endif /* TPM_BACKEND_INT_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 2/9] tpm-backend: Move thread handling inside TPMBackend Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 18:27 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 4/9] tpm-backend: Made few interface methods optional Amarnath Valluri
` (6 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
Initialize and free TPMBackend data members in it's own instance_init() and
instance_finalize methods.
Took the opportunity to fix object cleanup in tpm_backend_{create,destroy}
methods
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
backends/tpm.c | 8 ++++++--
hw/tpm/tpm_passthrough.c | 18 +++++++++++-------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index 0ec0c67..99d2b2b 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -57,7 +57,7 @@ void tpm_backend_destroy(TPMBackend *s)
k->ops->destroy(s);
- tpm_backend_thread_end(s);
+ object_unref(OBJECT(s));
}
int tpm_backend_init(TPMBackend *s, TPMState *state,
@@ -182,11 +182,13 @@ static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp)
static void tpm_backend_instance_init(Object *obj)
{
+ TPMBackend *s = TPM_BACKEND(obj);
+
object_property_add_bool(obj, "opened",
tpm_backend_prop_get_opened,
tpm_backend_prop_set_opened,
NULL);
-
+ s->fe_model = -1;
}
static void tpm_backend_instance_finalize(Object *obj)
@@ -194,6 +196,8 @@ static void tpm_backend_instance_finalize(Object *obj)
TPMBackend *s = TPM_BACKEND(obj);
g_free(s->id);
+ g_free(s->path);
+ g_free(s->cancel_path);
tpm_backend_thread_end(s);
}
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index f50d9cf..65831b5 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -417,8 +417,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
tb->id = g_strdup(id);
- /* let frontend set the fe_model to proper value */
- tb->fe_model = -1;
if (tpm_passthrough_handle_device_opts(opts, tb)) {
goto err_exit;
@@ -432,7 +430,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
return tb;
err_exit:
- g_free(tb->id);
+ object_unref(obj);
return NULL;
}
@@ -445,10 +443,6 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
qemu_close(tpm_pt->tpm_fd);
qemu_close(tpm_pt->cancel_fd);
-
- g_free(tb->id);
- g_free(tb->path);
- g_free(tb->cancel_path);
g_free(tpm_pt->tpm_dev);
}
@@ -486,10 +480,20 @@ static const TPMDriverOps tpm_passthrough_driver = {
static void tpm_passthrough_inst_init(Object *obj)
{
+ TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
+
+ tpm_pt->tpm_fd = -1;
+ tpm_pt->cancel_fd = -1;
}
static void tpm_passthrough_inst_finalize(Object *obj)
{
+ TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
+
+ tpm_passthrough_cancel_cmd(TPM_BACKEND(obj));
+
+ qemu_close(tpm_pt->tpm_fd);
+ qemu_close(tpm_pt->cancel_fd);
}
static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 4/9] tpm-backend: Made few interface methods optional
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (2 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 18:29 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo Amarnath Valluri
` (5 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
This allows backend implementations left optional interface methods.
For mandatory methods assertion checks added.
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
backends/tpm.c | 28 +++++++++++++++++++++-------
hw/tpm/tpm_passthrough.c | 16 ----------------
2 files changed, 21 insertions(+), 23 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index 99d2b2b..c2be17b 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -48,14 +48,16 @@ const char *tpm_backend_get_desc(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
- return k->ops->desc();
+ return k->ops->desc ? k->ops->desc() : "";
}
void tpm_backend_destroy(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
- k->ops->destroy(s);
+ if (k->ops->destroy) {
+ k->ops->destroy(s);
+ }
object_unref(OBJECT(s));
}
@@ -68,7 +70,7 @@ int tpm_backend_init(TPMBackend *s, TPMState *state,
s->tpm_state = state;
s->recv_data_callback = datacb;
- return k->ops->init(s);
+ return k->ops->init ? k->ops->init(s) : 0;
}
int tpm_backend_startup_tpm(TPMBackend *s)
@@ -82,13 +84,15 @@ int tpm_backend_startup_tpm(TPMBackend *s)
NULL);
g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
- return k->ops->startup_tpm(s);
+ return k->ops->startup_tpm ? k->ops->startup_tpm(s) : 0;
}
bool tpm_backend_had_startup_error(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ assert(k->ops->had_startup_error);
+
return k->ops->had_startup_error(s);
}
@@ -96,6 +100,8 @@ size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ assert(k->ops->realloc_buffer);
+
return k->ops->realloc_buffer(sb);
}
@@ -109,7 +115,9 @@ void tpm_backend_reset(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
- k->ops->reset(s);
+ if (k->ops->reset) {
+ k->ops->reset(s);
+ }
tpm_backend_thread_end(s);
}
@@ -118,6 +126,8 @@ void tpm_backend_cancel_cmd(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ g_assert(k->ops->cancel_cmd);
+
k->ops->cancel_cmd(s);
}
@@ -125,20 +135,24 @@ bool tpm_backend_get_tpm_established_flag(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
- return k->ops->get_tpm_established_flag(s);
+ return k->ops->get_tpm_established_flag ?
+ k->ops->get_tpm_established_flag(s) : false;
}
int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
- return k->ops->reset_tpm_established_flag(s, locty);
+ return k->ops->reset_tpm_established_flag ?
+ k->ops->reset_tpm_established_flag(s, locty) : 0;
}
TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ assert(k->ops->get_tpm_version);
+
return k->ops->get_tpm_version(s);
}
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 65831b5..0f543bd 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -227,15 +227,6 @@ static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd cmd)
}
}
-/*
- * Start the TPM (thread). If it had been started before, then terminate
- * and start it again.
- */
-static int tpm_passthrough_startup_tpm(TPMBackend *tb)
-{
- return 0;
-}
-
static void tpm_passthrough_reset(TPMBackend *tb)
{
TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -247,11 +238,6 @@ static void tpm_passthrough_reset(TPMBackend *tb)
tpm_pt->had_startup_error = false;
}
-static int tpm_passthrough_init(TPMBackend *tb)
-{
- return 0;
-}
-
static bool tpm_passthrough_get_tpm_established_flag(TPMBackend *tb)
{
return false;
@@ -467,8 +453,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
.desc = tpm_passthrough_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,
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (3 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 4/9] tpm-backend: Made few interface methods optional Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 18:51 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface Amarnath Valluri
` (4 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
TPM configuration options are backend implementation details and shall not be
part of base TPMBackend object, and these shall not be accessed directly outside
of the class, hence added a new interface method, get_tpm_options() to
TPMDriverOps., which shall be implemented by the derived classes to return
configured tpm options.
A new tpm backend api - tpm_backend_query_tpm() which uses _get_tpm_options() to
prepare TpmInfo.
Made TPMPassthroughOptions type inherited from newly defined TPMOptions base type.
The TPMOptions base type is intentionally left empty and supposed to be
inherited by all backend implementations to define their tpm configuration
options.
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
backends/tpm.c | 24 +++++++++++++++++--
hmp.c | 10 ++++----
hw/tpm/tpm_passthrough.c | 57 +++++++++++++++++++++++++++++++++++---------
include/sysemu/tpm_backend.h | 25 +++++++++++++++++--
qapi-schema.json | 41 +++++++++++++++----------------
tpm.c | 32 +------------------------
6 files changed, 118 insertions(+), 71 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index c2be17b..c96f462 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -156,6 +156,28 @@ TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
return k->ops->get_tpm_version(s);
}
+TPMOptions *tpm_backend_get_tpm_options(TPMBackend *s)
+{
+ TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+
+ assert(k->ops->get_tpm_options);
+
+ return k->ops->get_tpm_options(s);
+}
+
+TPMInfo *tpm_backend_query_tpm(TPMBackend *s)
+{
+ TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ TPMInfo *info = g_new0(TPMInfo, 1);
+
+ info->type = k->ops->type;
+ info->id = g_strdup(s->id);
+ info->model = s->fe_model;
+ info->options = tpm_backend_get_tpm_options(s);
+
+ return info;
+}
+
static bool tpm_backend_prop_get_opened(Object *obj, Error **errp)
{
TPMBackend *s = TPM_BACKEND(obj);
@@ -210,8 +232,6 @@ static void tpm_backend_instance_finalize(Object *obj)
TPMBackend *s = TPM_BACKEND(obj);
g_free(s->id);
- g_free(s->path);
- g_free(s->cancel_path);
tpm_backend_thread_end(s);
}
diff --git a/hmp.c b/hmp.c
index edb8970..9caf7c8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -955,18 +955,18 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
c, TpmModel_lookup[ti->model]);
monitor_printf(mon, " \\ %s: type=%s",
- ti->id, TpmTypeOptionsKind_lookup[ti->options->type]);
+ ti->id, TpmType_lookup[ti->type]);
- switch (ti->options->type) {
- case TPM_TYPE_OPTIONS_KIND_PASSTHROUGH:
- tpo = ti->options->u.passthrough.data;
+ switch (ti->type) {
+ case TPM_TYPE_PASSTHROUGH:
+ tpo = (TPMPassthroughOptions *)ti->options;
monitor_printf(mon, "%s%s%s%s",
tpo->has_path ? ",path=" : "",
tpo->has_path ? tpo->path : "",
tpo->has_cancel_path ? ",cancel-path=" : "",
tpo->has_cancel_path ? tpo->cancel_path : "");
break;
- case TPM_TYPE_OPTIONS_KIND__MAX:
+ default:
break;
}
monitor_printf(mon, "\n");
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 0f543bd..71bdf25 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -49,6 +49,7 @@
struct TPMPassthruState {
TPMBackend parent;
+ TPMPassthroughOptions *ops;
char *tpm_dev;
int tpm_fd;
bool tpm_executing;
@@ -313,15 +314,14 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
* in Documentation/ABI/stable/sysfs-class-tpm.
* From /dev/tpm0 create /sys/class/misc/tpm0/device/cancel
*/
-static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
+static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
{
- TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
int fd = -1;
char *dev;
char path[PATH_MAX];
- if (tb->cancel_path) {
- fd = qemu_open(tb->cancel_path, O_WRONLY);
+ if (tpm_pt->ops->cancel_path) {
+ fd = qemu_open(tpm_pt->ops->cancel_path, O_WRONLY);
if (fd < 0) {
error_report("Could not open TPM cancel path : %s",
strerror(errno));
@@ -336,7 +336,7 @@ static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
dev) < sizeof(path)) {
fd = qemu_open(path, O_WRONLY);
if (fd >= 0) {
- tb->cancel_path = g_strdup(path);
+ tpm_pt->ops->cancel_path = g_strdup(path);
} else {
error_report("tpm_passthrough: Could not open TPM cancel "
"path %s : %s", path, strerror(errno));
@@ -356,17 +356,24 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
const char *value;
value = qemu_opt_get(opts, "cancel-path");
- tb->cancel_path = g_strdup(value);
+ if (value) {
+ tpm_pt->ops->cancel_path = g_strdup(value);
+ tpm_pt->ops->has_cancel_path = true;
+ } else {
+ tpm_pt->ops->has_cancel_path = false;
+ }
value = qemu_opt_get(opts, "path");
if (!value) {
value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
+ tpm_pt->ops->has_path = false;
+ } else {
+ tpm_pt->ops->has_path = true;
}
+ tpm_pt->ops->path = g_strdup(value);
tpm_pt->tpm_dev = g_strdup(value);
- tb->path = g_strdup(tpm_pt->tpm_dev);
-
tpm_pt->tpm_fd = qemu_open(tpm_pt->tpm_dev, O_RDWR);
if (tpm_pt->tpm_fd < 0) {
error_report("Cannot access TPM device using '%s': %s",
@@ -387,8 +394,8 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
tpm_pt->tpm_fd = -1;
err_free_parameters:
- g_free(tb->path);
- tb->path = NULL;
+ g_free(tpm_pt->ops->path);
+ tpm_pt->ops->path = NULL;
g_free(tpm_pt->tpm_dev);
tpm_pt->tpm_dev = NULL;
@@ -408,7 +415,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
goto err_exit;
}
- tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tb);
+ tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tpm_pt);
if (tpm_pt->cancel_fd < 0) {
goto err_exit;
}
@@ -430,6 +437,30 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
qemu_close(tpm_pt->tpm_fd);
qemu_close(tpm_pt->cancel_fd);
g_free(tpm_pt->tpm_dev);
+
+ qapi_free_TPMPassthroughOptions(tpm_pt->ops);
+}
+
+static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
+{
+ TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
+ TPMPassthroughOptions *ops = g_new0(TPMPassthroughOptions, 1);
+
+ if (!ops) {
+ return NULL;
+ }
+
+ if (tpm_pt->ops->has_path) {
+ ops->has_path = true;
+ ops->path = g_strdup(tpm_pt->ops->path);
+ }
+
+ if (tpm_pt->ops->has_cancel_path) {
+ ops->has_cancel_path = true;
+ ops->cancel_path = g_strdup(tpm_pt->ops->cancel_path);
+ }
+
+ return (TPMOptions *)ops;
}
static const QemuOptDesc tpm_passthrough_cmdline_opts[] = {
@@ -460,12 +491,14 @@ static const TPMDriverOps tpm_passthrough_driver = {
.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,
+ .get_tpm_options = tpm_passthrough_get_tpm_options,
};
static void tpm_passthrough_inst_init(Object *obj)
{
TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
+ tpm_pt->ops = g_new0(TPMPassthroughOptions, 1);
tpm_pt->tpm_fd = -1;
tpm_pt->cancel_fd = -1;
}
@@ -478,6 +511,8 @@ static void tpm_passthrough_inst_finalize(Object *obj)
qemu_close(tpm_pt->tpm_fd);
qemu_close(tpm_pt->cancel_fd);
+
+ qapi_free_TPMPassthroughOptions(tpm_pt->ops);
}
static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index a538a7f..7f4d621 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -48,10 +48,9 @@ struct TPMBackend {
GThreadPool *thread_pool;
TPMRecvDataCB *recv_data_callback;
+ /* <public> */
char *id;
enum TpmModel fe_model;
- char *path;
- char *cancel_path;
QLIST_ENTRY(TPMBackend) list;
};
@@ -98,6 +97,8 @@ struct TPMDriverOps {
int (*reset_tpm_established_flag)(TPMBackend *t, uint8_t locty);
TPMVersion (*get_tpm_version)(TPMBackend *t);
+
+ TPMOptions *(*get_tpm_options)(TPMBackend *t);
};
@@ -230,6 +231,26 @@ void tpm_backend_open(TPMBackend *s, Error **errp);
*/
TPMVersion tpm_backend_get_tpm_version(TPMBackend *s);
+/**
+ * tpm_backend_get_tpm_options:
+ * @s: the backend
+ *
+ * Get the backend configuration options
+ *
+ * Returns newly allocated TPMOptions
+ */
+TPMOptions *tpm_backend_get_tpm_options(TPMBackend *s);
+
+/**
+ * tpm_backend_query_tpm:
+ * @s: the backend
+ *
+ * Query backend tpm info
+ *
+ * Returns newly allocated TPMInfo
+ */
+TPMInfo *tpm_backend_query_tpm(TPMBackend *s);
+
TPMBackend *qemu_find_tpm(const char *id);
const TPMDriverOps *tpm_get_backend_driver(const char *type);
diff --git a/qapi-schema.json b/qapi-schema.json
index b921994..3f1ca20 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5140,6 +5140,16 @@
{ 'command': 'query-tpm-types', 'returns': ['TpmType'] }
##
+# @TPMOptions:
+#
+# Base type for TPM options
+#
+# Since: 2.10
+##
+{ 'struct': 'TPMOptions',
+ 'data': { } }
+
+##
# @TPMPassthroughOptions:
#
# Information about the TPM passthrough type
@@ -5151,20 +5161,9 @@
#
# Since: 1.5
##
-{ 'struct': 'TPMPassthroughOptions', 'data': { '*path' : 'str',
- '*cancel-path' : 'str'} }
+{ 'struct': 'TPMPassthroughOptions', 'base': 'TPMOptions',
+ 'data': { '*path' : 'str', '*cancel-path' : 'str'} }
-##
-# @TpmTypeOptions:
-#
-# A union referencing different TPM backend types' configuration options
-#
-# @type: 'passthrough' The configuration options for the TPM passthrough type
-#
-# Since: 1.5
-##
-{ 'union': 'TpmTypeOptions',
- 'data': { 'passthrough' : 'TPMPassthroughOptions' } }
##
# @TPMInfo:
@@ -5173,6 +5172,8 @@
#
# @id: The Id of the TPM
#
+# @type: The TPM backend type
+#
# @model: The TPM frontend model
#
# @options: The TPM (backend) type configuration options
@@ -5181,8 +5182,9 @@
##
{ 'struct': 'TPMInfo',
'data': {'id': 'str',
+ 'type': 'TpmType',
'model': 'TpmModel',
- 'options': 'TpmTypeOptions' } }
+ 'options': 'TPMOptions' } }
##
# @query-tpm:
@@ -5199,13 +5201,12 @@
# <- { "return":
# [
# { "model": "tpm-tis",
+# "type": "passthrough",
# "options":
-# { "type": "passthrough",
-# "data":
-# { "cancel-path": "/sys/class/misc/tpm0/device/cancel",
-# "path": "/dev/tpm0"
-# }
-# },
+# {
+# "cancel-path": "/sys/class/misc/tpm0/device/cancel",
+# "path": "/dev/tpm0"
+# },
# "id": "tpm0"
# }
# ]
diff --git a/tpm.c b/tpm.c
index 0ee021a..1b6b550 100644
--- a/tpm.c
+++ b/tpm.c
@@ -249,36 +249,6 @@ static const TPMDriverOps *tpm_driver_find_by_type(enum TpmType type)
return NULL;
}
-static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv)
-{
- TPMInfo *res = g_new0(TPMInfo, 1);
- TPMPassthroughOptions *tpo;
-
- res->id = g_strdup(drv->id);
- res->model = drv->fe_model;
- res->options = g_new0(TpmTypeOptions, 1);
-
- switch (tpm_backend_get_type(drv)) {
- case TPM_TYPE_PASSTHROUGH:
- res->options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
- tpo = g_new0(TPMPassthroughOptions, 1);
- res->options->u.passthrough.data = tpo;
- if (drv->path) {
- tpo->path = g_strdup(drv->path);
- tpo->has_path = true;
- }
- if (drv->cancel_path) {
- tpo->cancel_path = g_strdup(drv->cancel_path);
- tpo->has_cancel_path = true;
- }
- break;
- case TPM_TYPE__MAX:
- break;
- }
-
- return res;
-}
-
/*
* Walk the list of active TPM backends and collect information about them
* following the schema description in qapi-schema.json.
@@ -293,7 +263,7 @@ TPMInfoList *qmp_query_tpm(Error **errp)
continue;
}
info = g_new0(TPMInfoList, 1);
- info->value = qmp_query_tpm_inst(drv);
+ info->value = tpm_backend_query_tpm(drv);
if (!cur_item) {
head = cur_item = info;
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (4 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 18:59 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 7/9] tpm-backend: Move realloc_buffer() implementation to base class Amarnath Valluri
` (3 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
As TPMBackend is a Qemu Object, we can use object_unref() inplace of
tpm_backend_destroy() to free the backend object, hence removed destroy() from
TPMDriverOps interface.
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
backends/tpm.c | 11 -----------
hw/tpm/tpm_passthrough.c | 14 --------------
include/sysemu/tpm_backend.h | 7 -------
tpm.c | 2 +-
4 files changed, 1 insertion(+), 33 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index c96f462..3493df6 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s)
return k->ops->desc ? k->ops->desc() : "";
}
-void tpm_backend_destroy(TPMBackend *s)
-{
- TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
-
- if (k->ops->destroy) {
- k->ops->destroy(s);
- }
-
- object_unref(OBJECT(s));
-}
-
int tpm_backend_init(TPMBackend *s, TPMState *state,
TPMRecvDataCB *datacb)
{
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 71bdf25..8e11ed3 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -428,19 +428,6 @@ err_exit:
return NULL;
}
-static void tpm_passthrough_destroy(TPMBackend *tb)
-{
- TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
-
- tpm_passthrough_cancel_cmd(tb);
-
- qemu_close(tpm_pt->tpm_fd);
- qemu_close(tpm_pt->cancel_fd);
- g_free(tpm_pt->tpm_dev);
-
- qapi_free_TPMPassthroughOptions(tpm_pt->ops);
-}
-
static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
{
TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
.opts = tpm_passthrough_cmdline_opts,
.desc = tpm_passthrough_create_desc,
.create = tpm_passthrough_create,
- .destroy = tpm_passthrough_destroy,
.realloc_buffer = tpm_passthrough_realloc_buffer,
.reset = tpm_passthrough_reset,
.had_startup_error = tpm_passthrough_get_startup_error,
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 7f4d621..8f8f133 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -77,7 +77,6 @@ struct TPMDriverOps {
const char *(*desc)(void);
TPMBackend *(*create)(QemuOpts *opts, const char *id);
- void (*destroy)(TPMBackend *t);
/* initialize the backend */
int (*init)(TPMBackend *t);
@@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
const char *tpm_backend_get_desc(TPMBackend *s);
/**
- * tpm_backend_destroy:
- * @s: the backend to destroy
- */
-void tpm_backend_destroy(TPMBackend *s);
-
-/**
* tpm_backend_init:
* @s: the backend to initialized
* @state: TPMState
diff --git a/tpm.c b/tpm.c
index 1b6b550..43d980e 100644
--- a/tpm.c
+++ b/tpm.c
@@ -197,7 +197,7 @@ void tpm_cleanup(void)
QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
QLIST_REMOVE(drv, list);
- tpm_backend_destroy(drv);
+ object_unref(OBJECT(drv));
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 7/9] tpm-backend: Move realloc_buffer() implementation to base class
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (5 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 19:01 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 8/9] tpm-passthrough: move reusable code to utils Amarnath Valluri
` (2 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
Provide base implementation of realloc_buffer(), so that backend implementations
can resue.
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
backends/tpm.c | 9 ++++++++-
hw/tpm/tpm_passthrough.c | 12 ------------
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/backends/tpm.c b/backends/tpm.c
index 3493df6..0da73e6 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -88,8 +88,15 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
{
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+ if (!k->ops->realloc_buffer) {
+ size_t wanted_size = 4096; /* Linux tpm.c buffer size */
- assert(k->ops->realloc_buffer);
+ if (sb->size != wanted_size) {
+ sb->buffer = g_realloc(sb->buffer, wanted_size);
+ sb->size = wanted_size;
+ }
+ return sb->size;
+ }
return k->ops->realloc_buffer(sb);
}
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 8e11ed3..1bffb6d 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -258,17 +258,6 @@ static bool tpm_passthrough_get_startup_error(TPMBackend *tb)
return tpm_pt->had_startup_error;
}
-static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb)
-{
- size_t wanted_size = 4096; /* Linux tpm.c buffer size */
-
- if (sb->size != wanted_size) {
- sb->buffer = g_realloc(sb->buffer, wanted_size);
- sb->size = wanted_size;
- }
- return sb->size;
-}
-
static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
{
TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -470,7 +459,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
.opts = tpm_passthrough_cmdline_opts,
.desc = tpm_passthrough_create_desc,
.create = tpm_passthrough_create,
- .realloc_buffer = tpm_passthrough_realloc_buffer,
.reset = tpm_passthrough_reset,
.had_startup_error = tpm_passthrough_get_startup_error,
.cancel_cmd = tpm_passthrough_cancel_cmd,
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 8/9] tpm-passthrough: move reusable code to utils
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (6 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 7/9] tpm-backend: Move realloc_buffer() implementation to base class Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-25 19:09 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator Amarnath Valluri
2017-04-12 23:52 ` [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM no-reply
9 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
hw/tpm/tpm_passthrough.c | 64 ++++--------------------------------------------
hw/tpm/tpm_util.c | 25 +++++++++++++++++++
hw/tpm/tpm_util.h | 4 +++
3 files changed, 34 insertions(+), 59 deletions(-)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 1bffb6d..ca7c8bb 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -68,27 +68,6 @@ typedef struct TPMPassthruState TPMPassthruState;
static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
-static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
-{
- int ret, remain;
-
- remain = len;
- while (remain > 0) {
- ret = write(fd, buf, remain);
- if (ret < 0) {
- if (errno != EINTR && errno != EAGAIN) {
- return -1;
- }
- } else if (ret == 0) {
- break;
- } else {
- buf += ret;
- remain -= ret;
- }
- }
- return len - remain;
-}
-
static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
{
int ret;
@@ -102,45 +81,12 @@ static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
}
return ret;
}
-
-static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
-{
- struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)buf;
-
- return be32_to_cpu(resp->len);
-}
-
-/*
- * Write an error message in the given output buffer.
- */
-static void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len)
-{
- if (out_len >= sizeof(struct tpm_resp_hdr)) {
- struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
-
- resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
- resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
- resp->errcode = cpu_to_be32(TPM_FAIL);
- }
-}
-
-static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len)
-{
- struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
-
- if (in_len >= sizeof(*hdr)) {
- return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
- }
-
- return false;
-}
-
static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
const uint8_t *in, uint32_t in_len,
uint8_t *out, uint32_t out_len,
bool *selftest_done)
{
- int ret;
+ ssize_t ret;
bool is_selftest;
const struct tpm_resp_hdr *hdr;
@@ -148,9 +94,9 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
tpm_pt->tpm_executing = true;
*selftest_done = false;
- is_selftest = tpm_passthrough_is_selftest(in, in_len);
+ is_selftest = tpm_util_is_selftest(in, in_len);
- ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len);
+ ret = qemu_write_full(tpm_pt->tpm_fd, (const void *)in, (size_t)in_len);
if (ret != in_len) {
if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) {
error_report("tpm_passthrough: error while transmitting data "
@@ -170,7 +116,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
strerror(errno), errno);
}
} else if (ret < sizeof(struct tpm_resp_hdr) ||
- tpm_passthrough_get_size_from_buffer(out) != ret) {
+ ((struct tpm_resp_hdr *)out)->len != ret) {
ret = -1;
error_report("tpm_passthrough: received invalid response "
"packet from TPM");
@@ -183,7 +129,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
err_exit:
if (ret < 0) {
- tpm_write_fatal_error_response(out, out_len);
+ tpm_util_write_fatal_error_response(out, out_len);
}
tpm_pt->tpm_executing = false;
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index 7b35429..fb929f6 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -24,6 +24,31 @@
#include "tpm_int.h"
/*
+ * Write an error message in the given output buffer.
+ */
+void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
+{
+ if (out_len >= sizeof(struct tpm_resp_hdr)) {
+ struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
+
+ resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
+ resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
+ resp->errcode = cpu_to_be32(TPM_FAIL);
+ }
+}
+
+bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
+{
+ struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
+
+ if (in_len >= sizeof(*hdr)) {
+ return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
+ }
+
+ return false;
+}
+
+/*
* A basic test of a TPM device. We expect a well formatted response header
* (error response is fine) within one second.
*/
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index df76245..2f7c961 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -24,6 +24,10 @@
#include "sysemu/tpm_backend.h"
+void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len);
+
+bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len);
+
int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
#endif /* TPM_TPM_UTIL_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (7 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 8/9] tpm-passthrough: move reusable code to utils Amarnath Valluri
@ 2017-04-07 14:30 ` Amarnath Valluri
2017-04-07 14:41 ` Daniel P. Berrange
2017-04-25 19:35 ` Stefan Berger
2017-04-12 23:52 ` [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM no-reply
9 siblings, 2 replies; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-07 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: patrick.ohly, stefanb, marcandre.lureau, berrange, Amarnath Valluri
This change introduces a new TPM backend driver that can communicate with
swtpm(software TPM emulator) using unix domain socket interface.
Swtpm uses two unix sockets, one for plain TPM commands and responses, and one
for out-of-band control messages.
The swtpm and associated tools can be found here:
https://github.com/stefanberger/swtpm
Usage:
# setup TPM state directory
mkdir /tmp/mytpm
chown -R tss:root /tmp/mytpm
/usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
# Ask qemu to use TPM emulator with given tpm state directory
qemu-system-x86_64 \
[...] \
-tpmdev emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
-device tpm-tis,tpmdev=tpm0 \
[...]
Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
configure | 15 +-
hmp.c | 21 ++
hw/tpm/Makefile.objs | 1 +
hw/tpm/tpm_emulator.c | 927 ++++++++++++++++++++++++++++++++++++++++++++++++++
hw/tpm/tpm_ioctl.h | 243 +++++++++++++
qapi-schema.json | 36 +-
qemu-options.hx | 53 ++-
tpm.c | 2 +-
8 files changed, 1289 insertions(+), 9 deletions(-)
create mode 100644 hw/tpm/tpm_emulator.c
create mode 100644 hw/tpm/tpm_ioctl.h
diff --git a/configure b/configure
index 4901b9a..bef41f3 100755
--- a/configure
+++ b/configure
@@ -3347,10 +3347,15 @@ fi
##########################################
# TPM passthrough is only on x86 Linux
-if test "$targetos" = Linux && test "$cpu" = i386 -o "$cpu" = x86_64; then
- tpm_passthrough=$tpm
+if test "$targetos" = Linux; then
+ tpm_emulator=$tpm
+ if test "$cpu" = i386 -o "$cpu" = x86_64; then
+ tpm_passthrough=$tpm
+ else
+ tpm_passthrough=no
+ fi
else
- tpm_passthrough=no
+ tpm_emulator=no
fi
##########################################
@@ -5125,6 +5130,7 @@ echo "gcov enabled $gcov"
echo "TPM support $tpm"
echo "libssh2 support $libssh2"
echo "TPM passthrough $tpm_passthrough"
+echo "TPM emulator $tpm_emulator"
echo "QOM debugging $qom_cast_debug"
echo "lzo support $lzo"
echo "snappy support $snappy"
@@ -5704,6 +5710,9 @@ if test "$tpm" = "yes"; then
if test "$tpm_passthrough" = "yes"; then
echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
fi
+ if test "$tpm_emulator" = "yes"; then
+ echo "CONFIG_TPM_EMULATOR=y" >> $config_host_mak
+ fi
fi
echo "TRACE_BACKENDS=$trace_backends" >> $config_host_mak
diff --git a/hmp.c b/hmp.c
index 9caf7c8..e7fd426 100644
--- a/hmp.c
+++ b/hmp.c
@@ -937,6 +937,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
Error *err = NULL;
unsigned int c = 0;
TPMPassthroughOptions *tpo;
+ TPMEmulatorOptions *teo;
info_list = qmp_query_tpm(&err);
if (err) {
@@ -966,6 +967,26 @@ 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_EMULATOR:
+ teo = (TPMEmulatorOptions *)(ti->options);
+ monitor_printf(mon, ",tmpstatedir=%s", teo->tpmstatedir);
+ monitor_printf(mon, ",spawn=%s", teo->spawn ? "on" : "off");
+ if (teo->has_path) {
+ monitor_printf(mon, ",path=%s", teo->path);
+ }
+ if (teo->has_data_path) {
+ monitor_printf(mon, ",data-path=%s", teo->data_path);
+ }
+ if (teo->has_ctrl_path) {
+ monitor_printf(mon, ",ctrl-path=%s", teo->ctrl_path);
+ }
+ if (teo->has_logfile) {
+ monitor_printf(mon, ",logfile=%s", teo->logfile);
+ }
+ if (teo->has_loglevel) {
+ monitor_printf(mon, ",loglevel=%ld", teo->loglevel);
+ }
+ break;
default:
break;
}
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index 64cecc3..41f0b7a 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,2 +1,3 @@
common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o tpm_util.o
+common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o tpm_util.o
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
new file mode 100644
index 0000000..d001ed9
--- /dev/null
+++ b/hw/tpm/tpm_emulator.c
@@ -0,0 +1,927 @@
+/*
+ * emulator TPM driver
+ *
+ * Copyright (c) 2017 Intel Corporation
+ * Author: Amarnath Valluri <amarnath.valluri@intel.com>
+ *
+ * Copyright (c) 2010 - 2013 IBM Corporation
+ * Authors:
+ * Stefan Berger <stefanb@us.ibm.com>
+ *
+ * Copyright (C) 2011 IAIK, Graz University of Technology
+ * Author: Andreas Niederl
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/sockets.h"
+#include "io/channel-socket.h"
+#include "sysemu/tpm_backend.h"
+#include "tpm_int.h"
+#include "hw/hw.h"
+#include "hw/i386/pc.h"
+#include "tpm_util.h"
+#include "tpm_ioctl.h"
+#include "qapi/error.h"
+
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <stdio.h>
+
+#define DEBUG_TPM 0
+
+#define DPRINT(fmt, ...) do { \
+ if (DEBUG_TPM) { \
+ fprintf(stderr, fmt, ## __VA_ARGS__); \
+ } \
+} while (0);
+
+#define DPRINTF(fmt, ...) DPRINT("tpm-emulator: "fmt"\n", __VA_ARGS__)
+
+#define TYPE_TPM_EMULATOR "tpm-emulator"
+#define TPM_EMULATOR(obj) \
+ OBJECT_CHECK(TPMEmulator, (obj), TYPE_TPM_EMULATOR)
+
+static const TPMDriverOps tpm_emulator_driver;
+
+/* data structures */
+typedef struct TPMEmulator {
+ TPMBackend parent;
+
+ TPMEmulatorOptions *ops;
+ QIOChannel *data_ioc;
+ QIOChannel *ctrl_ioc;
+ bool op_executing;
+ bool op_canceled;
+ bool child_running;
+ TPMVersion tpm_version;
+ ptm_cap caps; /* capabilities of the TPM */
+ uint8_t cur_locty_number; /* last set locality */
+ QemuMutex state_lock;
+} TPMEmulator;
+
+#define TPM_DEFAULT_EMULATOR "swtpm"
+#define TPM_DEFAULT_LOGLEVEL 5
+#define TPM_EMULATOR_PIDFILE "/tmp/qemu-tpm.pid"
+#define TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(S, cap) (((S)->caps & (cap)) == (cap))
+#define TPM_EMULATOR_IOCTL_TO_CMD(ioctlnum) \
+ ((ioctlnum >> _IOC_NRSHIFT) & _IOC_NRMASK) + 1
+
+static int tpm_emulator_ctrlcmd(QIOChannel *ioc, unsigned long cmd, void *msg,
+ size_t msg_len_in, size_t msg_len_out)
+{
+ ssize_t n;
+
+ uint32_t cmd_no = cpu_to_be32(TPM_EMULATOR_IOCTL_TO_CMD(cmd));
+ struct iovec iov[2] = {
+ { .iov_base = &cmd_no, .iov_len = sizeof(cmd_no), },
+ { .iov_base = msg, .iov_len = msg_len_in, },
+ };
+
+ n = qio_channel_writev(ioc, iov, 2, NULL);
+ if (n > 0) {
+ if (msg_len_out > 0) {
+ n = qio_channel_read(ioc, (char *)msg, msg_len_out, NULL);
+ /* simulate ioctl return value */
+ if (n > 0) {
+ n = 0;
+ }
+ } else {
+ n = 0;
+ }
+ }
+ return n;
+}
+
+static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_pt,
+ const uint8_t *in, uint32_t in_len,
+ uint8_t *out, uint32_t out_len,
+ bool *selftest_done)
+{
+ ssize_t ret;
+ bool is_selftest;
+ const struct tpm_resp_hdr *hdr;
+
+ if (!tpm_pt->child_running) {
+ return -1;
+ }
+
+ tpm_pt->op_canceled = false;
+ tpm_pt->op_executing = true;
+ *selftest_done = false;
+
+ is_selftest = tpm_util_is_selftest(in, in_len);
+
+ ret = qio_channel_write(tpm_pt->data_ioc, (const char *)in, (size_t)in_len,
+ NULL);
+ if (ret != in_len) {
+ if (!tpm_pt->op_canceled || errno != ECANCELED) {
+ error_report("tpm-emulator: error while transmitting data "
+ "to TPM: %s (%i)", strerror(errno), errno);
+ }
+ goto err_exit;
+ }
+
+ tpm_pt->op_executing = false;
+
+ ret = qio_channel_read(tpm_pt->data_ioc, (char *)out, (size_t)out_len, NULL);
+ if (ret < 0) {
+ if (!tpm_pt->op_canceled || errno != ECANCELED) {
+ error_report("tpm-emulator: error while reading data from "
+ "TPM: %s (%i)", strerror(errno), errno);
+ }
+ } else if (ret < sizeof(struct tpm_resp_hdr) ||
+ be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
+ ret = -1;
+ error_report("tpm-emulator: received invalid response "
+ "packet from TPM");
+ }
+
+ if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
+ hdr = (struct tpm_resp_hdr *)out;
+ *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
+ }
+
+err_exit:
+ if (ret < 0) {
+ tpm_util_write_fatal_error_response(out, out_len);
+ }
+
+ tpm_pt->op_executing = false;
+
+ return ret;
+}
+
+static int tpm_emulator_set_locality(TPMEmulator *tpm_pt,
+ uint8_t locty_number)
+{
+ ptm_loc loc;
+
+ if (!tpm_pt->child_running) {
+ return -1;
+ }
+
+ DPRINTF("%s : locality: 0x%x", __func__, locty_number);
+
+ if (tpm_pt->cur_locty_number != locty_number) {
+ DPRINTF("setting locality : 0x%x", locty_number);
+ loc.u.req.loc = cpu_to_be32(locty_number);
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_SET_LOCALITY, &loc,
+ sizeof(loc), sizeof(loc)) < 0) {
+ error_report("tpm-emulator: could not set locality : %s",
+ strerror(errno));
+ return -1;
+ }
+ loc.u.resp.tpm_result = be32_to_cpu(loc.u.resp.tpm_result);
+ if (loc.u.resp.tpm_result != 0) {
+ error_report("tpm-emulator: TPM result for set locality : 0x%x",
+ loc.u.resp.tpm_result);
+ return -1;
+ }
+ tpm_pt->cur_locty_number = locty_number;
+ }
+ return 0;
+}
+
+static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd cmd)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+ TPMLocality *locty = NULL;
+ bool selftest_done = false;
+
+ DPRINTF("processing command type %d", cmd);
+
+ switch (cmd) {
+ case TPM_BACKEND_CMD_PROCESS_CMD:
+ qemu_mutex_lock(&tpm_pt->state_lock);
+ locty = tb->tpm_state->locty_data;
+ if (tpm_emulator_set_locality(tpm_pt,
+ tb->tpm_state->locty_number) < 0) {
+ tpm_util_write_fatal_error_response(locty->r_buffer.buffer,
+ locty->r_buffer.size);
+ } else {
+ tpm_emulator_unix_tx_bufs(tpm_pt, locty->w_buffer.buffer,
+ locty->w_offset, locty->r_buffer.buffer,
+ locty->r_buffer.size, &selftest_done);
+ }
+ tb->recv_data_callback(tb->tpm_state, tb->tpm_state->locty_number,
+ selftest_done);
+ qemu_mutex_unlock(&tpm_pt->state_lock);
+ break;
+ case TPM_BACKEND_CMD_INIT:
+ case TPM_BACKEND_CMD_END:
+ case TPM_BACKEND_CMD_TPM_RESET:
+ /* nothing to do */
+ break;
+ }
+}
+
+/*
+ * Gracefully shut down the external unixio TPM
+ */
+static void tpm_emulator_shutdown(TPMEmulator *tpm_pt)
+{
+ ptm_res res;
+
+ if (!tpm_pt->child_running) {
+ return;
+ }
+
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_SHUTDOWN, &res, 0,
+ sizeof(res)) < 0) {
+ error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
+ strerror(errno));
+ } else if (res != 0) {
+ error_report("tpm-emulator: TPM result for sutdown: 0x%x",
+ be32_to_cpu(res));
+ }
+}
+
+static int tpm_emulator_probe_caps(TPMEmulator *tpm_pt)
+{
+ if (!tpm_pt->child_running) {
+ return -1;
+ }
+
+ DPRINTF("%s", __func__);
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_GET_CAPABILITY,
+ &tpm_pt->caps, 0, sizeof(tpm_pt->caps)) < 0) {
+ error_report("tpm-emulator: probing failed : %s", strerror(errno));
+ return -1;
+ }
+
+ tpm_pt->caps = be64_to_cpu(tpm_pt->caps);
+
+ DPRINTF("capbilities : 0x%lx", tpm_pt->caps);
+
+ return 0;
+}
+
+static int tpm_emulator_check_caps(TPMEmulator *tpm_pt)
+{
+ ptm_cap caps = 0;
+ const char *tpm = NULL;
+
+ /* 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-emulator: TPM version has not been set");
+ return -1;
+ }
+
+ if (!TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(tpm_pt, caps)) {
+ error_report("tpm-emulator: TPM does not implement minimum set of "
+ "required capabilities for TPM %s (0x%x)", tpm, (int)caps);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int tpm_emulator_init_tpm(TPMEmulator *tpm_pt)
+{
+ ptm_init init;
+ ptm_res res;
+
+ if (!tpm_pt->child_running) {
+ return -1;
+ }
+
+ DPRINTF("%s", __func__);
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_INIT, &init, sizeof(init),
+ sizeof(init)) < 0) {
+ error_report("tpm-emulator: could not send INIT: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ res = be32_to_cpu(init.u.resp.tpm_result);
+ if (res) {
+ error_report("tpm-emulator: TPM result for PTM_INIT: 0x%x", res);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int tpm_emulator_startup_tpm(TPMBackend *tb)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+
+ DPRINTF("%s", __func__);
+
+ tpm_emulator_init_tpm(tpm_pt) ;
+
+ return 0;
+}
+
+static bool tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+ ptm_est est;
+
+ DPRINTF("%s", __func__);
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_GET_TPMESTABLISHED, &est, 0,
+ sizeof(est)) < 0) {
+ error_report("tpm-emulator: Could not get the TPM established flag: %s",
+ strerror(errno));
+ return false;
+ }
+ DPRINTF("established flag: %0x", est.u.resp.bit);
+
+ return (est.u.resp.bit != 0);
+}
+
+static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
+ uint8_t locty)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+ ptm_reset_est reset_est;
+ ptm_res res;
+
+ /* only a TPM 2.0 will support this */
+ if (tpm_pt->tpm_version == TPM_VERSION_2_0) {
+ reset_est.u.req.loc = cpu_to_be32(tpm_pt->cur_locty_number);
+
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_RESET_TPMESTABLISHED,
+ &reset_est, sizeof(reset_est),
+ sizeof(reset_est)) < 0) {
+ error_report("tpm-emulator: Could not reset the establishment bit: "
+ "%s", strerror(errno));
+ return -1;
+ }
+
+ res = be32_to_cpu(reset_est.u.resp.tpm_result);
+ if (res) {
+ error_report("tpm-emulator: TPM result for rest establixhed flag: "
+ "0x%x", res);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+static bool tpm_emulator_had_startup_error(TPMBackend *tb)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+
+ return !tpm_pt->child_running;
+}
+
+static void tpm_emulator_cancel_cmd(TPMBackend *tb)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+ ptm_res res;
+
+ /*
+ * As of Linux 3.7 the tpm_tis driver does not properly cancel
+ * commands on all TPM manufacturers' TPMs.
+ * Only cancel if we're busy so we don't cancel someone else's
+ * command, e.g., a command executed on the host.
+ */
+ if (tpm_pt->op_executing) {
+ if (TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(tpm_pt, PTM_CAP_CANCEL_TPM_CMD)) {
+ if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_CANCEL_TPM_CMD, &res,
+ 0, sizeof(res)) < 0) {
+ error_report("tpm-emulator: Could not cancel command: %s",
+ strerror(errno));
+ } else if (res != 0) {
+ error_report("tpm-emulator: Failed to cancel TPM: 0x%x",
+ be32_to_cpu(res));
+ } else {
+ tpm_pt->op_canceled = true;
+ }
+ }
+ }
+}
+
+static void tpm_emulator_reset(TPMBackend *tb)
+{
+ DPRINTF("%s", __func__);
+
+ tpm_emulator_cancel_cmd(tb);
+}
+
+static const char *tpm_emulator_desc(void)
+{
+ return "TPM emulator backend driver";
+}
+
+static TPMVersion tpm_emulator_get_tpm_version(TPMBackend *tb)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+
+ return tpm_pt->tpm_version;
+}
+
+static gboolean tpm_emulator_fd_handler(QIOChannel *ioc, GIOCondition cnd, void *opaque)
+{
+ TPMEmulator *tpm_pt = opaque;
+
+ if (cnd & G_IO_ERR || cnd & G_IO_HUP) {
+ error_report("TPM backend disappeared");
+ tpm_pt->child_running = false;
+ return false;
+ }
+
+ return true;
+}
+
+static QIOChannel *_iochannel_new(const char *path, int fd, Error **err)
+{
+ int socket = path ? unix_connect(path, err) : fd;
+ if (socket < 0) {
+ return NULL;
+ }
+
+ return QIO_CHANNEL(qio_channel_socket_new_fd(socket, err));
+}
+
+static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
+{
+ int fds[2] = { -1, -1 };
+ int ctrl_fds[2] = { -1, -1 };
+ pid_t cpid;
+
+ if (!tpm_pt->ops->has_data_path) {
+ if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
+ return -1;
+ }
+ }
+
+ if (!tpm_pt->ops->has_ctrl_path) {
+ if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
+ if (!tpm_pt->ops->has_data_path) {
+ closesocket(fds[0]);
+ closesocket(fds[1]);
+ }
+ return -1;
+ }
+ }
+
+ cpid = qemu_fork(NULL);
+ if (cpid < 0) {
+ error_report("tpm-emulator: Fork failure: %s", strerror(errno));
+ if (!tpm_pt->ops->has_data_path) {
+ closesocket(fds[0]);
+ closesocket(fds[1]);
+ }
+ if (!tpm_pt->ops->has_ctrl_path) {
+ closesocket(ctrl_fds[0]);
+ closesocket(ctrl_fds[1]);
+ }
+ return -1;
+ }
+
+ if (cpid == 0) { /* CHILD */
+ enum {
+ PARAM_PATH,
+ PARAM_IFACE,
+ PARAM_SERVER, PARAM_SERVER_ARGS,
+ PARAM_CTRL, PARAM_CTRL_ARGS,
+ PARAM_STATE, PARAM_STATE_ARGS,
+ PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
+ PARAM_LOG, PARAM_LOG_ARGS,
+ PARAM_MAX
+ };
+
+ int i;
+ int data_fd = -1, ctrl_fd = -1;
+ char *argv[PARAM_MAX+1];
+
+ /* close all unused inherited sockets */
+ if (fds[0] >= 0)
+ closesocket(fds[0]);
+ if (ctrl_fds[0] >= 0)
+ closesocket(ctrl_fds[0]);
+
+ i = STDERR_FILENO + 1;
+ if (fds[1] >= 0) {
+ data_fd = dup2(fds[1], i++);
+ if (data_fd < 0) {
+ error_report("tpm-emulator: dup2() failure - %s",
+ strerror(errno));
+ goto exit_child;
+ }
+ }
+ if (ctrl_fds[1] >= 0) {
+ ctrl_fd = dup2(ctrl_fds[1], i++);
+ if (ctrl_fd < 0) {
+ error_report("tpm-emulator: dup2() failure - %s",
+ strerror(errno));
+ goto exit_child;
+ }
+ }
+ for ( ; i < _SC_OPEN_MAX; i++) {
+ closesocket(i);
+ }
+
+ argv[PARAM_MAX] = NULL;
+ argv[PARAM_PATH] = g_strdup(tpm_pt->ops->path);
+ argv[PARAM_IFACE] = g_strdup("socket");
+ if (tpm_pt->ops->has_data_path) {
+ argv[PARAM_SERVER] = g_strdup("--server");
+ argv[PARAM_SERVER_ARGS] = g_strdup_printf("type=unixio,path=%s",
+ tpm_pt->ops->data_path);
+ } else {
+ argv[PARAM_SERVER] = g_strdup("--fd");
+ argv[PARAM_SERVER_ARGS] = g_strdup_printf("%d", data_fd);
+ }
+
+ argv[PARAM_CTRL] = g_strdup("--ctrl");
+ if (tpm_pt->ops->has_ctrl_path) {
+ argv[PARAM_CTRL_ARGS] = g_strdup_printf("type=unixio,path=%s",
+ tpm_pt->ops->ctrl_path);
+ } else {
+ argv[PARAM_CTRL_ARGS] = g_strdup_printf("type=unixio,clientfd=%d",
+ ctrl_fd);
+ }
+
+ argv[PARAM_STATE] = g_strdup("--tpmstate");
+ argv[PARAM_STATE_ARGS] = g_strdup_printf("dir=%s",
+ tpm_pt->ops->tpmstatedir);
+ argv[PARAM_PIDFILE] = g_strdup("--pid");
+ argv[PARAM_PIDFILE_ARGS] = g_strdup_printf("file=%s",
+ TPM_EMULATOR_PIDFILE);
+ if (tpm_pt->ops->has_logfile) {
+ argv[PARAM_LOG] = g_strdup("--log");
+ argv[PARAM_LOG_ARGS] = g_strdup_printf("file=%s,level=%d",
+ tpm_pt->ops->logfile, (int)tpm_pt->ops->loglevel);
+ } else {
+ /* truncate logs */
+ argv[PARAM_LOG] = NULL;
+ }
+ DPRINTF("%s", "Running cmd: ")
+ for (i = 0; argv[i]; i++) {
+ DPRINT(" %s", argv[i])
+ }
+ DPRINT("\n")
+ if (execv(tpm_pt->ops->path, (char * const *)argv) < 0) {
+ error_report("execv() failure : %s", strerror(errno));
+ }
+
+exit_child:
+ g_strfreev(argv);
+ if (data_fd >= 0)
+ closesocket(data_fd);
+ if (ctrl_fd >= 0)
+ closesocket(ctrl_fd);
+
+ exit(0);
+ } else { /* self */
+ struct stat st;
+ DPRINTF("child pid: %d", cpid);
+ int rc;
+ int timeout = 3; /* wait for max 3 seconds */
+
+ /* close unsed sockets */
+ if (fds[1] >= 0)
+ closesocket(fds[1]);
+ if (ctrl_fds[1] >= 0)
+ closesocket(ctrl_fds[1]);
+
+ while((rc = stat(TPM_EMULATOR_PIDFILE, &st)) < 0 && timeout--) {
+ sleep(1);
+ }
+
+ if (timeout == -1) {
+ error_report("tpm-emulator: failed to find pid file: %s",
+ strerror(errno));
+ goto err_kill_child;
+ }
+
+ tpm_pt->data_ioc = _iochannel_new(tpm_pt->ops->data_path, fds[0], NULL);
+ if (!tpm_pt->data_ioc) {
+ error_report("tpm-emulator: Unable to connect socket : %s",
+ tpm_pt->ops->data_path);
+ goto err_kill_child;
+ }
+
+ tpm_pt->ctrl_ioc = _iochannel_new(tpm_pt->ops->ctrl_path, ctrl_fds[0], NULL);
+ if (!tpm_pt->ctrl_ioc) {
+ error_report("tpm-emulator: Unable to connect socket : %s",
+ tpm_pt->ops->ctrl_path);
+ goto err_kill_child;
+ }
+
+ tpm_pt->child_running = true;
+
+ qemu_add_child_watch(cpid);
+
+ qio_channel_add_watch(tpm_pt->data_ioc, G_IO_HUP|G_IO_ERR,
+ tpm_emulator_fd_handler, tpm_pt, NULL);
+ }
+
+ return 0;
+
+err_kill_child:
+ kill(cpid, SIGTERM);
+ closesocket(fds[0]);
+ closesocket(ctrl_fds[0]);
+ tpm_pt->child_running = false;
+
+ return -1;
+}
+
+static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_pt, QemuOpts *opts)
+{
+ const char *value;
+
+ value = qemu_opt_get(opts, "tpmstatedir");
+ if (!value) {
+ error_report("tpm-emulator: Missing tpm state directory");
+ return -1;
+ }
+ tpm_pt->ops->tpmstatedir = g_strdup(value);
+
+ tpm_pt->ops->spawn = qemu_opt_get_bool(opts, "spawn", false);
+
+ value = qemu_opt_get(opts, "path");
+ if (!value) {
+ value = TPM_DEFAULT_EMULATOR;
+ tpm_pt->ops->has_path = false;
+ } else {
+ tpm_pt->ops->has_path = true;
+ if (value[0] == '/') {
+ struct stat st;
+ if (stat(value, &st) < 0 || !(S_ISREG(st.st_mode)
+ || S_ISLNK(st.st_mode))) {
+ error_report("tpm-emulator: Invalid emulator path: %s", value);
+ return -1;
+ }
+ }
+ }
+ tpm_pt->ops->path = g_strdup(value);
+
+ value = qemu_opt_get(opts, "data-path");
+ if (value) {
+ tpm_pt->ops->has_data_path = true;
+ tpm_pt->ops->data_path = g_strdup(value);
+ } else {
+ tpm_pt->ops->has_data_path = false;
+ if (!tpm_pt->ops->spawn) {
+ error_report("tpm-emulator: missing mandatory data-path");
+ return -1;
+ }
+ }
+
+ value = qemu_opt_get(opts, "ctrl-path");
+ if (value) {
+ tpm_pt->ops->has_ctrl_path = true;
+ tpm_pt->ops->ctrl_path = g_strdup(value);
+ } else {
+ tpm_pt->ops->has_ctrl_path = false;
+ if (!tpm_pt->ops->spawn) {
+ error_report("tpm-emulator: missing mandatory ctrl-path");
+ return -1;
+ }
+ }
+
+ value = qemu_opt_get(opts, "logfile");
+ if (value) {
+ tpm_pt->ops->has_logfile = true;
+ tpm_pt->ops->logfile = g_strdup(value);
+ tpm_pt->ops->loglevel = qemu_opt_get_number(opts, "loglevel",
+ TPM_DEFAULT_LOGLEVEL);
+ tpm_pt->ops->has_loglevel = tpm_pt->ops->loglevel !=
+ TPM_DEFAULT_LOGLEVEL;
+ }
+
+ if (tpm_pt->ops->spawn) {
+ if (tpm_emulator_spawn_emulator(tpm_pt) < 0) {
+ goto err_close_dev;
+ }
+ } else {
+ tpm_pt->data_ioc = _iochannel_new(tpm_pt->ops->data_path, -1, NULL);
+ if (tpm_pt->data_ioc == NULL) {
+ error_report("tpm-emulator: Failed to connect data socket: %s",
+ tpm_pt->ops->data_path);
+ goto err_close_dev;
+ }
+ tpm_pt->ctrl_ioc = _iochannel_new(tpm_pt->ops->ctrl_path, -1, NULL);
+ if (tpm_pt->ctrl_ioc == NULL) {
+ DPRINTF("Failed to connect control socket: %s",
+ strerror(errno));
+ goto err_close_dev;
+ }
+ tpm_pt->child_running = true;
+ }
+
+ if (tpm_emulator_probe_caps(tpm_pt) ||
+ tpm_emulator_init_tpm(tpm_pt)) {
+ goto err_close_dev;
+ }
+
+ /* FIXME: tpm_util_test_tpmdev() accepts only on socket fd, as it also used
+ * by passthrough driver, which not yet using GIOChannel.
+ */
+ if (tpm_util_test_tpmdev(QIO_CHANNEL_SOCKET(tpm_pt->data_ioc)->fd,
+ &tpm_pt->tpm_version)) {
+ error_report("'%s' is not emulating TPM device.", tpm_pt->ops->path);
+ goto err_close_dev;
+ }
+
+ DPRINTF("TPM Version %s", tpm_pt->tpm_version == TPM_VERSION_1_2 ? "1.2" :
+ (tpm_pt->tpm_version == TPM_VERSION_2_0 ? "2.0" : "Unspecified"));
+
+ if (tpm_emulator_check_caps(tpm_pt)) {
+ goto err_close_dev;
+ }
+
+ return 0;
+
+err_close_dev:
+ DPRINTF("%s", "Startup error, shutting down...");
+ tpm_emulator_shutdown(tpm_pt);
+ return -1;
+}
+
+static TPMBackend *tpm_emulator_create(QemuOpts *opts, const char *id)
+{
+ TPMBackend *tb = TPM_BACKEND(object_new(TYPE_TPM_EMULATOR));
+
+ tb->id = g_strdup(id);
+
+ if (tpm_emulator_handle_device_opts(TPM_EMULATOR(tb), opts)) {
+ goto err_exit;
+ }
+
+ return tb;
+
+err_exit:
+ object_unref(OBJECT(tb));
+
+ return NULL;
+}
+
+static TPMOptions *tpm_emulator_get_tpm_options(TPMBackend *tb)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
+ TPMEmulatorOptions *ops = g_new0(TPMEmulatorOptions, 1);
+
+ if (!ops) {
+ return NULL;
+ }
+ DPRINTF("%s", __func__);
+
+ ops->tpmstatedir = g_strdup(tpm_pt->ops->tpmstatedir);
+ ops->spawn = tpm_pt->ops->spawn;
+ if (tpm_pt->ops->has_path) {
+ ops->has_path = true;
+ ops->path = g_strdup(tpm_pt->ops->path);
+ }
+ if (tpm_pt->ops->has_data_path) {
+ ops->has_data_path = true;
+ ops->data_path = g_strdup(tpm_pt->ops->data_path);
+ }
+ if (tpm_pt->ops->has_ctrl_path) {
+ ops->has_ctrl_path = true;
+ ops->ctrl_path = g_strdup(tpm_pt->ops->ctrl_path);
+ }
+ if (tpm_pt->ops->has_logfile) {
+ ops->has_logfile = true;
+ ops->logfile = g_strdup(tpm_pt->ops->logfile);
+ }
+ if (tpm_pt->ops->has_loglevel) {
+ ops->has_loglevel = true;
+ ops->loglevel = tpm_pt->ops->loglevel;
+ }
+
+ return (TPMOptions *)ops;
+}
+
+static const QemuOptDesc tpm_emulator_cmdline_opts[] = {
+ TPM_STANDARD_CMDLINE_OPTS,
+ {
+ .name = "tpmstatedir",
+ .type = QEMU_OPT_STRING,
+ .help = "TPM state directroy",
+ },
+ {
+ .name = "spawn",
+ .type = QEMU_OPT_BOOL,
+ .help = "Wether to spwan given emlatory binary",
+ },
+ {
+ .name = "path",
+ .type = QEMU_OPT_STRING,
+ .help = "Path to TPM emulator binary",
+ },
+ {
+ .name = "data-path",
+ .type = QEMU_OPT_STRING,
+ .help = "Socket path to use for data exhange",
+ },
+ {
+ .name = "ctrl-path",
+ .type = QEMU_OPT_STRING,
+ .help = "Socket path to use for out-of-band control messages",
+ },
+ {
+ .name = "logfile",
+ .type = QEMU_OPT_STRING,
+ .help = "Path to log file",
+ },
+ {
+ .name = "level",
+ .type = QEMU_OPT_STRING,
+ .help = "Log level number",
+ },
+ { /* end of list */ },
+};
+
+static const TPMDriverOps tpm_emulator_driver = {
+ .type = TPM_TYPE_EMULATOR,
+ .opts = tpm_emulator_cmdline_opts,
+ .desc = tpm_emulator_desc,
+ .create = tpm_emulator_create,
+ .startup_tpm = tpm_emulator_startup_tpm,
+ .reset = tpm_emulator_reset,
+ .had_startup_error = tpm_emulator_had_startup_error,
+ .cancel_cmd = tpm_emulator_cancel_cmd,
+ .get_tpm_established_flag = tpm_emulator_get_tpm_established_flag,
+ .reset_tpm_established_flag = tpm_emulator_reset_tpm_established_flag,
+ .get_tpm_version = tpm_emulator_get_tpm_version,
+ .get_tpm_options = tpm_emulator_get_tpm_options,
+};
+
+static void tpm_emulator_inst_init(Object *obj)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(obj);
+
+ DPRINTF("%s", __func__);
+ tpm_pt->ops = g_new0(TPMEmulatorOptions, 1);
+ tpm_pt->data_ioc = tpm_pt->ctrl_ioc = NULL;
+ tpm_pt->op_executing = tpm_pt->op_canceled = false;
+ tpm_pt->child_running = false;
+ tpm_pt->cur_locty_number = ~0;
+ qemu_mutex_init(&tpm_pt->state_lock);
+}
+
+static void tpm_emulator_inst_finalize(Object *obj)
+{
+ TPMEmulator *tpm_pt = TPM_EMULATOR(obj);
+
+ tpm_emulator_cancel_cmd(TPM_BACKEND(obj));
+ tpm_emulator_shutdown(tpm_pt);
+
+ if (tpm_pt->data_ioc) {
+ qio_channel_close(tpm_pt->data_ioc, NULL);
+ }
+ if (tpm_pt->ctrl_ioc) {
+ qio_channel_close(tpm_pt->ctrl_ioc, NULL);
+ }
+ if (tpm_pt->ops) {
+ qapi_free_TPMEmulatorOptions(tpm_pt->ops);
+ }
+}
+
+static void tpm_emulator_class_init(ObjectClass *klass, void *data)
+{
+ TPMBackendClass *tbc = TPM_BACKEND_CLASS(klass);
+ tbc->ops = &tpm_emulator_driver;
+ tbc->handle_request = tpm_emulator_handle_request;
+}
+
+static const TypeInfo tpm_emulator_info = {
+ .name = TYPE_TPM_EMULATOR,
+ .parent = TYPE_TPM_BACKEND,
+ .instance_size = sizeof(TPMEmulator),
+ .class_init = tpm_emulator_class_init,
+ .instance_init = tpm_emulator_inst_init,
+ .instance_finalize = tpm_emulator_inst_finalize,
+};
+
+static void tpm_emulator_register(void)
+{
+ type_register_static(&tpm_emulator_info);
+ tpm_register_driver(&tpm_emulator_driver);
+}
+
+type_init(tpm_emulator_register)
diff --git a/hw/tpm/tpm_ioctl.h b/hw/tpm/tpm_ioctl.h
new file mode 100644
index 0000000..af49708
--- /dev/null
+++ b/hw/tpm/tpm_ioctl.h
@@ -0,0 +1,243 @@
+/*
+ * tpm_ioctl.h
+ *
+ * (c) Copyright IBM Corporation 2014, 2015.
+ *
+ * This file is licensed under the terms of the 3-clause BSD license
+ */
+#ifndef _TPM_IOCTL_H_
+#define _TPM_IOCTL_H_
+
+#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 ptm_res as the first element.
+ * ptm_res corresponds to the error code of a command executed by the TPM.
+ */
+
+typedef uint32_t ptm_res;
+
+/* PTM_GET_TPMESTABLISHED: get the establishment bit */
+struct ptm_est {
+ union {
+ struct {
+ ptm_res tpm_result;
+ unsigned char bit; /* TPM established bit */
+ } resp; /* response */
+ } u;
+};
+
+/* PTM_RESET_TPMESTABLISHED: reset establishment bit */
+struct ptm_reset_est {
+ union {
+ struct {
+ uint8_t loc; /* locality to use */
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ } resp; /* response */
+ } u;
+};
+
+/* PTM_INIT */
+struct ptm_init {
+ union {
+ struct {
+ uint32_t init_flags; /* see definitions below */
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ } resp; /* response */
+ } u;
+};
+
+/* above init_flags */
+#define PTM_INIT_FLAG_DELETE_VOLATILE (1 << 0)
+ /* delete volatile state file after reading it */
+
+/* PTM_SET_LOCALITY */
+struct ptm_loc {
+ union {
+ struct {
+ uint8_t loc; /* locality to set */
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ } resp; /* response */
+ } u;
+};
+
+/* PTM_HASH_DATA: hash given data */
+struct ptm_hdata {
+ union {
+ struct {
+ uint32_t length;
+ uint8_t data[4096];
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ } resp; /* response */
+ } 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 PTM_STATE_BLOB_SIZE (3 * 1024)
+
+/*
+ * The following is the data structure to get state blobs from the TPM.
+ * If the size of the state blob exceeds the PTM_STATE_BLOB_SIZE, multiple reads
+ * with this ioctl and with adjusted offset are necessary. All bytes
+ * must be transferred and the transfer is done once the last byte has been
+ * returned.
+ * It is possible to use the read() interface for reading the data; however, the
+ * first bytes of the state blob will be part of the response to the ioctl(); a
+ * subsequent read() is only necessary if the total length (totlength) exceeds
+ * the number of received bytes. seek() is not supported.
+ */
+struct ptm_getstate {
+ union {
+ struct {
+ uint32_t state_flags; /* may be: PTM_STATE_FLAG_DECRYPTED */
+ uint32_t type; /* which blob to pull */
+ uint32_t offset; /* offset from where to read */
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ uint32_t state_flags; /* may be: PTM_STATE_FLAG_ENCRYPTED */
+ uint32_t totlength; /* total length that will be transferred */
+ uint32_t length; /* number of bytes in following buffer */
+ uint8_t data[PTM_STATE_BLOB_SIZE];
+ } resp; /* response */
+ } 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 PTM_STATE_FLAG_DECRYPTED 1 /* on input: get decrypted state */
+#define PTM_STATE_FLAG_ENCRYPTED 2 /* on output: state is encrypted */
+
+/*
+ * The following is the data structure to set state blobs in the TPM.
+ * If the size of the state blob exceeds the PTM_STATE_BLOB_SIZE, multiple
+ * 'writes' using this ioctl are necessary. The last packet is indicated
+ * by the length being smaller than the PTM_STATE_BLOB_SIZE.
+ * The very first packet may have a length indicator of '0' enabling
+ * a write() with all the bytes from a buffer. If the write() interface
+ * is used, a final ioctl with a non-full buffer must be made to indicate
+ * that all data were transferred (a write with 0 bytes would not work).
+ */
+struct ptm_setstate {
+ union {
+ struct {
+ uint32_t state_flags; /* may be PTM_STATE_FLAG_ENCRYPTED */
+ uint32_t type; /* which blob to set */
+ uint32_t length; /* length of the data;
+ use 0 on the first packet to
+ transfer using write() */
+ uint8_t data[PTM_STATE_BLOB_SIZE];
+ } req; /* request */
+ struct {
+ ptm_res tpm_result;
+ } resp; /* response */
+ } u;
+};
+
+/*
+ * PTM_GET_CONFIG: Data structure to get runtime configuration information
+ * such as which keys are applied.
+ */
+struct ptm_getconfig {
+ union {
+ struct {
+ ptm_res tpm_result;
+ uint32_t flags;
+ } resp; /* response */
+ } u;
+};
+
+#define PTM_CONFIG_FLAG_FILE_KEY 0x1
+#define PTM_CONFIG_FLAG_MIGRATION_KEY 0x2
+
+
+typedef uint64_t ptm_cap;
+typedef struct ptm_est ptm_est;
+typedef struct ptm_reset_est ptm_reset_est;
+typedef struct ptm_loc ptm_loc;
+typedef struct ptm_hdata ptm_hdata;
+typedef struct ptm_init ptm_init;
+typedef struct ptm_getstate ptm_getstate;
+typedef struct ptm_setstate ptm_setstate;
+typedef struct ptm_getconfig ptm_getconfig;
+
+/* 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)
+#define PTM_CAP_GET_CONFIG (1 << 11)
+
+enum {
+ PTM_GET_CAPABILITY = _IOR('P', 0, ptm_cap),
+ PTM_INIT = _IOWR('P', 1, ptm_init),
+ PTM_SHUTDOWN = _IOR('P', 2, ptm_res),
+ PTM_GET_TPMESTABLISHED = _IOR('P', 3, ptm_est),
+ PTM_SET_LOCALITY = _IOWR('P', 4, ptm_loc),
+ PTM_HASH_START = _IOR('P', 5, ptm_res),
+ PTM_HASH_DATA = _IOWR('P', 6, ptm_hdata),
+ PTM_HASH_END = _IOR('P', 7, ptm_res),
+ PTM_CANCEL_TPM_CMD = _IOR('P', 8, ptm_res),
+ PTM_STORE_VOLATILE = _IOR('P', 9, ptm_res),
+ PTM_RESET_TPMESTABLISHED = _IOWR('P', 10, ptm_reset_est),
+ PTM_GET_STATEBLOB = _IOWR('P', 11, ptm_getstate),
+ PTM_SET_STATEBLOB = _IOWR('P', 12, ptm_setstate),
+ PTM_STOP = _IOR('P', 13, ptm_res),
+ PTM_GET_CONFIG = _IOR('P', 14, ptm_getconfig),
+};
+
+/*
+ * Commands used by the non-CUSE TPMs
+ *
+ * All messages container big-endian data.
+ *
+ * The return messages only contain the 'resp' part of the unions
+ * in the data structures above. Besides that the limits in the
+ * buffers above (ptm_hdata:u.req.data and ptm_get_state:u.resp.data
+ * and ptm_set_state:u.req.data) are 0xffffffff.
+ */
+enum {
+ CMD_GET_CAPABILITY = 1,
+ CMD_INIT,
+ CMD_SHUTDOWN,
+ CMD_GET_TPMESTABLISHED,
+ CMD_SET_LOCALITY,
+ CMD_HASH_START,
+ CMD_HASH_DATA,
+ CMD_HASH_END,
+ CMD_CANCEL_TPM_CMD,
+ CMD_STORE_VOLATILE,
+ CMD_RESET_TPMESTABLISHED,
+ CMD_GET_STATEBLOB,
+ CMD_SET_STATEBLOB,
+ CMD_STOP,
+ CMD_GET_CONFIG,
+};
+
+#endif /* _TPM_IOCTL_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 3f1ca20..116d1e6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5117,10 +5117,12 @@
# An enumeration of TPM types
#
# @passthrough: TPM passthrough type
+# @emulator: Software Emulator TPM type
+# Since: 2.10
#
# Since: 1.5
##
-{ 'enum': 'TpmType', 'data': [ 'passthrough' ] }
+{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator' ] }
##
# @query-tpm-types:
@@ -5134,7 +5136,7 @@
# Example:
#
# -> { "execute": "query-tpm-types" }
-# <- { "return": [ "passthrough" ] }
+# <- { "return": [ "passthrough", "emulator" ] }
#
##
{ 'command': 'query-tpm-types', 'returns': ['TpmType'] }
@@ -5164,6 +5166,36 @@
{ 'struct': 'TPMPassthroughOptions', 'base': 'TPMOptions',
'data': { '*path' : 'str', '*cancel-path' : 'str'} }
+##
+# @TPMEmulatorOptions:
+#
+# Information about the TPM emulator
+#
+# @tpmstatedir: TPM emulator state directory
+# @spawn: true if, qemu has to spawn a new emulator process with given @path,
+# otherwise it connects to already rinning emulator with given @data-path
+# and @ctrl-path sockets. (default: 'false')
+# @path: TPM emulator binary path to spawn.(default: 'swtpm')
+# @data-path: path of the unix socket to use for exchanging data messages, if
+# not provided socket pairs are used when @sapwn is true.
+# @ctrl-path: path of the unix socket file to use for exchagning out-of-band
+# control messages, if not provided socket pairs are used when
+# @spawn is true.
+# @logfile: file to use to place TPM emulator logs, if not provided logging is
+# disabled.
+# @loglevel: optional log level number, loglevel is ignored if no logfile
+# provided. (default: 5)
+#
+# Since: 2.10
+##
+{ 'struct': 'TPMEmulatorOptions', 'base': 'TPMOptions',
+ 'data': { 'tpmstatedir' : 'str',
+ 'spawn': 'bool',
+ '*path': 'str',
+ '*data-path': 'str',
+ '*ctrl-path': 'str',
+ '*logfile': 'str',
+ '*loglevel': 'int' } }
##
# @TPMInfo:
diff --git a/qemu-options.hx b/qemu-options.hx
index 99af8ed..aae0de0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2846,7 +2846,15 @@ 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 emulator,id=id,spawn=on|off,tpmstatedir=dir[,path=emulator-path,data-path=path,ctrl-path=path,logfile=path,loglevel=level]\n"
+ " spawn=on|off controls spawning support\n"
+ " use tpmstatedir to provide path to the tpm state dirctory\n"
+ " use path to provide the emulator binary to launch; default is 'swtpm'\n"
+ " use data-path to provide the socket path for exchanging data messages\n"
+ " use ctrl-path to provide the socket path for sending control messages to software emulator\n"
+ " use logfile to provide where to place the swtpm logs\n"
+ " use loglevel to controls the swtpm log level\n",
QEMU_ARCH_ALL)
STEXI
@@ -2855,8 +2863,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{emulator}.
The specific backend type will determine the applicable options.
The @code{-tpmdev} option creates the TPM backend and requires a
@@ -2906,6 +2914,45 @@ 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 emulator, id=@var{id}, tpmstatedir=@var{path}, spawn=@var{on|off}, path=@var{emulator-binary-path}, data-path=@var{path}, ctrl-path=@var{path}, logfile=@var{path}, loglevel=@var{level}
+
+(Linux-host only) Enable access to a TPM emulator using unix domain sockets.
+
+@option{tpmstatedir} specifies the tpm state directory
+
+@option{spawn} specifies if qemu should spawn new emulator process with given @option{path}
+
+@option{path} specifies the emulator binary path to use for spawning
+
+@option{data-path} optional socket path to use for exchanging TPM data with emulator
+
+@option{ctrl-path} optional socket path to use for sending control data to emulator
+
+@option{logfile} optional log file to use to place log messages
+
+@option{loglevel} specifies the log level to use
+
+To create TPM emulator backend device that spawns new swtpm binary and communicate with socket pairs:
+@example
+
+-tpmdev emulator,id=tpm0,tpmstatedir=/tmp/my-tpm,spawn=on,path=/usr/local/bin/swtpm,logfile=/tmp/qemu-tpm.log,loglevel=5 -device tpm-tis,tpmdev=tpm0
+
+@end example
+
+To create TPM emulator backend device that spawns new swtpm binary and communicate using unix file system sockets:
+@example
+
+-tpmdev emulator,id=tpm0,tpmstatedir=/tmp/my-tpm,spawn=on,path=/usr/local/bin/swtpm,data-path=/tmp/swtpm-data.socket,ctrl-path=/tmp/swtpm-ctrl.socket,logfile=/tmp/qemu-tpm.log,loglevel=5 -device tpm-tis,tpmdev=tpm0
+
+@end example
+
+To create a TOM emulator backend device that connects to already running swtpm binary using file system sockets:
+@example
+
+-tpmdev emulator,id=tpm0,tpmstatedir=/tmp/my-tpm,spawn=off,data-path=/tmp/swtpm-data.socket,ctrl-path=/tmp/swtpm-ctrl.socket,logfile=/tmp/qemu-tpm.log,loglevel=5 -device tpm-tis,tpmdev=tpm0
+
+@end example
+
@end table
ETEXI
diff --git a/tpm.c b/tpm.c
index 43d980e..ce07c40 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,
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator Amarnath Valluri
@ 2017-04-07 14:41 ` Daniel P. Berrange
2017-04-07 15:11 ` Marc-André Lureau
2017-04-10 7:08 ` Amarnath Valluri
2017-04-25 19:35 ` Stefan Berger
1 sibling, 2 replies; 34+ messages in thread
From: Daniel P. Berrange @ 2017-04-07 14:41 UTC (permalink / raw)
To: Amarnath Valluri; +Cc: qemu-devel, patrick.ohly, stefanb, marcandre.lureau
On Fri, Apr 07, 2017 at 05:30:31PM +0300, Amarnath Valluri wrote:
> This change introduces a new TPM backend driver that can communicate with
> swtpm(software TPM emulator) using unix domain socket interface.
>
> Swtpm uses two unix sockets, one for plain TPM commands and responses, and one
> for out-of-band control messages.
>
> The swtpm and associated tools can be found here:
> https://github.com/stefanberger/swtpm
>
> Usage:
> # setup TPM state directory
> mkdir /tmp/mytpm
> chown -R tss:root /tmp/mytpm
> /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
>
> # Ask qemu to use TPM emulator with given tpm state directory
> qemu-system-x86_64 \
> [...] \
> -tpmdev emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
> -device tpm-tis,tpmdev=tpm0 \
> [...]
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> configure | 15 +-
> hmp.c | 21 ++
> hw/tpm/Makefile.objs | 1 +
> hw/tpm/tpm_emulator.c | 927 ++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/tpm/tpm_ioctl.h | 243 +++++++++++++
> qapi-schema.json | 36 +-
> qemu-options.hx | 53 ++-
> tpm.c | 2 +-
> 8 files changed, 1289 insertions(+), 9 deletions(-)
> create mode 100644 hw/tpm/tpm_emulator.c
> create mode 100644 hw/tpm/tpm_ioctl.h
> +static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
> +{
> + int fds[2] = { -1, -1 };
> + int ctrl_fds[2] = { -1, -1 };
> + pid_t cpid;
> +
> + if (!tpm_pt->ops->has_data_path) {
> + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
> + return -1;
> + }
> + }
> +
> + if (!tpm_pt->ops->has_ctrl_path) {
> + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
> + if (!tpm_pt->ops->has_data_path) {
> + closesocket(fds[0]);
> + closesocket(fds[1]);
> + }
> + return -1;
> + }
> + }
> +
> + cpid = qemu_fork(NULL);
> + if (cpid < 0) {
> + error_report("tpm-emulator: Fork failure: %s", strerror(errno));
> + if (!tpm_pt->ops->has_data_path) {
> + closesocket(fds[0]);
> + closesocket(fds[1]);
> + }
> + if (!tpm_pt->ops->has_ctrl_path) {
> + closesocket(ctrl_fds[0]);
> + closesocket(ctrl_fds[1]);
> + }
> + return -1;
> + }
> +
> + if (cpid == 0) { /* CHILD */
> + enum {
> + PARAM_PATH,
> + PARAM_IFACE,
> + PARAM_SERVER, PARAM_SERVER_ARGS,
> + PARAM_CTRL, PARAM_CTRL_ARGS,
> + PARAM_STATE, PARAM_STATE_ARGS,
> + PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
> + PARAM_LOG, PARAM_LOG_ARGS,
> + PARAM_MAX
> + };
> +
> + int i;
> + int data_fd = -1, ctrl_fd = -1;
> + char *argv[PARAM_MAX+1];
> +
> + /* close all unused inherited sockets */
> + if (fds[0] >= 0)
> + closesocket(fds[0]);
> + if (ctrl_fds[0] >= 0)
> + closesocket(ctrl_fds[0]);
The 'if' checks are pointless - its already guaranteed by the
fact you check socketpair() status.
> + i = STDERR_FILENO + 1;
> + if (fds[1] >= 0) {
> + data_fd = dup2(fds[1], i++);
> + if (data_fd < 0) {
> + error_report("tpm-emulator: dup2() failure - %s",
> + strerror(errno));
> + goto exit_child;
> + }
> + }
> + if (ctrl_fds[1] >= 0) {
> + ctrl_fd = dup2(ctrl_fds[1], i++);
> + if (ctrl_fd < 0) {
> + error_report("tpm-emulator: dup2() failure - %s",
> + strerror(errno));
> + goto exit_child;
> + }
> + }
> + for ( ; i < _SC_OPEN_MAX; i++) {
Errr, _SC_OPEN_MAX is not the maximum number of FDs - it is parameter to
use with sysconf() to query the number of files - you must call sysconf().
> + closesocket(i);
close, not closesocket - you can't assume these are all sockets.
> + DPRINT("\n")
> + if (execv(tpm_pt->ops->path, (char * const *)argv) < 0) {
> + error_report("execv() failure : %s", strerror(errno));
> + }
> +
> +exit_child:
> + g_strfreev(argv);
> + if (data_fd >= 0)
> + closesocket(data_fd);
> + if (ctrl_fd >= 0)
> + closesocket(ctrl_fd);
> +
> + exit(0);
You need _exit(), not exit() as you don't want to run atexit() handlers
here. You also want '1' not '0' since this is a failure scenario.
> + } else { /* self */
> + struct stat st;
> + DPRINTF("child pid: %d", cpid);
> + int rc;
> + int timeout = 3; /* wait for max 3 seconds */
> +
> + /* close unsed sockets */
> + if (fds[1] >= 0)
> + closesocket(fds[1]);
> + if (ctrl_fds[1] >= 0)
> + closesocket(ctrl_fds[1]);
> +
> + while((rc = stat(TPM_EMULATOR_PIDFILE, &st)) < 0 && timeout--) {
> + sleep(1);
> + }
A fixed 3 second timeout will inevitably cause failures on systems with
high load.
Presumably you're trying to handle the scenario where the child process
exits without creating the pid file ?
In which case you can use 'kill(cpid, 0)' and if errno == ESRCH
then the child has exited.
> +
> + if (timeout == -1) {
> + error_report("tpm-emulator: failed to find pid file: %s",
> + strerror(errno));
> + goto err_kill_child;
> + }
> +
> + tpm_pt->data_ioc = _iochannel_new(tpm_pt->ops->data_path, fds[0], NULL);
> + if (!tpm_pt->data_ioc) {
> + error_report("tpm-emulator: Unable to connect socket : %s",
> + tpm_pt->ops->data_path);
> + goto err_kill_child;
> + }
> +
> + tpm_pt->ctrl_ioc = _iochannel_new(tpm_pt->ops->ctrl_path, ctrl_fds[0], NULL);
> + if (!tpm_pt->ctrl_ioc) {
> + error_report("tpm-emulator: Unable to connect socket : %s",
> + tpm_pt->ops->ctrl_path);
> + goto err_kill_child;
> + }
> +
> + tpm_pt->child_running = true;
> +
> + qemu_add_child_watch(cpid);
> +
> + qio_channel_add_watch(tpm_pt->data_ioc, G_IO_HUP|G_IO_ERR,
> + tpm_emulator_fd_handler, tpm_pt, NULL);
> + }
> +
> + return 0;
> +
> +err_kill_child:
> + kill(cpid, SIGTERM);
> + closesocket(fds[0]);
> + closesocket(ctrl_fds[0]);
You can't assume the child has gone after a SIGTERM. You need to
check, and be prepared to SIGKILL after time reasonable time,
if needed.
> + tpm_pt->child_running = false;
> +
> + return -1;
> +}
> +
> +static const QemuOptDesc tpm_emulator_cmdline_opts[] = {
> + TPM_STANDARD_CMDLINE_OPTS,
> + {
> + .name = "tpmstatedir",
> + .type = QEMU_OPT_STRING,
> + .help = "TPM state directroy",
> + },
> + {
> + .name = "spawn",
> + .type = QEMU_OPT_BOOL,
> + .help = "Wether to spwan given emlatory binary",
> + },
> + {
> + .name = "path",
> + .type = QEMU_OPT_STRING,
> + .help = "Path to TPM emulator binary",
> + },
> + {
> + .name = "data-path",
> + .type = QEMU_OPT_STRING,
> + .help = "Socket path to use for data exhange",
> + },
> + {
> + .name = "ctrl-path",
> + .type = QEMU_OPT_STRING,
> + .help = "Socket path to use for out-of-band control messages",
> + },
I'm still not convinced by the need for 2 separate UNIX sockets, unless
there's a performance reason, but that looks unlikely.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-07 14:41 ` Daniel P. Berrange
@ 2017-04-07 15:11 ` Marc-André Lureau
2017-04-10 7:34 ` Amarnath Valluri
2017-04-10 7:08 ` Amarnath Valluri
1 sibling, 1 reply; 34+ messages in thread
From: Marc-André Lureau @ 2017-04-07 15:11 UTC (permalink / raw)
To: Daniel P. Berrange, Amarnath Valluri; +Cc: qemu-devel, patrick.ohly, stefanb
Hi
On Fri, Apr 7, 2017 at 4:41 PM Daniel P. Berrange
> > + .name = "data-path",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Socket path to use for data exhange",
> > + },
> > + {
> > + .name = "ctrl-path",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Socket path to use for out-of-band control messages",
> > + },
>
> I'm still not convinced by the need for 2 separate UNIX sockets, unless
> there's a performance reason, but that looks unlikely.
>
>
We would also need something more than an implementation of the protocol
that qemu is going to rely on as an external dependency. A protocol
specification would help to start the discussion. Alternatively, I would
suggest to not rely on a public protocol, and implement the helper as part
of qemu with a private protocol (but still documented).
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-07 14:41 ` Daniel P. Berrange
2017-04-07 15:11 ` Marc-André Lureau
@ 2017-04-10 7:08 ` Amarnath Valluri
2017-04-10 8:31 ` Daniel P. Berrange
2017-04-10 16:15 ` Stefan Berger
1 sibling, 2 replies; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-10 7:08 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel, patrick.ohly, stefanb, marcandre.lureau
On 07.04.2017 17:41, Daniel P. Berrange wrote:
> On Fri, Apr 07, 2017 at 05:30:31PM +0300, Amarnath Valluri wrote:
>> This change introduces a new TPM backend driver that can communicate with
>> swtpm(software TPM emulator) using unix domain socket interface.
>>
>> Swtpm uses two unix sockets, one for plain TPM commands and responses, and one
>> for out-of-band control messages.
>>
>> The swtpm and associated tools can be found here:
>> https://github.com/stefanberger/swtpm
>>
>> Usage:
>> # setup TPM state directory
>> mkdir /tmp/mytpm
>> chown -R tss:root /tmp/mytpm
>> /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
>>
>> # Ask qemu to use TPM emulator with given tpm state directory
>> qemu-system-x86_64 \
>> [...] \
>> -tpmdev emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
>> -device tpm-tis,tpmdev=tpm0 \
>> [...]
>>
>> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
>> ---
>> configure | 15 +-
>> hmp.c | 21 ++
>> hw/tpm/Makefile.objs | 1 +
>> hw/tpm/tpm_emulator.c | 927 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/tpm/tpm_ioctl.h | 243 +++++++++++++
>> qapi-schema.json | 36 +-
>> qemu-options.hx | 53 ++-
>> tpm.c | 2 +-
>> 8 files changed, 1289 insertions(+), 9 deletions(-)
>> create mode 100644 hw/tpm/tpm_emulator.c
>> create mode 100644 hw/tpm/tpm_ioctl.h
>> +static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
>> +{
>> + int fds[2] = { -1, -1 };
>> + int ctrl_fds[2] = { -1, -1 };
>> + pid_t cpid;
>> +
>> + if (!tpm_pt->ops->has_data_path) {
>> + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
>> + return -1;
>> + }
>> + }
>> +
>> + if (!tpm_pt->ops->has_ctrl_path) {
>> + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
>> + if (!tpm_pt->ops->has_data_path) {
>> + closesocket(fds[0]);
>> + closesocket(fds[1]);
>> + }
>> + return -1;
>> + }
>> + }
>> +
>> + cpid = qemu_fork(NULL);
>> + if (cpid < 0) {
>> + error_report("tpm-emulator: Fork failure: %s", strerror(errno));
>> + if (!tpm_pt->ops->has_data_path) {
>> + closesocket(fds[0]);
>> + closesocket(fds[1]);
>> + }
>> + if (!tpm_pt->ops->has_ctrl_path) {
>> + closesocket(ctrl_fds[0]);
>> + closesocket(ctrl_fds[1]);
>> + }
>> + return -1;
>> + }
>> +
>> + if (cpid == 0) { /* CHILD */
>> + enum {
>> + PARAM_PATH,
>> + PARAM_IFACE,
>> + PARAM_SERVER, PARAM_SERVER_ARGS,
>> + PARAM_CTRL, PARAM_CTRL_ARGS,
>> + PARAM_STATE, PARAM_STATE_ARGS,
>> + PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
>> + PARAM_LOG, PARAM_LOG_ARGS,
>> + PARAM_MAX
>> + };
>> +
>> + int i;
>> + int data_fd = -1, ctrl_fd = -1;
>> + char *argv[PARAM_MAX+1];
>> +
>> + /* close all unused inherited sockets */
>> + if (fds[0] >= 0)
>> + closesocket(fds[0]);
>> + if (ctrl_fds[0] >= 0)
>> + closesocket(ctrl_fds[0]);
> The 'if' checks are pointless - its already guaranteed by the
> fact you check socketpair() status.
socketpairs might not be created in case of data-path & ctrl-path
provided, so i feel these checks are needed.
>
>> + i = STDERR_FILENO + 1;
>> + if (fds[1] >= 0) {
>> + data_fd = dup2(fds[1], i++);
>> + if (data_fd < 0) {
>> + error_report("tpm-emulator: dup2() failure - %s",
>> + strerror(errno));
>> + goto exit_child;
>> + }
>> + }
>> + if (ctrl_fds[1] >= 0) {
>> + ctrl_fd = dup2(ctrl_fds[1], i++);
>> + if (ctrl_fd < 0) {
>> + error_report("tpm-emulator: dup2() failure - %s",
>> + strerror(errno));
>> + goto exit_child;
>> + }
>> + }
>> + for ( ; i < _SC_OPEN_MAX; i++) {
> Errr, _SC_OPEN_MAX is not the maximum number of FDs - it is parameter to
> use with sysconf() to query the number of files - you must call sysconf().
Ya, thanks for educating me, i will change this.
>
>> + closesocket(i);
> close, not closesocket - you can't assume these are all sockets.
Does this change makes any difference, as per
include/sysemu/os-posix.h, closesocket() is define as close(), and this
backend is targeted only for "Linux" targets. Please let me know if i am
missing something.
>
>
>> + DPRINT("\n")
>> + if (execv(tpm_pt->ops->path, (char * const *)argv) < 0) {
>> + error_report("execv() failure : %s", strerror(errno));
>> + }
>> +
>> +exit_child:
>> + g_strfreev(argv);
>> + if (data_fd >= 0)
>> + closesocket(data_fd);
>> + if (ctrl_fd >= 0)
>> + closesocket(ctrl_fd);
>> +
>> + exit(0);
> You need _exit(), not exit() as you don't want to run atexit() handlers
> here. You also want '1' not '0' since this is a failure scenario.
Sure, i will change this.
>
>> + } else { /* self */
>> + struct stat st;
>> + DPRINTF("child pid: %d", cpid);
>> + int rc;
>> + int timeout = 3; /* wait for max 3 seconds */
>> +
>> + /* close unsed sockets */
>> + if (fds[1] >= 0)
>> + closesocket(fds[1]);
>> + if (ctrl_fds[1] >= 0)
>> + closesocket(ctrl_fds[1]);
>> +
>> + while((rc = stat(TPM_EMULATOR_PIDFILE, &st)) < 0 && timeout--) {
>> + sleep(1);
>> + }
> A fixed 3 second timeout will inevitably cause failures on systems with
> high load.
>
> Presumably you're trying to handle the scenario where the child process
> exits without creating the pid file ?
The check is mainly to know if the child setup is done and ready to
accept requests.
>
> In which case you can use 'kill(cpid, 0)' and if errno == ESRCH
> then the child has exited.
>
>> +
>> + if (timeout == -1) {
>> + error_report("tpm-emulator: failed to find pid file: %s",
>> + strerror(errno));
>> + goto err_kill_child;
>> + }
>> +
>> + tpm_pt->data_ioc = _iochannel_new(tpm_pt->ops->data_path, fds[0], NULL);
>> + if (!tpm_pt->data_ioc) {
>> + error_report("tpm-emulator: Unable to connect socket : %s",
>> + tpm_pt->ops->data_path);
>> + goto err_kill_child;
>> + }
>> +
>> + tpm_pt->ctrl_ioc = _iochannel_new(tpm_pt->ops->ctrl_path, ctrl_fds[0], NULL);
>> + if (!tpm_pt->ctrl_ioc) {
>> + error_report("tpm-emulator: Unable to connect socket : %s",
>> + tpm_pt->ops->ctrl_path);
>> + goto err_kill_child;
>> + }
>> +
>> + tpm_pt->child_running = true;
>> +
>> + qemu_add_child_watch(cpid);
>> +
>> + qio_channel_add_watch(tpm_pt->data_ioc, G_IO_HUP|G_IO_ERR,
>> + tpm_emulator_fd_handler, tpm_pt, NULL);
>> + }
>> +
>> + return 0;
>> +
>> +err_kill_child:
>> + kill(cpid, SIGTERM);
>> + closesocket(fds[0]);
>> + closesocket(ctrl_fds[0]);
> You can't assume the child has gone after a SIGTERM. You need to
> check, and be prepared to SIGKILL after time reasonable time,
> if needed.
Ok, i will add this check.
>
>> + tpm_pt->child_running = false;
>> +
>> + return -1;
>> +}
>> +
>> +static const QemuOptDesc tpm_emulator_cmdline_opts[] = {
>> + TPM_STANDARD_CMDLINE_OPTS,
>> + {
>> + .name = "tpmstatedir",
>> + .type = QEMU_OPT_STRING,
>> + .help = "TPM state directroy",
>> + },
>> + {
>> + .name = "spawn",
>> + .type = QEMU_OPT_BOOL,
>> + .help = "Wether to spwan given emlatory binary",
>> + },
>> + {
>> + .name = "path",
>> + .type = QEMU_OPT_STRING,
>> + .help = "Path to TPM emulator binary",
>> + },
>> + {
>> + .name = "data-path",
>> + .type = QEMU_OPT_STRING,
>> + .help = "Socket path to use for data exhange",
>> + },
>> + {
>> + .name = "ctrl-path",
>> + .type = QEMU_OPT_STRING,
>> + .help = "Socket path to use for out-of-band control messages",
>> + },
> I'm still not convinced by the need for 2 separate UNIX sockets, unless
> there's a performance reason, but that looks unlikely.
I myself, is not expert of swtpm's interface, hence i cannot defend this.
i am just trying to enable emulation support in qemu using swtpm. Stefan
Berger is the right person to comment on this.
- Amarnath
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-07 15:11 ` Marc-André Lureau
@ 2017-04-10 7:34 ` Amarnath Valluri
2017-04-10 9:54 ` Marc-André Lureau
0 siblings, 1 reply; 34+ messages in thread
From: Amarnath Valluri @ 2017-04-10 7:34 UTC (permalink / raw)
To: Marc-André Lureau, Daniel P. Berrange
Cc: qemu-devel, patrick.ohly, stefanb
On 07.04.2017 18:11, Marc-André Lureau wrote:
> Hi
>
> On Fri, Apr 7, 2017 at 4:41 PM Daniel P. Berrange
>
> > + .name = "data-path",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Socket path to use for data exhange",
> > + },
> > + {
> > + .name = "ctrl-path",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Socket path to use for out-of-band control
> messages",
> > + },
>
> I'm still not convinced by the need for 2 separate UNIX sockets,
> unless
> there's a performance reason, but that looks unlikely.
>
>
> We would also need something more than an implementation of the
> protocol that qemu is going to rely on as an external dependency. A
> protocol specification would help to start the discussion.
I would agree with you, Can we adopt the current swtpm's control
interface as initial proposal.
> Alternatively, I would suggest to not rely on a public protocol,
What do you mean by public protocol here, can you please help me
understanding this.
- Amarnath
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-10 7:08 ` Amarnath Valluri
@ 2017-04-10 8:31 ` Daniel P. Berrange
2017-04-10 16:15 ` Stefan Berger
1 sibling, 0 replies; 34+ messages in thread
From: Daniel P. Berrange @ 2017-04-10 8:31 UTC (permalink / raw)
To: Amarnath Valluri; +Cc: qemu-devel, patrick.ohly, stefanb, marcandre.lureau
On Mon, Apr 10, 2017 at 10:08:21AM +0300, Amarnath Valluri wrote:
>
>
> On 07.04.2017 17:41, Daniel P. Berrange wrote:
> > On Fri, Apr 07, 2017 at 05:30:31PM +0300, Amarnath Valluri wrote:
> > > This change introduces a new TPM backend driver that can communicate with
> > > swtpm(software TPM emulator) using unix domain socket interface.
> > >
> > > Swtpm uses two unix sockets, one for plain TPM commands and responses, and one
> > > for out-of-band control messages.
> > >
> > > The swtpm and associated tools can be found here:
> > > https://github.com/stefanberger/swtpm
> > >
> > > Usage:
> > > # setup TPM state directory
> > > mkdir /tmp/mytpm
> > > chown -R tss:root /tmp/mytpm
> > > /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
> > >
> > > # Ask qemu to use TPM emulator with given tpm state directory
> > > qemu-system-x86_64 \
> > > [...] \
> > > -tpmdev emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
> > > -device tpm-tis,tpmdev=tpm0 \
> > > [...]
> > >
> > > Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> > > ---
> > > configure | 15 +-
> > > hmp.c | 21 ++
> > > hw/tpm/Makefile.objs | 1 +
> > > hw/tpm/tpm_emulator.c | 927 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > hw/tpm/tpm_ioctl.h | 243 +++++++++++++
> > > qapi-schema.json | 36 +-
> > > qemu-options.hx | 53 ++-
> > > tpm.c | 2 +-
> > > 8 files changed, 1289 insertions(+), 9 deletions(-)
> > > create mode 100644 hw/tpm/tpm_emulator.c
> > > create mode 100644 hw/tpm/tpm_ioctl.h
> > > +static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
> > > +{
> > > + int fds[2] = { -1, -1 };
> > > + int ctrl_fds[2] = { -1, -1 };
> > > + pid_t cpid;
> > > +
> > > + if (!tpm_pt->ops->has_data_path) {
> > > + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
> > > + return -1;
> > > + }
> > > + }
> > > +
> > > + if (!tpm_pt->ops->has_ctrl_path) {
> > > + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
> > > + if (!tpm_pt->ops->has_data_path) {
> > > + closesocket(fds[0]);
> > > + closesocket(fds[1]);
> > > + }
> > > + return -1;
> > > + }
> > > + }
> > > +
> > > + cpid = qemu_fork(NULL);
> > > + if (cpid < 0) {
> > > + error_report("tpm-emulator: Fork failure: %s", strerror(errno));
> > > + if (!tpm_pt->ops->has_data_path) {
> > > + closesocket(fds[0]);
> > > + closesocket(fds[1]);
> > > + }
> > > + if (!tpm_pt->ops->has_ctrl_path) {
> > > + closesocket(ctrl_fds[0]);
> > > + closesocket(ctrl_fds[1]);
> > > + }
> > > + return -1;
> > > + }
> > > +
> > > + if (cpid == 0) { /* CHILD */
> > > + enum {
> > > + PARAM_PATH,
> > > + PARAM_IFACE,
> > > + PARAM_SERVER, PARAM_SERVER_ARGS,
> > > + PARAM_CTRL, PARAM_CTRL_ARGS,
> > > + PARAM_STATE, PARAM_STATE_ARGS,
> > > + PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
> > > + PARAM_LOG, PARAM_LOG_ARGS,
> > > + PARAM_MAX
> > > + };
> > > +
> > > + int i;
> > > + int data_fd = -1, ctrl_fd = -1;
> > > + char *argv[PARAM_MAX+1];
> > > +
> > > + /* close all unused inherited sockets */
> > > + if (fds[0] >= 0)
> > > + closesocket(fds[0]);
> > > + if (ctrl_fds[0] >= 0)
> > > + closesocket(ctrl_fds[0]);
> > The 'if' checks are pointless - its already guaranteed by the
> > fact you check socketpair() status.
> socketpairs might not be created in case of data-path & ctrl-path provided,
> so i feel these checks are needed.
> >
> > > + i = STDERR_FILENO + 1;
> > > + if (fds[1] >= 0) {
> > > + data_fd = dup2(fds[1], i++);
> > > + if (data_fd < 0) {
> > > + error_report("tpm-emulator: dup2() failure - %s",
> > > + strerror(errno));
> > > + goto exit_child;
> > > + }
> > > + }
> > > + if (ctrl_fds[1] >= 0) {
> > > + ctrl_fd = dup2(ctrl_fds[1], i++);
> > > + if (ctrl_fd < 0) {
> > > + error_report("tpm-emulator: dup2() failure - %s",
> > > + strerror(errno));
> > > + goto exit_child;
> > > + }
> > > + }
> > > + for ( ; i < _SC_OPEN_MAX; i++) {
> > Errr, _SC_OPEN_MAX is not the maximum number of FDs - it is parameter to
> > use with sysconf() to query the number of files - you must call sysconf().
> Ya, thanks for educating me, i will change this.
> >
> > > + closesocket(i);
> > close, not closesocket - you can't assume these are all sockets.
> Does this change makes any difference, as per include/sysemu/os-posix.h,
> closesocket() is define as close(), and this backend is targeted only for
> "Linux" targets. Please let me know if i am missing something.
Using closesocket() is misleading even if it does happen to call close()
on Linux.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-10 7:34 ` Amarnath Valluri
@ 2017-04-10 9:54 ` Marc-André Lureau
2017-04-10 10:07 ` Patrick Ohly
0 siblings, 1 reply; 34+ messages in thread
From: Marc-André Lureau @ 2017-04-10 9:54 UTC (permalink / raw)
To: Amarnath Valluri, Daniel P. Berrange; +Cc: qemu-devel, patrick.ohly, stefanb
Hi
On Mon, Apr 10, 2017 at 9:33 AM Amarnath Valluri <amarnath.valluri@intel.com>
wrote:
>
>
> On 07.04.2017 18:11, Marc-André Lureau wrote:
>
> Hi
>
> On Fri, Apr 7, 2017 at 4:41 PM Daniel P. Berrange
>
> > + .name = "data-path",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Socket path to use for data exhange",
> > + },
> > + {
> > + .name = "ctrl-path",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Socket path to use for out-of-band control messages",
> > + },
>
> I'm still not convinced by the need for 2 separate UNIX sockets, unless
> there's a performance reason, but that looks unlikely.
>
>
> We would also need something more than an implementation of the protocol
> that qemu is going to rely on as an external dependency. A protocol
> specification would help to start the discussion.
>
> I would agree with you, Can we adopt the current swtpm's control interface
> as initial proposal.
>
>
Perhaps, but I think it would be a mistake to rely on it without some
review.
> Alternatively, I would suggest to not rely on a public protocol,
>
> What do you mean by public protocol here, can you please help me
> understanding this.
>
>
By "public protocol", I mean qemu communication with a foreign project,
swtpm or other.
If qemu grows new needs, or if the protocol is found limited or buggy, it
may change. Subtle interactions may break between various implementations.
The minimum would be some versioning or capabilities. A document describing
the states and messages allowed/denied & effects would be quite necessary.
Otoh, there doesn't seem to be other users of this protocol, or other
implementations. So it may make sense to make it qemu-specific, and thus
"private": the protocol and implementation can evolve without risk to break
other users. This gives us a lot more flexibility and control, and doesn't
have to be very strictly documented (although it is still better to be
strict, but requires more effort).
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-10 9:54 ` Marc-André Lureau
@ 2017-04-10 10:07 ` Patrick Ohly
2017-04-10 16:14 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Patrick Ohly @ 2017-04-10 10:07 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Amarnath Valluri, Daniel P. Berrange, qemu-devel, stefanb
On Mon, 2017-04-10 at 09:54 +0000, Marc-André Lureau wrote:
> By "public protocol", I mean qemu communication with a foreign
> project, swtpm or other.
>
> If qemu grows new needs, or if the protocol is found limited or buggy,
> it may change. Subtle interactions may break between various
> implementations. The minimum would be some versioning or
> capabilities. A document describing the states and messages
> allowed/denied & effects would be quite necessary.
Stefan, is there any documentation besides the source?
Just asking, I don't think it is needed because...
> Otoh, there doesn't seem to be other users of this protocol, or other
> implementations. So it may make sense to make it qemu-specific, and
> thus "private": the protocol and implementation can evolve without
> risk to break other users. This gives us a lot more flexibility and
> control, and doesn't have to be very strictly documented (although it
> is still better to be strict, but requires more effort).
... I suspect it falls into this camp. I can't think of any users of the
protocol besides swtpm itself and now qemu. Stefan, is that correct?
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-10 10:07 ` Patrick Ohly
@ 2017-04-10 16:14 ` Stefan Berger
2017-04-10 21:11 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2017-04-10 16:14 UTC (permalink / raw)
To: Patrick Ohly, Marc-André Lureau
Cc: Amarnath Valluri, Daniel P. Berrange, qemu-devel
On 04/10/2017 06:07 AM, Patrick Ohly wrote:
> On Mon, 2017-04-10 at 09:54 +0000, Marc-André Lureau wrote:
>
>> By "public protocol", I mean qemu communication with a foreign
>> project, swtpm or other.
>>
>> If qemu grows new needs, or if the protocol is found limited or buggy,
>> it may change. Subtle interactions may break between various
>> implementations. The minimum would be some versioning or
>> capabilities. A document describing the states and messages
>> allowed/denied & effects would be quite necessary.
> Stefan, is there any documentation besides the source?
No.
>
> Just asking, I don't think it is needed because...
>
>> Otoh, there doesn't seem to be other users of this protocol, or other
>> implementations. So it may make sense to make it qemu-specific, and
>> thus "private": the protocol and implementation can evolve without
>> risk to break other users. This gives us a lot more flexibility and
>> control, and doesn't have to be very strictly documented (although it
>> is still better to be strict, but requires more effort).
> ... I suspect it falls into this camp. I can't think of any users of the
> protocol besides swtpm itself and now qemu. Stefan, is that correct?
>
Correct. Only swtpm client tools (swtpm_ioctl) and now your QEMU patches
are using the protocol.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-10 7:08 ` Amarnath Valluri
2017-04-10 8:31 ` Daniel P. Berrange
@ 2017-04-10 16:15 ` Stefan Berger
1 sibling, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-10 16:15 UTC (permalink / raw)
To: Amarnath Valluri, Daniel P. Berrange
Cc: qemu-devel, patrick.ohly, marcandre.lureau
On 04/10/2017 03:08 AM, Amarnath Valluri wrote:
>
>
> On 07.04.2017 17:41, Daniel P. Berrange wrote:
>> On Fri, Apr 07, 2017 at 05:30:31PM +0300, Amarnath Valluri wrote:
>>> This change introduces a new TPM backend driver that can communicate
>>> with
>>> swtpm(software TPM emulator) using unix domain socket interface.
>>>
>>> Swtpm uses two unix sockets, one for plain TPM commands and
>>> responses, and one
>>> for out-of-band control messages.
>>>
>>> The swtpm and associated tools can be found here:
>>> https://github.com/stefanberger/swtpm
>>>
>>> Usage:
>>> # setup TPM state directory
>>> mkdir /tmp/mytpm
>>> chown -R tss:root /tmp/mytpm
>>> /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
>>>
>>> # Ask qemu to use TPM emulator with given tpm state directory
>>> qemu-system-x86_64 \
>>> [...] \
>>> -tpmdev
>>> emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
>>> -device tpm-tis,tpmdev=tpm0 \
>>> [...]
>>>
>>> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
>>> ---
>>> configure | 15 +-
>>> hmp.c | 21 ++
>>> hw/tpm/Makefile.objs | 1 +
>>> hw/tpm/tpm_emulator.c | 927
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> hw/tpm/tpm_ioctl.h | 243 +++++++++++++
>>> qapi-schema.json | 36 +-
>>> qemu-options.hx | 53 ++-
>>> tpm.c | 2 +-
>>> 8 files changed, 1289 insertions(+), 9 deletions(-)
>>> create mode 100644 hw/tpm/tpm_emulator.c
>>> create mode 100644 hw/tpm/tpm_ioctl.h
>>> +static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
>>> +{
>>> + int fds[2] = { -1, -1 };
>>> + int ctrl_fds[2] = { -1, -1 };
>>> + pid_t cpid;
>>> +
>>> + if (!tpm_pt->ops->has_data_path) {
>>> + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
>>> + return -1;
>>> + }
>>> + }
>>> +
>>> + if (!tpm_pt->ops->has_ctrl_path) {
>>> + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
>>> + if (!tpm_pt->ops->has_data_path) {
>>> + closesocket(fds[0]);
>>> + closesocket(fds[1]);
>>> + }
>>> + return -1;
>>> + }
>>> + }
>>> +
>>> + cpid = qemu_fork(NULL);
>>> + if (cpid < 0) {
>>> + error_report("tpm-emulator: Fork failure: %s",
>>> strerror(errno));
>>> + if (!tpm_pt->ops->has_data_path) {
>>> + closesocket(fds[0]);
>>> + closesocket(fds[1]);
>>> + }
>>> + if (!tpm_pt->ops->has_ctrl_path) {
>>> + closesocket(ctrl_fds[0]);
>>> + closesocket(ctrl_fds[1]);
>>> + }
>>> + return -1;
>>> + }
>>> +
>>> + if (cpid == 0) { /* CHILD */
>>> + enum {
>>> + PARAM_PATH,
>>> + PARAM_IFACE,
>>> + PARAM_SERVER, PARAM_SERVER_ARGS,
>>> + PARAM_CTRL, PARAM_CTRL_ARGS,
>>> + PARAM_STATE, PARAM_STATE_ARGS,
>>> + PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
>>> + PARAM_LOG, PARAM_LOG_ARGS,
>>> + PARAM_MAX
>>> + };
>>> +
>>> + int i;
>>> + int data_fd = -1, ctrl_fd = -1;
>>> + char *argv[PARAM_MAX+1];
>>> +
>>> + /* close all unused inherited sockets */
>>> + if (fds[0] >= 0)
>>> + closesocket(fds[0]);
>>> + if (ctrl_fds[0] >= 0)
>>> + closesocket(ctrl_fds[0]);
>> The 'if' checks are pointless - its already guaranteed by the
>> fact you check socketpair() status.
> socketpairs might not be created in case of data-path & ctrl-path
> provided, so i feel these checks are needed.
>>
>>> + i = STDERR_FILENO + 1;
>>> + if (fds[1] >= 0) {
>>> + data_fd = dup2(fds[1], i++);
>>> + if (data_fd < 0) {
>>> + error_report("tpm-emulator: dup2() failure - %s",
>>> + strerror(errno));
>>> + goto exit_child;
>>> + }
>>> + }
>>> + if (ctrl_fds[1] >= 0) {
>>> + ctrl_fd = dup2(ctrl_fds[1], i++);
>>> + if (ctrl_fd < 0) {
>>> + error_report("tpm-emulator: dup2() failure - %s",
>>> + strerror(errno));
>>> + goto exit_child;
>>> + }
>>> + }
>>> + for ( ; i < _SC_OPEN_MAX; i++) {
>> Errr, _SC_OPEN_MAX is not the maximum number of FDs - it is parameter to
>> use with sysconf() to query the number of files - you must call
>> sysconf().
> Ya, thanks for educating me, i will change this.
>>
>>> + closesocket(i);
>> close, not closesocket - you can't assume these are all sockets.
> Does this change makes any difference, as per
> include/sysemu/os-posix.h, closesocket() is define as close(), and
> this backend is targeted only for "Linux" targets. Please let me know
> if i am missing something.
>>
>>
>>> + DPRINT("\n")
>>> + if (execv(tpm_pt->ops->path, (char * const *)argv) < 0) {
>>> + error_report("execv() failure : %s", strerror(errno));
>>> + }
>>> +
>>> +exit_child:
>>> + g_strfreev(argv);
>>> + if (data_fd >= 0)
>>> + closesocket(data_fd);
>>> + if (ctrl_fd >= 0)
>>> + closesocket(ctrl_fd);
>>> +
>>> + exit(0);
>> You need _exit(), not exit() as you don't want to run atexit() handlers
>> here. You also want '1' not '0' since this is a failure scenario.
> Sure, i will change this.
>>
>>> + } else { /* self */
>>> + struct stat st;
>>> + DPRINTF("child pid: %d", cpid);
>>> + int rc;
>>> + int timeout = 3; /* wait for max 3 seconds */
>>> +
>>> + /* close unsed sockets */
>>> + if (fds[1] >= 0)
>>> + closesocket(fds[1]);
>>> + if (ctrl_fds[1] >= 0)
>>> + closesocket(ctrl_fds[1]);
>>> +
>>> + while((rc = stat(TPM_EMULATOR_PIDFILE, &st)) < 0 &&
>>> timeout--) {
>>> + sleep(1);
>>> + }
>> A fixed 3 second timeout will inevitably cause failures on systems with
>> high load.
>>
>> Presumably you're trying to handle the scenario where the child process
>> exits without creating the pid file ?
> The check is mainly to know if the child setup is done and ready to
> accept requests.
>>
>> In which case you can use 'kill(cpid, 0)' and if errno == ESRCH
>> then the child has exited.
>>
>>> +
>>> + if (timeout == -1) {
>>> + error_report("tpm-emulator: failed to find pid file: %s",
>>> + strerror(errno));
>>> + goto err_kill_child;
>>> + }
>>> +
>>> + tpm_pt->data_ioc = _iochannel_new(tpm_pt->ops->data_path,
>>> fds[0], NULL);
>>> + if (!tpm_pt->data_ioc) {
>>> + error_report("tpm-emulator: Unable to connect socket :
>>> %s",
>>> + tpm_pt->ops->data_path);
>>> + goto err_kill_child;
>>> + }
>>> +
>>> + tpm_pt->ctrl_ioc = _iochannel_new(tpm_pt->ops->ctrl_path,
>>> ctrl_fds[0], NULL);
>>> + if (!tpm_pt->ctrl_ioc) {
>>> + error_report("tpm-emulator: Unable to connect socket :
>>> %s",
>>> + tpm_pt->ops->ctrl_path);
>>> + goto err_kill_child;
>>> + }
>>> +
>>> + tpm_pt->child_running = true;
>>> +
>>> + qemu_add_child_watch(cpid);
>>> +
>>> + qio_channel_add_watch(tpm_pt->data_ioc, G_IO_HUP|G_IO_ERR,
>>> + tpm_emulator_fd_handler, tpm_pt, NULL);
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +err_kill_child:
>>> + kill(cpid, SIGTERM);
>>> + closesocket(fds[0]);
>>> + closesocket(ctrl_fds[0]);
>> You can't assume the child has gone after a SIGTERM. You need to
>> check, and be prepared to SIGKILL after time reasonable time,
>> if needed.
> Ok, i will add this check.
>>
>>> + tpm_pt->child_running = false;
>>> +
>>> + return -1;
>>> +}
>>> +
>>> +static const QemuOptDesc tpm_emulator_cmdline_opts[] = {
>>> + TPM_STANDARD_CMDLINE_OPTS,
>>> + {
>>> + .name = "tpmstatedir",
>>> + .type = QEMU_OPT_STRING,
>>> + .help = "TPM state directroy",
>>> + },
>>> + {
>>> + .name = "spawn",
>>> + .type = QEMU_OPT_BOOL,
>>> + .help = "Wether to spwan given emlatory binary",
>>> + },
>>> + {
>>> + .name = "path",
>>> + .type = QEMU_OPT_STRING,
>>> + .help = "Path to TPM emulator binary",
>>> + },
>>> + {
>>> + .name = "data-path",
>>> + .type = QEMU_OPT_STRING,
>>> + .help = "Socket path to use for data exhange",
>>> + },
>>> + {
>>> + .name = "ctrl-path",
>>> + .type = QEMU_OPT_STRING,
>>> + .help = "Socket path to use for out-of-band control messages",
>>> + },
>> I'm still not convinced by the need for 2 separate UNIX sockets, unless
>> there's a performance reason, but that looks unlikely.
> I myself, is not expert of swtpm's interface, hence i cannot defend this.
> i am just trying to enable emulation support in qemu using swtpm.
> Stefan Berger is the right person to comment on this.
>
I don't understand what the issue is with running the control and data
channels over different file descriptors.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-10 16:14 ` Stefan Berger
@ 2017-04-10 21:11 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-10 21:11 UTC (permalink / raw)
To: Patrick Ohly, Marc-André Lureau
Cc: Amarnath Valluri, Daniel P. Berrange, qemu-devel
On 04/10/2017 12:14 PM, Stefan Berger wrote:
> On 04/10/2017 06:07 AM, Patrick Ohly wrote:
>> On Mon, 2017-04-10 at 09:54 +0000, Marc-André Lureau wrote:
>>
>>> By "public protocol", I mean qemu communication with a foreign
>>> project, swtpm or other.
>>>
>>> If qemu grows new needs, or if the protocol is found limited or buggy,
>>> it may change. Subtle interactions may break between various
>>> implementations. The minimum would be some versioning or
>>> capabilities. A document describing the states and messages
>>> allowed/denied & effects would be quite necessary.
>> Stefan, is there any documentation besides the source?
>
> No.
I now started to describe the protocol on the Wiki:
https://github.com/stefanberger/swtpm/wiki/Control-Channel-Specification
However, there's also a description in an existing man page:
https://github.com/stefanberger/swtpm/blob/master/man/man3/swtpm_ioctls.pod
>
>>
>> Just asking, I don't think it is needed because...
>>
>>> Otoh, there doesn't seem to be other users of this protocol, or other
>>> implementations. So it may make sense to make it qemu-specific, and
>>> thus "private": the protocol and implementation can evolve without
>>> risk to break other users. This gives us a lot more flexibility and
>>> control, and doesn't have to be very strictly documented (although it
>>> is still better to be strict, but requires more effort).
>> ... I suspect it falls into this camp. I can't think of any users of the
>> protocol besides swtpm itself and now qemu. Stefan, is that correct?
>>
>
> Correct. Only swtpm client tools (swtpm_ioctl) and now your QEMU
> patches are using the protocol.
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
` (8 preceding siblings ...)
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator Amarnath Valluri
@ 2017-04-12 23:52 ` no-reply
9 siblings, 0 replies; 34+ messages in thread
From: no-reply @ 2017-04-12 23:52 UTC (permalink / raw)
To: amarnath.valluri
Cc: famz, qemu-devel, patrick.ohly, marcandre.lureau, stefanb
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 1491575431-32170-1-git-send-email-amarnath.valluri@intel.com
Subject: [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
fb65a2b tpm: Added support for TPM emulator
0588e1b tpm-passthrough: move reusable code to utils
3dc626f tpm-backend: Move realloc_buffer() implementation to base class
3f0ba45 tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
2badc93 tmp backend: Add new api to read backend TpmInfo
8bb890f tpm-backend: Made few interface methods optional
e7a1f98 tpm-backend: Initialize and free data members in it's own methods
7e162a1 tpm-backend: Move thread handling inside TPMBackend
b739744 tpm-backend: Remove unneeded member variable from backend class
=== OUTPUT BEGIN ===
Checking PATCH 1/9: tpm-backend: Remove unneeded member variable from backend class...
Checking PATCH 2/9: tpm-backend: Move thread handling inside TPMBackend...
ERROR: space prohibited between function name and open parenthesis '('
#30: FILE: backends/tpm.c:27:
+ assert (k->handle_request != NULL);
total: 1 errors, 0 warnings, 308 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/9: tpm-backend: Initialize and free data members in it's own methods...
Checking PATCH 4/9: tpm-backend: Made few interface methods optional...
Checking PATCH 5/9: tmp backend: Add new api to read backend TpmInfo...
Checking PATCH 6/9: tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface...
Checking PATCH 7/9: tpm-backend: Move realloc_buffer() implementation to base class...
Checking PATCH 8/9: tpm-passthrough: move reusable code to utils...
Checking PATCH 9/9: tpm: Added support for TPM emulator...
ERROR: Macros with complex values should be enclosed in parenthesis
#206: FILE: hw/tpm/tpm_emulator.c:82:
+#define TPM_EMULATOR_IOCTL_TO_CMD(ioctlnum) \
+ ((ioctlnum >> _IOC_NRSHIFT) & _IOC_NRMASK) + 1
WARNING: line over 80 characters
#266: FILE: hw/tpm/tpm_emulator.c:142:
+ ret = qio_channel_read(tpm_pt->data_ioc, (char *)out, (size_t)out_len, NULL);
WARNING: line over 80 characters
#567: FILE: hw/tpm/tpm_emulator.c:443:
+static gboolean tpm_emulator_fd_handler(QIOChannel *ioc, GIOCondition cnd, void *opaque)
ERROR: trailing whitespace
#586: FILE: hw/tpm/tpm_emulator.c:462:
+ $
ERROR: trailing whitespace
#601: FILE: hw/tpm/tpm_emulator.c:477:
+ $
ERROR: spaces required around that '+' (ctx:VxV)
#640: FILE: hw/tpm/tpm_emulator.c:516:
+ char *argv[PARAM_MAX+1];
^
ERROR: braces {} are necessary for all arms of this statement
#643: FILE: hw/tpm/tpm_emulator.c:519:
+ if (fds[0] >= 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#645: FILE: hw/tpm/tpm_emulator.c:521:
+ if (ctrl_fds[0] >= 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#715: FILE: hw/tpm/tpm_emulator.c:591:
+ if (data_fd >= 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#717: FILE: hw/tpm/tpm_emulator.c:593:
+ if (ctrl_fd >= 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#728: FILE: hw/tpm/tpm_emulator.c:604:
+ if (fds[1] >= 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#730: FILE: hw/tpm/tpm_emulator.c:606:
+ if (ctrl_fds[1] >= 0)
[...]
ERROR: space required before the open parenthesis '('
#733: FILE: hw/tpm/tpm_emulator.c:609:
+ while((rc = stat(TPM_EMULATOR_PIDFILE, &st)) < 0 && timeout--) {
ERROR: trailing whitespace
#735: FILE: hw/tpm/tpm_emulator.c:611:
+ } $
WARNING: line over 80 characters
#750: FILE: hw/tpm/tpm_emulator.c:626:
+ tpm_pt->ctrl_ioc = _iochannel_new(tpm_pt->ops->ctrl_path, ctrl_fds[0], NULL);
ERROR: spaces required around that '|' (ctx:VxV)
#761: FILE: hw/tpm/tpm_emulator.c:637:
+ qio_channel_add_watch(tpm_pt->data_ioc, G_IO_HUP|G_IO_ERR,
^
total: 13 errors, 3 warnings, 1376 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class Amarnath Valluri
@ 2017-04-25 18:19 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 18:19 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> TPMDriverOps inside TPMBackend is not required, as it is supposed to be a class
> member. The only possible reason for keeping in TPMBackend was, to get the
> backend type in tpm.c where dedicated backend api, tpm_backend_get_type() is
> present.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> hw/tpm/tpm_passthrough.c | 4 ----
> include/sysemu/tpm_backend.h | 1 -
> tpm.c | 2 +-
> 3 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 9234eb3..a0baf5f 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -46,8 +46,6 @@
> #define TPM_PASSTHROUGH(obj) \
> OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH)
>
> -static const TPMDriverOps tpm_passthrough_driver;
> -
> /* data structures */
> typedef struct TPMPassthruThreadParams {
> TPMState *tpm_state;
> @@ -462,8 +460,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
> /* let frontend set the fe_model to proper value */
> tb->fe_model = -1;
>
> - tb->ops = &tpm_passthrough_driver;
> -
> if (tpm_passthrough_handle_device_opts(opts, tb)) {
> goto err_exit;
> }
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index b58f52d..e7f590d 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -50,7 +50,6 @@ struct TPMBackend {
> enum TpmModel fe_model;
> char *path;
> char *cancel_path;
> - const TPMDriverOps *ops;
>
> QLIST_ENTRY(TPMBackend) list;
> };
> diff --git a/tpm.c b/tpm.c
> index 9a7c711..0ee021a 100644
> --- a/tpm.c
> +++ b/tpm.c
> @@ -258,7 +258,7 @@ static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv)
> res->model = drv->fe_model;
> res->options = g_new0(TpmTypeOptions, 1);
>
> - switch (drv->ops->type) {
> + switch (tpm_backend_get_type(drv)) {
> case TPM_TYPE_PASSTHROUGH:
> res->options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
> tpo = g_new0(TPMPassthroughOptions, 1);
Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/9] tpm-backend: Move thread handling inside TPMBackend
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 2/9] tpm-backend: Move thread handling inside TPMBackend Amarnath Valluri
@ 2017-04-25 18:21 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 18:21 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> Move thread handling inside TPMBackend, this way backend implementations need
> not to maintain their own thread life cycle, instead they needs to implement
> 'handle_request()' class method that always been called from a thread.
>
> This change made tpm_backend_int.h kind of useless, hence removed it.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> backends/tpm.c | 63 +++++++++++++++++++++++++---------------
> hw/tpm/tpm_passthrough.c | 58 +++++-------------------------------
> include/sysemu/tpm_backend.h | 32 ++++++++++++--------
> include/sysemu/tpm_backend_int.h | 41 --------------------------
> 4 files changed, 68 insertions(+), 126 deletions(-)
> delete mode 100644 include/sysemu/tpm_backend_int.h
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 536f262..0ec0c67 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -18,7 +18,24 @@
> #include "qapi/qmp/qerror.h"
> #include "sysemu/tpm.h"
> #include "qemu/thread.h"
> -#include "sysemu/tpm_backend_int.h"
> +
> +static void tpm_backend_worker_thread(gpointer data, gpointer user_data)
> +{
> + TPMBackend *s = TPM_BACKEND(user_data);
> + TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> + assert (k->handle_request != NULL);
> + k->handle_request(s, (TPMBackendCmd)data);
> +}
> +
> +static void tpm_backend_thread_end(TPMBackend *s)
> +{
> + if (s->thread_pool) {
> + g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_END, NULL);
> + g_thread_pool_free(s->thread_pool, FALSE, TRUE);
> + s->thread_pool = NULL;
> + }
> +}
>
> enum TpmType tpm_backend_get_type(TPMBackend *s)
> {
> @@ -39,6 +56,8 @@ void tpm_backend_destroy(TPMBackend *s)
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> k->ops->destroy(s);
> +
> + tpm_backend_thread_end(s);
> }
>
> int tpm_backend_init(TPMBackend *s, TPMState *state,
> @@ -46,13 +65,23 @@ int tpm_backend_init(TPMBackend *s, TPMState *state,
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> - return k->ops->init(s, state, datacb);
> + s->tpm_state = state;
> + s->recv_data_callback = datacb;
> +
> + return k->ops->init(s);
> }
>
> int tpm_backend_startup_tpm(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> + /* terminate a running TPM */
> + tpm_backend_thread_end(s);
> +
> + s->thread_pool = g_thread_pool_new(tpm_backend_worker_thread, s, 1, TRUE,
> + NULL);
> + g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
> +
> return k->ops->startup_tpm(s);
> }
>
> @@ -72,9 +101,8 @@ size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
>
> void tpm_backend_deliver_request(TPMBackend *s)
> {
> - TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> -
> - k->ops->deliver_request(s);
> + g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD,
> + NULL);
> }
>
> void tpm_backend_reset(TPMBackend *s)
> @@ -82,6 +110,8 @@ void tpm_backend_reset(TPMBackend *s)
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> k->ops->reset(s);
> +
> + tpm_backend_thread_end(s);
> }
>
> void tpm_backend_cancel_cmd(TPMBackend *s)
> @@ -156,29 +186,15 @@ static void tpm_backend_instance_init(Object *obj)
> tpm_backend_prop_get_opened,
> tpm_backend_prop_set_opened,
> NULL);
> -}
>
> -void tpm_backend_thread_deliver_request(TPMBackendThread *tbt)
> -{
> - g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD, NULL);
> }
>
> -void tpm_backend_thread_create(TPMBackendThread *tbt,
> - GFunc func, gpointer user_data)
> +static void tpm_backend_instance_finalize(Object *obj)
> {
> - if (!tbt->pool) {
> - tbt->pool = g_thread_pool_new(func, user_data, 1, TRUE, NULL);
> - g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
> - }
> -}
> + TPMBackend *s = TPM_BACKEND(obj);
>
> -void tpm_backend_thread_end(TPMBackendThread *tbt)
> -{
> - if (tbt->pool) {
> - g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_END, NULL);
> - g_thread_pool_free(tbt->pool, FALSE, TRUE);
> - tbt->pool = NULL;
> - }
> + g_free(s->id);
I don't see this being removed somewhere else. Makes me think that we'd
get a double-free here.
> + tpm_backend_thread_end(s);
> }
>
> static const TypeInfo tpm_backend_info = {
> @@ -186,6 +202,7 @@ static const TypeInfo tpm_backend_info = {
> .parent = TYPE_OBJECT,
> .instance_size = sizeof(TPMBackend),
> .instance_init = tpm_backend_instance_init,
> + .instance_finalize = tpm_backend_instance_finalize,
> .class_size = sizeof(TPMBackendClass),
> .abstract = true,
> };
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index a0baf5f..f50d9cf 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -30,7 +30,6 @@
> #include "tpm_int.h"
> #include "hw/hw.h"
> #include "hw/i386/pc.h"
> -#include "sysemu/tpm_backend_int.h"
> #include "tpm_tis.h"
> #include "tpm_util.h"
>
> @@ -47,20 +46,9 @@
> OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH)
>
> /* data structures */
> -typedef struct TPMPassthruThreadParams {
> - TPMState *tpm_state;
> -
> - TPMRecvDataCB *recv_data_callback;
> - TPMBackend *tb;
> -} TPMPassthruThreadParams;
> -
> struct TPMPassthruState {
> TPMBackend parent;
>
> - TPMBackendThread tbt;
> -
> - TPMPassthruThreadParams tpm_thread_params;
> -
> char *tpm_dev;
> int tpm_fd;
> bool tpm_executing;
> @@ -214,12 +202,9 @@ static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt,
> selftest_done);
> }
>
> -static void tpm_passthrough_worker_thread(gpointer data,
> - gpointer user_data)
> +static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd cmd)
> {
> - TPMPassthruThreadParams *thr_parms = user_data;
> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(thr_parms->tb);
> - TPMBackendCmd cmd = (TPMBackendCmd)data;
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> bool selftest_done = false;
>
> DPRINTF("tpm_passthrough: processing command type %d\n", cmd);
> @@ -227,12 +212,12 @@ 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_data,
> + tb->tpm_state->locty_data,
> &selftest_done);
>
> - thr_parms->recv_data_callback(thr_parms->tpm_state,
> - thr_parms->tpm_state->locty_number,
> - selftest_done);
> + tb->recv_data_callback(tb->tpm_state,
> + tb->tpm_state->locty_number,
> + selftest_done);
> break;
> case TPM_BACKEND_CMD_INIT:
> case TPM_BACKEND_CMD_END:
> @@ -248,15 +233,6 @@ static void tpm_passthrough_worker_thread(gpointer data,
> */
> static int tpm_passthrough_startup_tpm(TPMBackend *tb)
> {
> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> -
> - /* terminate a running TPM */
> - tpm_backend_thread_end(&tpm_pt->tbt);
> -
> - tpm_backend_thread_create(&tpm_pt->tbt,
> - tpm_passthrough_worker_thread,
> - &tpm_pt->tpm_thread_params);
> -
> return 0;
> }
>
> @@ -268,20 +244,11 @@ static void tpm_passthrough_reset(TPMBackend *tb)
>
> tpm_passthrough_cancel_cmd(tb);
>
> - tpm_backend_thread_end(&tpm_pt->tbt);
> -
> tpm_pt->had_startup_error = false;
> }
>
> -static int tpm_passthrough_init(TPMBackend *tb, TPMState *s,
> - TPMRecvDataCB *recv_data_cb)
> +static int tpm_passthrough_init(TPMBackend *tb)
> {
> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> -
> - tpm_pt->tpm_thread_params.tpm_state = s;
> - tpm_pt->tpm_thread_params.recv_data_callback = recv_data_cb;
> - tpm_pt->tpm_thread_params.tb = tb;
> -
> return 0;
> }
>
> @@ -315,13 +282,6 @@ static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb)
> return sb->size;
> }
>
> -static void tpm_passthrough_deliver_request(TPMBackend *tb)
> -{
> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> -
> - tpm_backend_thread_deliver_request(&tpm_pt->tbt);
> -}
> -
> static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
> {
> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> @@ -483,8 +443,6 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
>
> tpm_passthrough_cancel_cmd(tb);
>
> - tpm_backend_thread_end(&tpm_pt->tbt);
> -
> qemu_close(tpm_pt->tpm_fd);
> qemu_close(tpm_pt->cancel_fd);
>
> @@ -520,7 +478,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
> .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,
> @@ -540,6 +497,7 @@ static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
> TPMBackendClass *tbc = TPM_BACKEND_CLASS(klass);
>
> tbc->ops = &tpm_passthrough_driver;
> + tbc->handle_request = tpm_passthrough_handle_request;
> }
>
> static const TypeInfo tpm_passthrough_info = {
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index e7f590d..a538a7f 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -29,22 +29,24 @@
>
> typedef struct TPMBackendClass TPMBackendClass;
> typedef struct TPMBackend TPMBackend;
> -
> typedef struct TPMDriverOps TPMDriverOps;
> +typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty, bool selftest_done);
>
> -struct TPMBackendClass {
> - ObjectClass parent_class;
> -
> - const TPMDriverOps *ops;
> -
> - void (*opened)(TPMBackend *s, Error **errp);
> -};
> +typedef enum TPMBackendCmd {
> + TPM_BACKEND_CMD_INIT = 1,
> + TPM_BACKEND_CMD_PROCESS_CMD,
> + TPM_BACKEND_CMD_END,
> + TPM_BACKEND_CMD_TPM_RESET,
> +} TPMBackendCmd;
>
> struct TPMBackend {
> Object parent;
>
> /*< protected >*/
> bool opened;
> + TPMState *tpm_state;
> + GThreadPool *thread_pool;
> + TPMRecvDataCB *recv_data_callback;
>
> char *id;
> enum TpmModel fe_model;
> @@ -54,7 +56,15 @@ struct TPMBackend {
> QLIST_ENTRY(TPMBackend) list;
> };
>
> -typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty, bool selftest_done);
> +struct TPMBackendClass {
> + ObjectClass parent_class;
> +
> + const TPMDriverOps *ops;
> +
> + void (*opened)(TPMBackend *s, Error **errp);
> +
> + void (*handle_request)(TPMBackend *s, TPMBackendCmd cmd);
> +};
>
> typedef struct TPMSizedBuffer {
> uint32_t size;
> @@ -71,7 +81,7 @@ struct TPMDriverOps {
> void (*destroy)(TPMBackend *t);
>
> /* initialize the backend */
> - int (*init)(TPMBackend *t, TPMState *s, TPMRecvDataCB *datacb);
> + int (*init)(TPMBackend *t);
> /* start up the TPM on the backend */
> int (*startup_tpm)(TPMBackend *t);
> /* returns true if nothing will ever answer TPM requests */
> @@ -79,8 +89,6 @@ struct TPMDriverOps {
>
> size_t (*realloc_buffer)(TPMSizedBuffer *sb);
>
> - void (*deliver_request)(TPMBackend *t);
> -
> void (*reset)(TPMBackend *t);
>
> void (*cancel_cmd)(TPMBackend *t);
> diff --git a/include/sysemu/tpm_backend_int.h b/include/sysemu/tpm_backend_int.h
> deleted file mode 100644
> index 00639dd..0000000
> --- a/include/sysemu/tpm_backend_int.h
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/*
> - * common TPM backend driver functions
> - *
> - * Copyright (c) 2012-2013 IBM Corporation
> - * Authors:
> - * Stefan Berger <stefanb@us.ibm.com>
> - *
> - * This library is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> - *
> - * This library is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with this library; if not, see <http://www.gnu.org/licenses/>
> - */
> -
> -#ifndef TPM_BACKEND_INT_H
> -#define TPM_BACKEND_INT_H
> -
> -typedef struct TPMBackendThread {
> - GThreadPool *pool;
> -} TPMBackendThread;
> -
> -void tpm_backend_thread_deliver_request(TPMBackendThread *tbt);
> -void tpm_backend_thread_create(TPMBackendThread *tbt,
> - GFunc func, gpointer user_data);
> -void tpm_backend_thread_end(TPMBackendThread *tbt);
> -
> -typedef enum TPMBackendCmd {
> - TPM_BACKEND_CMD_INIT = 1,
> - TPM_BACKEND_CMD_PROCESS_CMD,
> - TPM_BACKEND_CMD_END,
> - TPM_BACKEND_CMD_TPM_RESET,
> -} TPMBackendCmd;
> -
> -#endif /* TPM_BACKEND_INT_H */
Otherwise looks good.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods Amarnath Valluri
@ 2017-04-25 18:27 ` Stefan Berger
2017-05-02 7:09 ` Amarnath Valluri
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 18:27 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau, berrange
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> Initialize and free TPMBackend data members in it's own instance_init() and
> instance_finalize methods.
>
> Took the opportunity to fix object cleanup in tpm_backend_{create,destroy}
> methods
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> backends/tpm.c | 8 ++++++--
> hw/tpm/tpm_passthrough.c | 18 +++++++++++-------
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 0ec0c67..99d2b2b 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -57,7 +57,7 @@ void tpm_backend_destroy(TPMBackend *s)
>
> k->ops->destroy(s);
>
> - tpm_backend_thread_end(s);
> + object_unref(OBJECT(s));
> }
>
> int tpm_backend_init(TPMBackend *s, TPMState *state,
> @@ -182,11 +182,13 @@ static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp)
>
> static void tpm_backend_instance_init(Object *obj)
> {
> + TPMBackend *s = TPM_BACKEND(obj);
> +
> object_property_add_bool(obj, "opened",
> tpm_backend_prop_get_opened,
> tpm_backend_prop_set_opened,
> NULL);
> -
> + s->fe_model = -1;
> }
>
> static void tpm_backend_instance_finalize(Object *obj)
> @@ -194,6 +196,8 @@ static void tpm_backend_instance_finalize(Object *obj)
> TPMBackend *s = TPM_BACKEND(obj);
>
> g_free(s->id);
> + g_free(s->path);
> + g_free(s->cancel_path);
> tpm_backend_thread_end(s);
> }
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index f50d9cf..65831b5 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -417,8 +417,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>
> tb->id = g_strdup(id);
> - /* let frontend set the fe_model to proper value */
> - tb->fe_model = -1;
>
> if (tpm_passthrough_handle_device_opts(opts, tb)) {
> goto err_exit;
> @@ -432,7 +430,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
> return tb;
>
> err_exit:
> - g_free(tb->id);
> + object_unref(obj);
>
> return NULL;
> }
> @@ -445,10 +443,6 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
>
> qemu_close(tpm_pt->tpm_fd);
> qemu_close(tpm_pt->cancel_fd);
> -
> - g_free(tb->id);
> - g_free(tb->path);
> - g_free(tb->cancel_path);
Looks like here you are correcting the error from the previous patch...
> g_free(tpm_pt->tpm_dev);
> }
>
> @@ -486,10 +480,20 @@ static const TPMDriverOps tpm_passthrough_driver = {
>
> static void tpm_passthrough_inst_init(Object *obj)
> {
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
> +
> + tpm_pt->tpm_fd = -1;
> + tpm_pt->cancel_fd = -1;
> }
>
> static void tpm_passthrough_inst_finalize(Object *obj)
> {
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
> +
> + tpm_passthrough_cancel_cmd(TPM_BACKEND(obj));
> +
> + qemu_close(tpm_pt->tpm_fd);
> + qemu_close(tpm_pt->cancel_fd);
> }
With these I am wondering whether they should not have been moved here
from tpm_passthrough_destroy()? The appear there as well.
> static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/9] tpm-backend: Made few interface methods optional
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 4/9] tpm-backend: Made few interface methods optional Amarnath Valluri
@ 2017-04-25 18:29 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 18:29 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau, berrange
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> This allows backend implementations left optional interface methods.
> For mandatory methods assertion checks added.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> backends/tpm.c | 28 +++++++++++++++++++++-------
> hw/tpm/tpm_passthrough.c | 16 ----------------
> 2 files changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 99d2b2b..c2be17b 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -48,14 +48,16 @@ const char *tpm_backend_get_desc(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> - return k->ops->desc();
> + return k->ops->desc ? k->ops->desc() : "";
> }
>
> void tpm_backend_destroy(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> - k->ops->destroy(s);
> + if (k->ops->destroy) {
> + k->ops->destroy(s);
> + }
>
> object_unref(OBJECT(s));
> }
> @@ -68,7 +70,7 @@ int tpm_backend_init(TPMBackend *s, TPMState *state,
> s->tpm_state = state;
> s->recv_data_callback = datacb;
>
> - return k->ops->init(s);
> + return k->ops->init ? k->ops->init(s) : 0;
> }
>
> int tpm_backend_startup_tpm(TPMBackend *s)
> @@ -82,13 +84,15 @@ int tpm_backend_startup_tpm(TPMBackend *s)
> NULL);
> g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
>
> - return k->ops->startup_tpm(s);
> + return k->ops->startup_tpm ? k->ops->startup_tpm(s) : 0;
> }
>
> bool tpm_backend_had_startup_error(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> + assert(k->ops->had_startup_error);
> +
> return k->ops->had_startup_error(s);
> }
>
> @@ -96,6 +100,8 @@ size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> + assert(k->ops->realloc_buffer);
> +
> return k->ops->realloc_buffer(sb);
> }
>
> @@ -109,7 +115,9 @@ void tpm_backend_reset(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> - k->ops->reset(s);
> + if (k->ops->reset) {
> + k->ops->reset(s);
> + }
>
> tpm_backend_thread_end(s);
> }
> @@ -118,6 +126,8 @@ void tpm_backend_cancel_cmd(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> + g_assert(k->ops->cancel_cmd);
> +
> k->ops->cancel_cmd(s);
> }
>
> @@ -125,20 +135,24 @@ bool tpm_backend_get_tpm_established_flag(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> - return k->ops->get_tpm_established_flag(s);
> + return k->ops->get_tpm_established_flag ?
> + k->ops->get_tpm_established_flag(s) : false;
> }
>
> int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> - return k->ops->reset_tpm_established_flag(s, locty);
> + return k->ops->reset_tpm_established_flag ?
> + k->ops->reset_tpm_established_flag(s, locty) : 0;
> }
>
> TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>
> + assert(k->ops->get_tpm_version);
> +
> return k->ops->get_tpm_version(s);
> }
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 65831b5..0f543bd 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -227,15 +227,6 @@ static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd cmd)
> }
> }
>
> -/*
> - * Start the TPM (thread). If it had been started before, then terminate
> - * and start it again.
> - */
> -static int tpm_passthrough_startup_tpm(TPMBackend *tb)
> -{
> - return 0;
> -}
> -
> static void tpm_passthrough_reset(TPMBackend *tb)
> {
> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> @@ -247,11 +238,6 @@ static void tpm_passthrough_reset(TPMBackend *tb)
> tpm_pt->had_startup_error = false;
> }
>
> -static int tpm_passthrough_init(TPMBackend *tb)
> -{
> - return 0;
> -}
> -
> static bool tpm_passthrough_get_tpm_established_flag(TPMBackend *tb)
> {
> return false;
> @@ -467,8 +453,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
> .desc = tpm_passthrough_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,
Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo Amarnath Valluri
@ 2017-04-25 18:51 ` Stefan Berger
2017-04-25 18:59 ` Eric Blake
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 18:51 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau, berrange
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> TPM configuration options are backend implementation details and shall not be
> part of base TPMBackend object, and these shall not be accessed directly outside
> of the class, hence added a new interface method, get_tpm_options() to
> TPMDriverOps., which shall be implemented by the derived classes to return
> configured tpm options.
>
> A new tpm backend api - tpm_backend_query_tpm() which uses _get_tpm_options() to
> prepare TpmInfo.
>
> Made TPMPassthroughOptions type inherited from newly defined TPMOptions base type.
> The TPMOptions base type is intentionally left empty and supposed to be
> inherited by all backend implementations to define their tpm configuration
> options.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> backends/tpm.c | 24 +++++++++++++++++--
> hmp.c | 10 ++++----
> hw/tpm/tpm_passthrough.c | 57 +++++++++++++++++++++++++++++++++++---------
> include/sysemu/tpm_backend.h | 25 +++++++++++++++++--
> qapi-schema.json | 41 +++++++++++++++----------------
> tpm.c | 32 +------------------------
> 6 files changed, 118 insertions(+), 71 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index c2be17b..c96f462 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -156,6 +156,28 @@ TPMVersion tpm_backend_get_tpm_version(TPMBackend *s)
> return k->ops->get_tpm_version(s);
> }
>
> +TPMOptions *tpm_backend_get_tpm_options(TPMBackend *s)
> +{
> + TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> + assert(k->ops->get_tpm_options);
> +
> + return k->ops->get_tpm_options(s);
> +}
> +
> +TPMInfo *tpm_backend_query_tpm(TPMBackend *s)
> +{
> + TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> + TPMInfo *info = g_new0(TPMInfo, 1);
> +
> + info->type = k->ops->type;
> + info->id = g_strdup(s->id);
> + info->model = s->fe_model;
> + info->options = tpm_backend_get_tpm_options(s);
> +
> + return info;
> +}
> +
> static bool tpm_backend_prop_get_opened(Object *obj, Error **errp)
> {
> TPMBackend *s = TPM_BACKEND(obj);
> @@ -210,8 +232,6 @@ static void tpm_backend_instance_finalize(Object *obj)
> TPMBackend *s = TPM_BACKEND(obj);
>
> g_free(s->id);
> - g_free(s->path);
> - g_free(s->cancel_path);
> tpm_backend_thread_end(s);
> }
>
> diff --git a/hmp.c b/hmp.c
> index edb8970..9caf7c8 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -955,18 +955,18 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> c, TpmModel_lookup[ti->model]);
>
> monitor_printf(mon, " \\ %s: type=%s",
> - ti->id, TpmTypeOptionsKind_lookup[ti->options->type]);
> + ti->id, TpmType_lookup[ti->type]);
>
> - switch (ti->options->type) {
> - case TPM_TYPE_OPTIONS_KIND_PASSTHROUGH:
> - tpo = ti->options->u.passthrough.data;
> + switch (ti->type) {
> + case TPM_TYPE_PASSTHROUGH:
> + tpo = (TPMPassthroughOptions *)ti->options;
> monitor_printf(mon, "%s%s%s%s",
> tpo->has_path ? ",path=" : "",
> tpo->has_path ? tpo->path : "",
> tpo->has_cancel_path ? ",cancel-path=" : "",
> tpo->has_cancel_path ? tpo->cancel_path : "");
> break;
> - case TPM_TYPE_OPTIONS_KIND__MAX:
> + default:
> break;
> }
> monitor_printf(mon, "\n");
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 0f543bd..71bdf25 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -49,6 +49,7 @@
> struct TPMPassthruState {
> TPMBackend parent;
>
> + TPMPassthroughOptions *ops;
> char *tpm_dev;
> int tpm_fd;
> bool tpm_executing;
> @@ -313,15 +314,14 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
> * in Documentation/ABI/stable/sysfs-class-tpm.
> * From /dev/tpm0 create /sys/class/misc/tpm0/device/cancel
> */
> -static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
> +static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
> {
> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> int fd = -1;
> char *dev;
> char path[PATH_MAX];
>
> - if (tb->cancel_path) {
> - fd = qemu_open(tb->cancel_path, O_WRONLY);
> + if (tpm_pt->ops->cancel_path) {
> + fd = qemu_open(tpm_pt->ops->cancel_path, O_WRONLY);
> if (fd < 0) {
> error_report("Could not open TPM cancel path : %s",
> strerror(errno));
> @@ -336,7 +336,7 @@ static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb)
> dev) < sizeof(path)) {
> fd = qemu_open(path, O_WRONLY);
> if (fd >= 0) {
> - tb->cancel_path = g_strdup(path);
> + tpm_pt->ops->cancel_path = g_strdup(path);
> } else {
> error_report("tpm_passthrough: Could not open TPM cancel "
> "path %s : %s", path, strerror(errno));
> @@ -356,17 +356,24 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
> const char *value;
>
> value = qemu_opt_get(opts, "cancel-path");
> - tb->cancel_path = g_strdup(value);
> + if (value) {
> + tpm_pt->ops->cancel_path = g_strdup(value);
> + tpm_pt->ops->has_cancel_path = true;
> + } else {
> + tpm_pt->ops->has_cancel_path = false;
> + }
>
> value = qemu_opt_get(opts, "path");
> if (!value) {
> value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
> + tpm_pt->ops->has_path = false;
> + } else {
> + tpm_pt->ops->has_path = true;
> }
>
> + tpm_pt->ops->path = g_strdup(value);
> tpm_pt->tpm_dev = g_strdup(value);
>
> - tb->path = g_strdup(tpm_pt->tpm_dev);
> -
> tpm_pt->tpm_fd = qemu_open(tpm_pt->tpm_dev, O_RDWR);
> if (tpm_pt->tpm_fd < 0) {
> error_report("Cannot access TPM device using '%s': %s",
> @@ -387,8 +394,8 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
> tpm_pt->tpm_fd = -1;
>
> err_free_parameters:
> - g_free(tb->path);
> - tb->path = NULL;
> + g_free(tpm_pt->ops->path);
> + tpm_pt->ops->path = NULL;
>
> g_free(tpm_pt->tpm_dev);
> tpm_pt->tpm_dev = NULL;
> @@ -408,7 +415,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
> goto err_exit;
> }
>
> - tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tb);
> + tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tpm_pt);
> if (tpm_pt->cancel_fd < 0) {
> goto err_exit;
> }
> @@ -430,6 +437,30 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
> qemu_close(tpm_pt->tpm_fd);
> qemu_close(tpm_pt->cancel_fd);
> g_free(tpm_pt->tpm_dev);
> +
> + qapi_free_TPMPassthroughOptions(tpm_pt->ops);
> +}
> +
> +static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
> +{
> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> + TPMPassthroughOptions *ops = g_new0(TPMPassthroughOptions, 1);
> +
> + if (!ops) {
> + return NULL;
> + }
> +
> + if (tpm_pt->ops->has_path) {
> + ops->has_path = true;
> + ops->path = g_strdup(tpm_pt->ops->path);
> + }
> +
> + if (tpm_pt->ops->has_cancel_path) {
> + ops->has_cancel_path = true;
> + ops->cancel_path = g_strdup(tpm_pt->ops->cancel_path);
> + }
> +
> + return (TPMOptions *)ops;
> }
>
> static const QemuOptDesc tpm_passthrough_cmdline_opts[] = {
> @@ -460,12 +491,14 @@ static const TPMDriverOps tpm_passthrough_driver = {
> .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,
> + .get_tpm_options = tpm_passthrough_get_tpm_options,
> };
>
> static void tpm_passthrough_inst_init(Object *obj)
> {
> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
>
> + tpm_pt->ops = g_new0(TPMPassthroughOptions, 1);
> tpm_pt->tpm_fd = -1;
> tpm_pt->cancel_fd = -1;
> }
> @@ -478,6 +511,8 @@ static void tpm_passthrough_inst_finalize(Object *obj)
>
> qemu_close(tpm_pt->tpm_fd);
> qemu_close(tpm_pt->cancel_fd);
> +
> + qapi_free_TPMPassthroughOptions(tpm_pt->ops);
> }
>
> static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index a538a7f..7f4d621 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -48,10 +48,9 @@ struct TPMBackend {
> GThreadPool *thread_pool;
> TPMRecvDataCB *recv_data_callback;
>
> + /* <public> */
> char *id;
> enum TpmModel fe_model;
> - char *path;
> - char *cancel_path;
>
> QLIST_ENTRY(TPMBackend) list;
> };
> @@ -98,6 +97,8 @@ struct TPMDriverOps {
> int (*reset_tpm_established_flag)(TPMBackend *t, uint8_t locty);
>
> TPMVersion (*get_tpm_version)(TPMBackend *t);
> +
> + TPMOptions *(*get_tpm_options)(TPMBackend *t);
> };
>
>
> @@ -230,6 +231,26 @@ void tpm_backend_open(TPMBackend *s, Error **errp);
> */
> TPMVersion tpm_backend_get_tpm_version(TPMBackend *s);
>
> +/**
> + * tpm_backend_get_tpm_options:
> + * @s: the backend
> + *
> + * Get the backend configuration options
> + *
> + * Returns newly allocated TPMOptions
> + */
> +TPMOptions *tpm_backend_get_tpm_options(TPMBackend *s);
> +
> +/**
> + * tpm_backend_query_tpm:
> + * @s: the backend
> + *
> + * Query backend tpm info
> + *
> + * Returns newly allocated TPMInfo
> + */
> +TPMInfo *tpm_backend_query_tpm(TPMBackend *s);
> +
> TPMBackend *qemu_find_tpm(const char *id);
>
> const TPMDriverOps *tpm_get_backend_driver(const char *type);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b921994..3f1ca20 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -5140,6 +5140,16 @@
> { 'command': 'query-tpm-types', 'returns': ['TpmType'] }
>
> ##
> +# @TPMOptions:
> +#
> +# Base type for TPM options
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'TPMOptions',
> + 'data': { } }
> +
> +##
> # @TPMPassthroughOptions:
> #
> # Information about the TPM passthrough type
> @@ -5151,20 +5161,9 @@
> #
> # Since: 1.5
> ##
> -{ 'struct': 'TPMPassthroughOptions', 'data': { '*path' : 'str',
> - '*cancel-path' : 'str'} }
> +{ 'struct': 'TPMPassthroughOptions', 'base': 'TPMOptions',
> + 'data': { '*path' : 'str', '*cancel-path' : 'str'} }
I don't think that you can change it like this anymore since this is
part of the public interface.
> -##
> -# @TpmTypeOptions:
> -#
> -# A union referencing different TPM backend types' configuration options
> -#
> -# @type: 'passthrough' The configuration options for the TPM passthrough type
> -#
> -# Since: 1.5
> -##
> -{ 'union': 'TpmTypeOptions',
> - 'data': { 'passthrough' : 'TPMPassthroughOptions' } }
>
> ##
> # @TPMInfo:
> @@ -5173,6 +5172,8 @@
> #
> # @id: The Id of the TPM
> #
> +# @type: The TPM backend type
> +#
> # @model: The TPM frontend model
> #
> # @options: The TPM (backend) type configuration options
> @@ -5181,8 +5182,9 @@
> ##
> { 'struct': 'TPMInfo',
> 'data': {'id': 'str',
> + 'type': 'TpmType',
> 'model': 'TpmModel',
> - 'options': 'TpmTypeOptions' } }
> + 'options': 'TPMOptions' } }
>
> ##
> # @query-tpm:
> @@ -5199,13 +5201,12 @@
> # <- { "return":
> # [
> # { "model": "tpm-tis",
> +# "type": "passthrough",
> # "options":
> -# { "type": "passthrough",
> -# "data":
> -# { "cancel-path": "/sys/class/misc/tpm0/device/cancel",
> -# "path": "/dev/tpm0"
> -# }
> -# },
> +# {
> +# "cancel-path": "/sys/class/misc/tpm0/device/cancel",
> +# "path": "/dev/tpm0"
> +# },
> # "id": "tpm0"
> # }
> # ]
> diff --git a/tpm.c b/tpm.c
> index 0ee021a..1b6b550 100644
> --- a/tpm.c
> +++ b/tpm.c
> @@ -249,36 +249,6 @@ static const TPMDriverOps *tpm_driver_find_by_type(enum TpmType type)
> return NULL;
> }
>
> -static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv)
> -{
> - TPMInfo *res = g_new0(TPMInfo, 1);
> - TPMPassthroughOptions *tpo;
> -
> - res->id = g_strdup(drv->id);
> - res->model = drv->fe_model;
> - res->options = g_new0(TpmTypeOptions, 1);
> -
> - switch (tpm_backend_get_type(drv)) {
> - case TPM_TYPE_PASSTHROUGH:
> - res->options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
> - tpo = g_new0(TPMPassthroughOptions, 1);
> - res->options->u.passthrough.data = tpo;
> - if (drv->path) {
> - tpo->path = g_strdup(drv->path);
> - tpo->has_path = true;
> - }
> - if (drv->cancel_path) {
> - tpo->cancel_path = g_strdup(drv->cancel_path);
> - tpo->has_cancel_path = true;
> - }
> - break;
> - case TPM_TYPE__MAX:
> - break;
> - }
> -
> - return res;
> -}
> -
> /*
> * Walk the list of active TPM backends and collect information about them
> * following the schema description in qapi-schema.json.
> @@ -293,7 +263,7 @@ TPMInfoList *qmp_query_tpm(Error **errp)
> continue;
> }
> info = g_new0(TPMInfoList, 1);
> - info->value = qmp_query_tpm_inst(drv);
> + info->value = tpm_backend_query_tpm(drv);
>
> if (!cur_item) {
> head = cur_item = info;
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface Amarnath Valluri
@ 2017-04-25 18:59 ` Stefan Berger
2017-05-02 7:42 ` Amarnath Valluri
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 18:59 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau, berrange
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> As TPMBackend is a Qemu Object, we can use object_unref() inplace of
> tpm_backend_destroy() to free the backend object, hence removed destroy() from
> TPMDriverOps interface.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> backends/tpm.c | 11 -----------
> hw/tpm/tpm_passthrough.c | 14 --------------
> include/sysemu/tpm_backend.h | 7 -------
> tpm.c | 2 +-
> 4 files changed, 1 insertion(+), 33 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index c96f462..3493df6 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s)
> return k->ops->desc ? k->ops->desc() : "";
> }
>
> -void tpm_backend_destroy(TPMBackend *s)
> -{
> - TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> -
> - if (k->ops->destroy) {
> - k->ops->destroy(s);
> - }
> -
> - object_unref(OBJECT(s));
> -}
> -
> int tpm_backend_init(TPMBackend *s, TPMState *state,
> TPMRecvDataCB *datacb)
> {
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 71bdf25..8e11ed3 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -428,19 +428,6 @@ err_exit:
> return NULL;
> }
>
> -static void tpm_passthrough_destroy(TPMBackend *tb)
> -{
> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> -
> - tpm_passthrough_cancel_cmd(tb);
> -
> - qemu_close(tpm_pt->tpm_fd);
> - qemu_close(tpm_pt->cancel_fd);
> - g_free(tpm_pt->tpm_dev);
This g_free should be in the finalizer as well, right? It isn't. So we
will leak some memory.
> -
> - qapi_free_TPMPassthroughOptions(tpm_pt->ops);
> -}
> -
> static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
> {
> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
> .opts = tpm_passthrough_cmdline_opts,
> .desc = tpm_passthrough_create_desc,
> .create = tpm_passthrough_create,
> - .destroy = tpm_passthrough_destroy,
> .realloc_buffer = tpm_passthrough_realloc_buffer,
> .reset = tpm_passthrough_reset,
> .had_startup_error = tpm_passthrough_get_startup_error,
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index 7f4d621..8f8f133 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -77,7 +77,6 @@ struct TPMDriverOps {
> const char *(*desc)(void);
>
> TPMBackend *(*create)(QemuOpts *opts, const char *id);
> - void (*destroy)(TPMBackend *t);
>
> /* initialize the backend */
> int (*init)(TPMBackend *t);
> @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
> const char *tpm_backend_get_desc(TPMBackend *s);
>
> /**
> - * tpm_backend_destroy:
> - * @s: the backend to destroy
> - */
> -void tpm_backend_destroy(TPMBackend *s);
> -
> -/**
> * tpm_backend_init:
> * @s: the backend to initialized
> * @state: TPMState
> diff --git a/tpm.c b/tpm.c
> index 1b6b550..43d980e 100644
> --- a/tpm.c
> +++ b/tpm.c
> @@ -197,7 +197,7 @@ void tpm_cleanup(void)
>
> QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
> QLIST_REMOVE(drv, list);
> - tpm_backend_destroy(drv);
> + object_unref(OBJECT(drv));
> }
> }
>
I *think* you should apply this patch after introducing the
tpm_passthrough_inst_finalize or merge it into that one.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo
2017-04-25 18:51 ` Stefan Berger
@ 2017-04-25 18:59 ` Eric Blake
2017-05-02 7:17 ` Amarnath Valluri
0 siblings, 1 reply; 34+ messages in thread
From: Eric Blake @ 2017-04-25 18:59 UTC (permalink / raw)
To: Stefan Berger, Amarnath Valluri, qemu-devel
Cc: patrick.ohly, marcandre.lureau
[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]
On 04/25/2017 01:51 PM, Stefan Berger wrote:
>> +++ b/qapi-schema.json
>> @@ -5140,6 +5140,16 @@
>> { 'command': 'query-tpm-types', 'returns': ['TpmType'] }
>>
>> ##
>> +# @TPMOptions:
>> +#
>> +# Base type for TPM options
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'TPMOptions',
>> + 'data': { } }
>> +
>> +##
>> # @TPMPassthroughOptions:
>> #
>> # Information about the TPM passthrough type
>> @@ -5151,20 +5161,9 @@
>> #
>> # Since: 1.5
>> ##
>> -{ 'struct': 'TPMPassthroughOptions', 'data': { '*path' : 'str',
>> - '*cancel-path' : 'str'} }
>> +{ 'struct': 'TPMPassthroughOptions', 'base': 'TPMOptions',
>> + 'data': { '*path' : 'str', '*cancel-path' : 'str'} }
>
>
> I don't think that you can change it like this anymore since this is
> part of the public interface.
The change is backwards compatible; it doesn't break any existing QMP
usage. It's weird to have an empty base class, though.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/9] tpm-backend: Move realloc_buffer() implementation to base class
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 7/9] tpm-backend: Move realloc_buffer() implementation to base class Amarnath Valluri
@ 2017-04-25 19:01 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 19:01 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> Provide base implementation of realloc_buffer(), so that backend implementations
> can resue.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> backends/tpm.c | 9 ++++++++-
> hw/tpm/tpm_passthrough.c | 12 ------------
> 2 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 3493df6..0da73e6 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -88,8 +88,15 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
> size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
> {
> TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> + if (!k->ops->realloc_buffer) {
> + size_t wanted_size = 4096; /* Linux tpm.c buffer size */
>
> - assert(k->ops->realloc_buffer);
> + if (sb->size != wanted_size) {
> + sb->buffer = g_realloc(sb->buffer, wanted_size);
> + sb->size = wanted_size;
> + }
> + return sb->size;
> + }
>
> return k->ops->realloc_buffer(sb);
> }
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 8e11ed3..1bffb6d 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -258,17 +258,6 @@ static bool tpm_passthrough_get_startup_error(TPMBackend *tb)
> return tpm_pt->had_startup_error;
> }
>
> -static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb)
> -{
> - size_t wanted_size = 4096; /* Linux tpm.c buffer size */
> -
> - if (sb->size != wanted_size) {
> - sb->buffer = g_realloc(sb->buffer, wanted_size);
> - sb->size = wanted_size;
> - }
> - return sb->size;
> -}
> -
> static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
> {
> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> @@ -470,7 +459,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
> .opts = tpm_passthrough_cmdline_opts,
> .desc = tpm_passthrough_create_desc,
> .create = tpm_passthrough_create,
> - .realloc_buffer = tpm_passthrough_realloc_buffer,
> .reset = tpm_passthrough_reset,
> .had_startup_error = tpm_passthrough_get_startup_error,
> .cancel_cmd = tpm_passthrough_cancel_cmd,
If you don't need this realloc_buffer function in your driver, you could
entirely remove it.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/9] tpm-passthrough: move reusable code to utils
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 8/9] tpm-passthrough: move reusable code to utils Amarnath Valluri
@ 2017-04-25 19:09 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 19:09 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> hw/tpm/tpm_passthrough.c | 64 ++++--------------------------------------------
> hw/tpm/tpm_util.c | 25 +++++++++++++++++++
> hw/tpm/tpm_util.h | 4 +++
> 3 files changed, 34 insertions(+), 59 deletions(-)
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 1bffb6d..ca7c8bb 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -68,27 +68,6 @@ typedef struct TPMPassthruState TPMPassthruState;
>
> static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
>
> -static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
> -{
> - int ret, remain;
> -
> - remain = len;
> - while (remain > 0) {
> - ret = write(fd, buf, remain);
> - if (ret < 0) {
> - if (errno != EINTR && errno != EAGAIN) {
> - return -1;
> - }
> - } else if (ret == 0) {
> - break;
> - } else {
> - buf += ret;
> - remain -= ret;
> - }
> - }
> - return len - remain;
> -}
> -
> static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
> {
> int ret;
> @@ -102,45 +81,12 @@ static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
> }
> return ret;
> }
> -
> -static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
> -{
> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)buf;
> -
> - return be32_to_cpu(resp->len);
> -}
> -
> -/*
> - * Write an error message in the given output buffer.
> - */
> -static void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len)
> -{
> - if (out_len >= sizeof(struct tpm_resp_hdr)) {
> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
> -
> - resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
> - resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
> - resp->errcode = cpu_to_be32(TPM_FAIL);
> - }
> -}
> -
> -static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len)
> -{
> - struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
> -
> - if (in_len >= sizeof(*hdr)) {
> - return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
> - }
> -
> - return false;
> -}
> -
> static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> const uint8_t *in, uint32_t in_len,
> uint8_t *out, uint32_t out_len,
> bool *selftest_done)
> {
> - int ret;
> + ssize_t ret;
> bool is_selftest;
> const struct tpm_resp_hdr *hdr;
>
> @@ -148,9 +94,9 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> tpm_pt->tpm_executing = true;
> *selftest_done = false;
>
> - is_selftest = tpm_passthrough_is_selftest(in, in_len);
> + is_selftest = tpm_util_is_selftest(in, in_len);
>
> - ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len);
> + ret = qemu_write_full(tpm_pt->tpm_fd, (const void *)in, (size_t)in_len);
> if (ret != in_len) {
> if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) {
> error_report("tpm_passthrough: error while transmitting data "
> @@ -170,7 +116,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> strerror(errno), errno);
> }
> } else if (ret < sizeof(struct tpm_resp_hdr) ||
> - tpm_passthrough_get_size_from_buffer(out) != ret) {
> + ((struct tpm_resp_hdr *)out)->len != ret) {
Looks like you are missing the be32_to_cpu from above removal.
- return be32_to_cpu(resp->len);
> ret = -1;
> error_report("tpm_passthrough: received invalid response "
> "packet from TPM");
> @@ -183,7 +129,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>
> err_exit:
> if (ret < 0) {
> - tpm_write_fatal_error_response(out, out_len);
> + tpm_util_write_fatal_error_response(out, out_len);
> }
>
> tpm_pt->tpm_executing = false;
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 7b35429..fb929f6 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -24,6 +24,31 @@
> #include "tpm_int.h"
>
> /*
> + * Write an error message in the given output buffer.
> + */
> +void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
> +{
> + if (out_len >= sizeof(struct tpm_resp_hdr)) {
> + struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
> +
> + resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
> + resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
> + resp->errcode = cpu_to_be32(TPM_FAIL);
> + }
> +}
> +
> +bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
> +{
> + struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
> +
> + if (in_len >= sizeof(*hdr)) {
> + return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
> + }
> +
> + return false;
> +}
> +
> +/*
> * A basic test of a TPM device. We expect a well formatted response header
> * (error response is fine) within one second.
> */
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index df76245..2f7c961 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -24,6 +24,10 @@
>
> #include "sysemu/tpm_backend.h"
>
> +void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len);
> +
> +bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len);
> +
> int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
>
> #endif /* TPM_TPM_UTIL_H */
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator Amarnath Valluri
2017-04-07 14:41 ` Daniel P. Berrange
@ 2017-04-25 19:35 ` Stefan Berger
1 sibling, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2017-04-25 19:35 UTC (permalink / raw)
To: Amarnath Valluri, qemu-devel; +Cc: patrick.ohly, marcandre.lureau
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> This change introduces a new TPM backend driver that can communicate with
> swtpm(software TPM emulator) using unix domain socket interface.
>
> Swtpm uses two unix sockets, one for plain TPM commands and responses, and one
> for out-of-band control messages.
I intend to add an option '--locality prepended' to 'swtpm', which I am
using to read the locality a command is supposed to be executing in. I
have a pending driver extension for the Linux vtpm proxy driver that
prepends this byte. Either the same byte or a more elaborate
swtpm-specific header {CMD_WRAPPED_CMD, locality, sizeof tpm-command,
tpm-command} could be used in every command sent from QEMU to pass along
the locality and could be activated via command line using 'swtpm ...
--locality wrapped-command'. That may not change much in regards to
setting the locality via the control channel, though.
Regarding this driver: There's the comment about the control channel and
data channel and them being separated. I still don't see why we need to
merge them or why they couldn't be passed via two fd's.
>
> The swtpm and associated tools can be found here:
> https://github.com/stefanberger/swtpm
>
> Usage:
> # setup TPM state directory
> mkdir /tmp/mytpm
> chown -R tss:root /tmp/mytpm
> /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
>
> # Ask qemu to use TPM emulator with given tpm state directory
> qemu-system-x86_64 \
> [...] \
> -tpmdev emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
> -device tpm-tis,tpmdev=tpm0 \
> [...]
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
> configure | 15 +-
> hmp.c | 21 ++
> hw/tpm/Makefile.objs | 1 +
> hw/tpm/tpm_emulator.c | 927 ++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/tpm/tpm_ioctl.h | 243 +++++++++++++
> qapi-schema.json | 36 +-
> qemu-options.hx | 53 ++-
> tpm.c | 2 +-
> 8 files changed, 1289 insertions(+), 9 deletions(-)
> create mode 100644 hw/tpm/tpm_emulator.c
> create mode 100644 hw/tpm/tpm_ioctl.h
>
> diff --git a/configure b/configure
> index 4901b9a..bef41f3 100755
> --- a/configure
> +++ b/configure
> @@ -3347,10 +3347,15 @@ fi
> ##########################################
> # TPM passthrough is only on x86 Linux
>
> -if test "$targetos" = Linux && test "$cpu" = i386 -o "$cpu" = x86_64; then
> - tpm_passthrough=$tpm
> +if test "$targetos" = Linux; then
> + tpm_emulator=$tpm
> + if test "$cpu" = i386 -o "$cpu" = x86_64; then
> + tpm_passthrough=$tpm
> + else
> + tpm_passthrough=no
> + fi
> else
> - tpm_passthrough=no
> + tpm_emulator=no
> fi
>
> ##########################################
> @@ -5125,6 +5130,7 @@ echo "gcov enabled $gcov"
> echo "TPM support $tpm"
> echo "libssh2 support $libssh2"
> echo "TPM passthrough $tpm_passthrough"
> +echo "TPM emulator $tpm_emulator"
> echo "QOM debugging $qom_cast_debug"
> echo "lzo support $lzo"
> echo "snappy support $snappy"
> @@ -5704,6 +5710,9 @@ if test "$tpm" = "yes"; then
> if test "$tpm_passthrough" = "yes"; then
> echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
> fi
> + if test "$tpm_emulator" = "yes"; then
> + echo "CONFIG_TPM_EMULATOR=y" >> $config_host_mak
> + fi
> fi
>
> echo "TRACE_BACKENDS=$trace_backends" >> $config_host_mak
> diff --git a/hmp.c b/hmp.c
> index 9caf7c8..e7fd426 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -937,6 +937,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> Error *err = NULL;
> unsigned int c = 0;
> TPMPassthroughOptions *tpo;
> + TPMEmulatorOptions *teo;
>
> info_list = qmp_query_tpm(&err);
> if (err) {
> @@ -966,6 +967,26 @@ 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_EMULATOR:
> + teo = (TPMEmulatorOptions *)(ti->options);
> + monitor_printf(mon, ",tmpstatedir=%s", teo->tpmstatedir);
> + monitor_printf(mon, ",spawn=%s", teo->spawn ? "on" : "off");
> + if (teo->has_path) {
> + monitor_printf(mon, ",path=%s", teo->path);
> + }
> + if (teo->has_data_path) {
> + monitor_printf(mon, ",data-path=%s", teo->data_path);
> + }
> + if (teo->has_ctrl_path) {
> + monitor_printf(mon, ",ctrl-path=%s", teo->ctrl_path);
> + }
> + if (teo->has_logfile) {
> + monitor_printf(mon, ",logfile=%s", teo->logfile);
> + }
> + if (teo->has_loglevel) {
> + monitor_printf(mon, ",loglevel=%ld", teo->loglevel);
> + }
> + break;
> default:
> break;
> }
> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
> index 64cecc3..41f0b7a 100644
> --- a/hw/tpm/Makefile.objs
> +++ b/hw/tpm/Makefile.objs
> @@ -1,2 +1,3 @@
> common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
> common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o tpm_util.o
> +common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o tpm_util.o
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> new file mode 100644
> index 0000000..d001ed9
> --- /dev/null
> +++ b/hw/tpm/tpm_emulator.c
> @@ -0,0 +1,927 @@
> +/*
> + * emulator TPM driver
> + *
> + * Copyright (c) 2017 Intel Corporation
> + * Author: Amarnath Valluri <amarnath.valluri@intel.com>
> + *
> + * Copyright (c) 2010 - 2013 IBM Corporation
> + * Authors:
> + * Stefan Berger <stefanb@us.ibm.com>
> + *
> + * Copyright (C) 2011 IAIK, Graz University of Technology
> + * Author: Andreas Niederl
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qemu/sockets.h"
> +#include "io/channel-socket.h"
> +#include "sysemu/tpm_backend.h"
> +#include "tpm_int.h"
> +#include "hw/hw.h"
> +#include "hw/i386/pc.h"
> +#include "tpm_util.h"
> +#include "tpm_ioctl.h"
> +#include "qapi/error.h"
> +
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <stdio.h>
> +
> +#define DEBUG_TPM 0
> +
> +#define DPRINT(fmt, ...) do { \
> + if (DEBUG_TPM) { \
> + fprintf(stderr, fmt, ## __VA_ARGS__); \
> + } \
> +} while (0);
> +
> +#define DPRINTF(fmt, ...) DPRINT("tpm-emulator: "fmt"\n", __VA_ARGS__)
> +
> +#define TYPE_TPM_EMULATOR "tpm-emulator"
> +#define TPM_EMULATOR(obj) \
> + OBJECT_CHECK(TPMEmulator, (obj), TYPE_TPM_EMULATOR)
> +
> +static const TPMDriverOps tpm_emulator_driver;
> +
> +/* data structures */
> +typedef struct TPMEmulator {
> + TPMBackend parent;
> +
> + TPMEmulatorOptions *ops;
> + QIOChannel *data_ioc;
> + QIOChannel *ctrl_ioc;
> + bool op_executing;
> + bool op_canceled;
> + bool child_running;
> + TPMVersion tpm_version;
> + ptm_cap caps; /* capabilities of the TPM */
> + uint8_t cur_locty_number; /* last set locality */
> + QemuMutex state_lock;
> +} TPMEmulator;
> +
> +#define TPM_DEFAULT_EMULATOR "swtpm"
> +#define TPM_DEFAULT_LOGLEVEL 5
> +#define TPM_EMULATOR_PIDFILE "/tmp/qemu-tpm.pid"
> +#define TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(S, cap) (((S)->caps & (cap)) == (cap))
> +#define TPM_EMULATOR_IOCTL_TO_CMD(ioctlnum) \
> + ((ioctlnum >> _IOC_NRSHIFT) & _IOC_NRMASK) + 1
You shouldn't need this part here. You could be using the CMD_-prefixed
commands instead of the ioctl codes.
> +
> +static int tpm_emulator_ctrlcmd(QIOChannel *ioc, unsigned long cmd, void *msg,
> + size_t msg_len_in, size_t msg_len_out)
> +{
> + ssize_t n;
> +
> + uint32_t cmd_no = cpu_to_be32(TPM_EMULATOR_IOCTL_TO_CMD(cmd));
> + struct iovec iov[2] = {
> + { .iov_base = &cmd_no, .iov_len = sizeof(cmd_no), },
> + { .iov_base = msg, .iov_len = msg_len_in, },
> + };
> +
> + n = qio_channel_writev(ioc, iov, 2, NULL);
> + if (n > 0) {
> + if (msg_len_out > 0) {
> + n = qio_channel_read(ioc, (char *)msg, msg_len_out, NULL);
> + /* simulate ioctl return value */
> + if (n > 0) {
> + n = 0;
> + }
> + } else {
> + n = 0;
> + }
> + }
> + return n;
> +}
> +
> +static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_pt,
> + const uint8_t *in, uint32_t in_len,
> + uint8_t *out, uint32_t out_len,
> + bool *selftest_done)
> +{
> + ssize_t ret;
> + bool is_selftest;
> + const struct tpm_resp_hdr *hdr;
> +
> + if (!tpm_pt->child_running) {
> + return -1;
> + }
> +
> + tpm_pt->op_canceled = false;
> + tpm_pt->op_executing = true;
> + *selftest_done = false;
> +
> + is_selftest = tpm_util_is_selftest(in, in_len);
> +
> + ret = qio_channel_write(tpm_pt->data_ioc, (const char *)in, (size_t)in_len,
> + NULL);
> + if (ret != in_len) {
> + if (!tpm_pt->op_canceled || errno != ECANCELED) {
> + error_report("tpm-emulator: error while transmitting data "
> + "to TPM: %s (%i)", strerror(errno), errno);
> + }
> + goto err_exit;
> + }
> +
> + tpm_pt->op_executing = false;
> +
> + ret = qio_channel_read(tpm_pt->data_ioc, (char *)out, (size_t)out_len, NULL);
> + if (ret < 0) {
> + if (!tpm_pt->op_canceled || errno != ECANCELED) {
> + error_report("tpm-emulator: error while reading data from "
> + "TPM: %s (%i)", strerror(errno), errno);
> + }
> + } else if (ret < sizeof(struct tpm_resp_hdr) ||
> + be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
> + ret = -1;
> + error_report("tpm-emulator: received invalid response "
> + "packet from TPM");
> + }
> +
> + if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
> + hdr = (struct tpm_resp_hdr *)out;
> + *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
> + }
> +
> +err_exit:
> + if (ret < 0) {
> + tpm_util_write_fatal_error_response(out, out_len);
> + }
> +
> + tpm_pt->op_executing = false;
> +
> + return ret;
> +}
> +
> +static int tpm_emulator_set_locality(TPMEmulator *tpm_pt,
> + uint8_t locty_number)
> +{
> + ptm_loc loc;
> +
> + if (!tpm_pt->child_running) {
> + return -1;
> + }
> +
> + DPRINTF("%s : locality: 0x%x", __func__, locty_number);
> +
> + if (tpm_pt->cur_locty_number != locty_number) {
> + DPRINTF("setting locality : 0x%x", locty_number);
> + loc.u.req.loc = cpu_to_be32(locty_number);
locality is just a byte!
> + if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_SET_LOCALITY, &loc,
> + sizeof(loc), sizeof(loc)) < 0) {
> + error_report("tpm-emulator: could not set locality : %s",
> + strerror(errno));
> + return -1;
> + }
> + loc.u.resp.tpm_result = be32_to_cpu(loc.u.resp.tpm_result);
> + if (loc.u.resp.tpm_result != 0) {
> + error_report("tpm-emulator: TPM result for set locality : 0x%x",
> + loc.u.resp.tpm_result);
> + return -1;
> + }
> + tpm_pt->cur_locty_number = locty_number;
> + }
> + return 0;
> +}
> +
> +static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd cmd)
> +{
> + TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
> + TPMLocality *locty = NULL;
> + bool selftest_done = false;
> +
> + DPRINTF("processing command type %d", cmd);
> +
> + switch (cmd) {
> + case TPM_BACKEND_CMD_PROCESS_CMD:
> + qemu_mutex_lock(&tpm_pt->state_lock);
> + locty = tb->tpm_state->locty_data;
> + if (tpm_emulator_set_locality(tpm_pt,
> + tb->tpm_state->locty_number) < 0) {
> + tpm_util_write_fatal_error_response(locty->r_buffer.buffer,
> + locty->r_buffer.size);
> + } else {
> + tpm_emulator_unix_tx_bufs(tpm_pt, locty->w_buffer.buffer,
> + locty->w_offset, locty->r_buffer.buffer,
> + locty->r_buffer.size, &selftest_done);
> + }
> + tb->recv_data_callback(tb->tpm_state, tb->tpm_state->locty_number,
> + selftest_done);
> + qemu_mutex_unlock(&tpm_pt->state_lock);
> + break;
> + case TPM_BACKEND_CMD_INIT:
> + case TPM_BACKEND_CMD_END:
> + case TPM_BACKEND_CMD_TPM_RESET:
> + /* nothing to do */
> + break;
> + }
> +}
> +
> +/*
> + * Gracefully shut down the external unixio TPM
> + */
> +static void tpm_emulator_shutdown(TPMEmulator *tpm_pt)
> +{
> + ptm_res res;
> +
> + if (!tpm_pt->child_running) {
> + return;
> + }
> +
> + if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_SHUTDOWN, &res, 0,
> + sizeof(res)) < 0) {
> + error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
> + strerror(errno));
> + } else if (res != 0) {
> + error_report("tpm-emulator: TPM result for sutdown: 0x%x",
> + be32_to_cpu(res));
> + }
> +}
> +
> +static int tpm_emulator_probe_caps(TPMEmulator *tpm_pt)
> +{
> + if (!tpm_pt->child_running) {
> + return -1;
> + }
> +
> + DPRINTF("%s", __func__);
> + if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_GET_CAPABILITY,
> + &tpm_pt->caps, 0, sizeof(tpm_pt->caps)) < 0) {
> + error_report("tpm-emulator: probing failed : %s", strerror(errno));
> + return -1;
> + }
> +
> + tpm_pt->caps = be64_to_cpu(tpm_pt->caps);
> +
> + DPRINTF("capbilities : 0x%lx", tpm_pt->caps);
> +
> + return 0;
> +}
> +
> +static int tpm_emulator_check_caps(TPMEmulator *tpm_pt)
> +{
> + ptm_cap caps = 0;
> + const char *tpm = NULL;
> +
> + /* 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-emulator: TPM version has not been set");
> + return -1;
> + }
> +
> + if (!TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(tpm_pt, caps)) {
> + error_report("tpm-emulator: TPM does not implement minimum set of "
> + "required capabilities for TPM %s (0x%x)", tpm, (int)caps);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int tpm_emulator_init_tpm(TPMEmulator *tpm_pt)
> +{
> + ptm_init init;
> + ptm_res res;
> +
> + if (!tpm_pt->child_running) {
> + return -1;
> + }
> +
> + DPRINTF("%s", __func__);
> + if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_INIT, &init, sizeof(init),
> + sizeof(init)) < 0) {
> + error_report("tpm-emulator: could not send INIT: %s",
> + strerror(errno));
> + return -1;
> + }
> +
> + res = be32_to_cpu(init.u.resp.tpm_result);
> + if (res) {
> + error_report("tpm-emulator: TPM result for PTM_INIT: 0x%x", res);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int tpm_emulator_startup_tpm(TPMBackend *tb)
> +{
> + TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
> +
> + DPRINTF("%s", __func__);
> +
> + tpm_emulator_init_tpm(tpm_pt) ;
> +
> + return 0;
> +}
> +
> +static bool tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
> +{
> + TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
> + ptm_est est;
> +
> + DPRINTF("%s", __func__);
> + if (tpm_emulator_ctrlcmd(tpm_pt->ctrl_ioc, PTM_GET_TPMESTABLISHED, &est, 0,
> + sizeof(est)) < 0) {
> + error_report("tpm-emulator: Could not get the TPM established flag: %s",
> + strerror(errno));
> + return false;
> + }
> + DPRINTF("established flag: %0x", est.u.resp.bit);
> +
> + return (est.u.resp.bit != 0);
> +}
> +
> +static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
> + uint8_t locty)
> +{
> + TPMEmulator *tpm_pt = TPM_EMULATOR(tb);
> + ptm_reset_est reset_est;
> + ptm_res res;
> +
> + /* only a TPM 2.0 will support this */
> + if (tpm_pt->tpm_version == TPM_VERSION_2_0) {
> + reset_est.u.req.loc = cpu_to_be32(tpm_pt->cur_locty_number);
locality is just a byte.
This is as far as I got today.
Stefan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods
2017-04-25 18:27 ` Stefan Berger
@ 2017-05-02 7:09 ` Amarnath Valluri
0 siblings, 0 replies; 34+ messages in thread
From: Amarnath Valluri @ 2017-05-02 7:09 UTC (permalink / raw)
To: Stefan Berger, qemu-devel; +Cc: patrick.ohly, marcandre.lureau, berrange
On 04/25/2017 09:27 PM, Stefan Berger wrote:
> On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
>> Initialize and free TPMBackend data members in it's own
>> instance_init() and
>> instance_finalize methods.
>>
>> Took the opportunity to fix object cleanup in
>> tpm_backend_{create,destroy}
>> methods
>>
>> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
>> ---
>> backends/tpm.c | 8 ++++++--
>> hw/tpm/tpm_passthrough.c | 18 +++++++++++-------
>> 2 files changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/backends/tpm.c b/backends/tpm.c
>> index 0ec0c67..99d2b2b 100644
>> --- a/backends/tpm.c
>> +++ b/backends/tpm.c
>> @@ -57,7 +57,7 @@ void tpm_backend_destroy(TPMBackend *s)
>>
>> k->ops->destroy(s);
>>
>> - tpm_backend_thread_end(s);
>> + object_unref(OBJECT(s));
>> }
>>
>> int tpm_backend_init(TPMBackend *s, TPMState *state,
>> @@ -182,11 +182,13 @@ static void tpm_backend_prop_set_opened(Object
>> *obj, bool value, Error **errp)
>>
>> static void tpm_backend_instance_init(Object *obj)
>> {
>> + TPMBackend *s = TPM_BACKEND(obj);
>> +
>> object_property_add_bool(obj, "opened",
>> tpm_backend_prop_get_opened,
>> tpm_backend_prop_set_opened,
>> NULL);
>> -
>> + s->fe_model = -1;
>> }
>>
>> static void tpm_backend_instance_finalize(Object *obj)
>> @@ -194,6 +196,8 @@ static void tpm_backend_instance_finalize(Object
>> *obj)
>> TPMBackend *s = TPM_BACKEND(obj);
>>
>> g_free(s->id);
>> + g_free(s->path);
>> + g_free(s->cancel_path);
>> tpm_backend_thread_end(s);
>> }
>>
>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>> index f50d9cf..65831b5 100644
>> --- a/hw/tpm/tpm_passthrough.c
>> +++ b/hw/tpm/tpm_passthrough.c
>> @@ -417,8 +417,6 @@ static TPMBackend
>> *tpm_passthrough_create(QemuOpts *opts, const char *id)
>> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>>
>> tb->id = g_strdup(id);
>> - /* let frontend set the fe_model to proper value */
>> - tb->fe_model = -1;
>>
>> if (tpm_passthrough_handle_device_opts(opts, tb)) {
>> goto err_exit;
>> @@ -432,7 +430,7 @@ static TPMBackend
>> *tpm_passthrough_create(QemuOpts *opts, const char *id)
>> return tb;
>>
>> err_exit:
>> - g_free(tb->id);
>> + object_unref(obj);
>>
>> return NULL;
>> }
>> @@ -445,10 +443,6 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
>>
>> qemu_close(tpm_pt->tpm_fd);
>> qemu_close(tpm_pt->cancel_fd);
>> -
>> - g_free(tb->id);
>> - g_free(tb->path);
>> - g_free(tb->cancel_path);
>
>
> Looks like here you are correcting the error from the previous patch...
>
>
>
>> g_free(tpm_pt->tpm_dev);
>> }
>>
>> @@ -486,10 +480,20 @@ static const TPMDriverOps
>> tpm_passthrough_driver = {
>>
>> static void tpm_passthrough_inst_init(Object *obj)
>> {
>> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
>> +
>> + tpm_pt->tpm_fd = -1;
>> + tpm_pt->cancel_fd = -1;
>> }
>>
>> static void tpm_passthrough_inst_finalize(Object *obj)
>> {
>> + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
>> +
>> + tpm_passthrough_cancel_cmd(TPM_BACKEND(obj));
>> +
>> + qemu_close(tpm_pt->tpm_fd);
>> + qemu_close(tpm_pt->cancel_fd);
>> }
>
> With these I am wondering whether they should not have been moved here
> from tpm_passthrough_destroy()? The appear there as well.
Yes, I agree with your point. Anyways, 'destroy()' method itself
removed from interface in later commit(4c0c5e1) in the series.
- Amarnath
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo
2017-04-25 18:59 ` Eric Blake
@ 2017-05-02 7:17 ` Amarnath Valluri
0 siblings, 0 replies; 34+ messages in thread
From: Amarnath Valluri @ 2017-05-02 7:17 UTC (permalink / raw)
To: Eric Blake, Stefan Berger, qemu-devel; +Cc: patrick.ohly, marcandre.lureau
On 04/25/2017 09:59 PM, Eric Blake wrote:
> On 04/25/2017 01:51 PM, Stefan Berger wrote:
>
>>> +++ b/qapi-schema.json
>>> @@ -5140,6 +5140,16 @@
>>> { 'command': 'query-tpm-types', 'returns': ['TpmType'] }
>>>
>>> ##
>>> +# @TPMOptions:
>>> +#
>>> +# Base type for TPM options
>>> +#
>>> +# Since: 2.10
>>> +##
>>> +{ 'struct': 'TPMOptions',
>>> + 'data': { } }
>>> +
>>> +##
>>> # @TPMPassthroughOptions:
>>> #
>>> # Information about the TPM passthrough type
>>> @@ -5151,20 +5161,9 @@
>>> #
>>> # Since: 1.5
>>> ##
>>> -{ 'struct': 'TPMPassthroughOptions', 'data': { '*path' : 'str',
>>> - '*cancel-path' : 'str'} }
>>> +{ 'struct': 'TPMPassthroughOptions', 'base': 'TPMOptions',
>>> + 'data': { '*path' : 'str', '*cancel-path' : 'str'} }
>>
>> I don't think that you can change it like this anymore since this is
>> part of the public interface.
> The change is backwards compatible; it doesn't break any existing QMP
> usage. It's weird to have an empty base class, though.
This empty base class(TPMOptions) makes sense when its shared by more
than one(Passthrough and Emulator) bakcends.
- Amarnath
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
2017-04-25 18:59 ` Stefan Berger
@ 2017-05-02 7:42 ` Amarnath Valluri
0 siblings, 0 replies; 34+ messages in thread
From: Amarnath Valluri @ 2017-05-02 7:42 UTC (permalink / raw)
To: Stefan Berger, qemu-devel; +Cc: patrick.ohly, marcandre.lureau, berrange
On 04/25/2017 09:59 PM, Stefan Berger wrote:
> On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
>> As TPMBackend is a Qemu Object, we can use object_unref() inplace of
>> tpm_backend_destroy() to free the backend object, hence removed
>> destroy() from
>> TPMDriverOps interface.
>>
>> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
>> ---
>> backends/tpm.c | 11 -----------
>> hw/tpm/tpm_passthrough.c | 14 --------------
>> include/sysemu/tpm_backend.h | 7 -------
>> tpm.c | 2 +-
>> 4 files changed, 1 insertion(+), 33 deletions(-)
>>
>> diff --git a/backends/tpm.c b/backends/tpm.c
>> index c96f462..3493df6 100644
>> --- a/backends/tpm.c
>> +++ b/backends/tpm.c
>> @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s)
>> return k->ops->desc ? k->ops->desc() : "";
>> }
>>
>> -void tpm_backend_destroy(TPMBackend *s)
>> -{
>> - TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>> -
>> - if (k->ops->destroy) {
>> - k->ops->destroy(s);
>> - }
>> -
>> - object_unref(OBJECT(s));
>> -}
>> -
>> int tpm_backend_init(TPMBackend *s, TPMState *state,
>> TPMRecvDataCB *datacb)
>> {
>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>> index 71bdf25..8e11ed3 100644
>> --- a/hw/tpm/tpm_passthrough.c
>> +++ b/hw/tpm/tpm_passthrough.c
>> @@ -428,19 +428,6 @@ err_exit:
>> return NULL;
>> }
>>
>> -static void tpm_passthrough_destroy(TPMBackend *tb)
>> -{
>> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>> -
>> - tpm_passthrough_cancel_cmd(tb);
>> -
>> - qemu_close(tpm_pt->tpm_fd);
>> - qemu_close(tpm_pt->cancel_fd);
>> - g_free(tpm_pt->tpm_dev);
>
> This g_free should be in the finalizer as well, right? It isn't. So we
> will leak some memory.
Yes, I will fix this.
>
>
>> -
>> - qapi_free_TPMPassthroughOptions(tpm_pt->ops);
>> -}
>> -
>> static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
>> {
>> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>> @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
>> .opts = tpm_passthrough_cmdline_opts,
>> .desc = tpm_passthrough_create_desc,
>> .create = tpm_passthrough_create,
>> - .destroy = tpm_passthrough_destroy,
>> .realloc_buffer = tpm_passthrough_realloc_buffer,
>> .reset = tpm_passthrough_reset,
>> .had_startup_error = tpm_passthrough_get_startup_error,
>> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
>> index 7f4d621..8f8f133 100644
>> --- a/include/sysemu/tpm_backend.h
>> +++ b/include/sysemu/tpm_backend.h
>> @@ -77,7 +77,6 @@ struct TPMDriverOps {
>> const char *(*desc)(void);
>>
>> TPMBackend *(*create)(QemuOpts *opts, const char *id);
>> - void (*destroy)(TPMBackend *t);
>>
>> /* initialize the backend */
>> int (*init)(TPMBackend *t);
>> @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
>> const char *tpm_backend_get_desc(TPMBackend *s);
>>
>> /**
>> - * tpm_backend_destroy:
>> - * @s: the backend to destroy
>> - */
>> -void tpm_backend_destroy(TPMBackend *s);
>> -
>> -/**
>> * tpm_backend_init:
>> * @s: the backend to initialized
>> * @state: TPMState
>> diff --git a/tpm.c b/tpm.c
>> index 1b6b550..43d980e 100644
>> --- a/tpm.c
>> +++ b/tpm.c
>> @@ -197,7 +197,7 @@ void tpm_cleanup(void)
>>
>> QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
>> QLIST_REMOVE(drv, list);
>> - tpm_backend_destroy(drv);
>> + object_unref(OBJECT(drv));
>> }
>> }
>>
>
> I *think* you should apply this patch after introducing the
> tpm_passthrough_inst_finalize or merge it into that one.
>
Ok, i even think it makes sense to meld this commit with
baeb9df(tpm-backend: Initialize and free data members in it's own methods).
- Amarnath
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2017-05-02 7:40 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 14:30 [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 1/9] tpm-backend: Remove unneeded member variable from backend class Amarnath Valluri
2017-04-25 18:19 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 2/9] tpm-backend: Move thread handling inside TPMBackend Amarnath Valluri
2017-04-25 18:21 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods Amarnath Valluri
2017-04-25 18:27 ` Stefan Berger
2017-05-02 7:09 ` Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 4/9] tpm-backend: Made few interface methods optional Amarnath Valluri
2017-04-25 18:29 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 5/9] tmp backend: Add new api to read backend TpmInfo Amarnath Valluri
2017-04-25 18:51 ` Stefan Berger
2017-04-25 18:59 ` Eric Blake
2017-05-02 7:17 ` Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface Amarnath Valluri
2017-04-25 18:59 ` Stefan Berger
2017-05-02 7:42 ` Amarnath Valluri
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 7/9] tpm-backend: Move realloc_buffer() implementation to base class Amarnath Valluri
2017-04-25 19:01 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 8/9] tpm-passthrough: move reusable code to utils Amarnath Valluri
2017-04-25 19:09 ` Stefan Berger
2017-04-07 14:30 ` [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator Amarnath Valluri
2017-04-07 14:41 ` Daniel P. Berrange
2017-04-07 15:11 ` Marc-André Lureau
2017-04-10 7:34 ` Amarnath Valluri
2017-04-10 9:54 ` Marc-André Lureau
2017-04-10 10:07 ` Patrick Ohly
2017-04-10 16:14 ` Stefan Berger
2017-04-10 21:11 ` Stefan Berger
2017-04-10 7:08 ` Amarnath Valluri
2017-04-10 8:31 ` Daniel P. Berrange
2017-04-10 16:15 ` Stefan Berger
2017-04-25 19:35 ` Stefan Berger
2017-04-12 23:52 ` [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM no-reply
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.