All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] tpm: add mssim backend
@ 2022-12-19 13:13 James Bottomley
  2022-12-19 13:13 ` [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format James Bottomley
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: James Bottomley @ 2022-12-19 13:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Berger, Daniel P . Berrangé, Markus Armbruster

From: James Bottomley <James.Bottomley@HansenPartnership.com>

The requested feedback was to convert the tpmdev handler to being json
based, which requires rethreading all the backends.  The good news is
this reduced quite a bit of code (especially as I converted it to
error_fatal handling as well, which removes the return status
threading).  The bad news is I can't test any of the conversions.
swtpm still isn't building on opensuse and, apparently, passthrough
doesn't like my native TPM because it doesn't allow cancellation.

v3 pulls out more unneeded code in the visitor conversion, makes
migration work on external state preservation of the simulator and
adds documentation

James

---

James Bottomley (2):
  tpm: convert tpmdev options processing to new visitor format
  tpm: add backend for mssim

 MAINTAINERS                    |   6 +
 backends/tpm/Kconfig           |   5 +
 backends/tpm/meson.build       |   1 +
 backends/tpm/tpm_emulator.c    |  35 ++---
 backends/tpm/tpm_mssim.c       | 264 +++++++++++++++++++++++++++++++++
 backends/tpm/tpm_mssim.h       |  43 ++++++
 backends/tpm/tpm_passthrough.c |  37 ++---
 docs/specs/tpm.rst             |  35 +++++
 include/sysemu/tpm.h           |   4 +-
 include/sysemu/tpm_backend.h   |   2 +-
 monitor/hmp-cmds.c             |  11 +-
 qapi/tpm.json                  |  37 ++---
 softmmu/tpm.c                  |  90 +++++------
 softmmu/vl.c                   |  19 +--
 14 files changed, 449 insertions(+), 140 deletions(-)
 create mode 100644 backends/tpm/tpm_mssim.c
 create mode 100644 backends/tpm/tpm_mssim.h

-- 
2.35.3



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

* [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format
  2022-12-19 13:13 [PATCH v3 0/2] tpm: add mssim backend James Bottomley
@ 2022-12-19 13:13 ` James Bottomley
  2022-12-21 16:32   ` Daniel P. Berrangé
  2022-12-19 13:13 ` [PATCH v3 2/2] tpm: add backend for mssim James Bottomley
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: James Bottomley @ 2022-12-19 13:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Berger, Daniel P . Berrangé, Markus Armbruster

From: James Bottomley <James.Bottomley@HansenPartnership.com>

Instead of processing the tpmdev options using the old qemu options,
convert to the new visitor format which also allows the passing of
json on the command line.

Signed-off-by: James Bottomley <jejb@linux.ibm.com>
---
 backends/tpm/tpm_emulator.c    | 35 ++++++-------
 backends/tpm/tpm_passthrough.c | 37 +++++---------
 include/sysemu/tpm.h           |  4 +-
 include/sysemu/tpm_backend.h   |  2 +-
 monitor/hmp-cmds.c             |  4 +-
 qapi/tpm.json                  | 26 ++--------
 softmmu/tpm.c                  | 90 ++++++++++++++--------------------
 softmmu/vl.c                   | 19 +------
 8 files changed, 73 insertions(+), 144 deletions(-)

diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 49cc3d749d..82988a2986 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -69,7 +69,7 @@ typedef struct TPMBlobBuffers {
 struct TPMEmulator {
     TPMBackend parent;
 
-    TPMEmulatorOptions *options;
+    TpmTypeOptions *options;
     CharBackend ctrl_chr;
     QIOChannel *data_ioc;
     TPMVersion tpm_version;
@@ -584,33 +584,28 @@ err_exit:
     return -1;
 }
 
-static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
+static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, TpmTypeOptions *opts)
 {
-    const char *value;
     Error *err = NULL;
     Chardev *dev;
 
-    value = qemu_opt_get(opts, "chardev");
-    if (!value) {
-        error_report("tpm-emulator: parameter 'chardev' is missing");
-        goto err;
-    }
+    tpm_emu->options = opts;
+    tpm_emu->data_ioc = NULL;
 
-    dev = qemu_chr_find(value);
+    dev = qemu_chr_find(opts->u.emulator.chardev);
     if (!dev) {
-        error_report("tpm-emulator: tpm chardev '%s' not found", value);
+        error_report("tpm-emulator: tpm chardev '%s' not found",
+                opts->u.emulator.chardev);
         goto err;
     }
 
     if (!qemu_chr_fe_init(&tpm_emu->ctrl_chr, dev, &err)) {
         error_prepend(&err, "tpm-emulator: No valid chardev found at '%s':",
-                      value);
+                      opts->u.emulator.chardev);
         error_report_err(err);
         goto err;
     }
 
-    tpm_emu->options->chardev = g_strdup(value);
-
     if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
         goto err;
     }
@@ -621,7 +616,7 @@ static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
     if (tpm_util_test_tpmdev(QIO_CHANNEL_SOCKET(tpm_emu->data_ioc)->fd,
                              &tpm_emu->tpm_version)) {
         error_report("'%s' is not emulating TPM device. Error: %s",
-                      tpm_emu->options->chardev, strerror(errno));
+                      tpm_emu->options->u.emulator.chardev, strerror(errno));
         goto err;
     }
 
@@ -649,7 +644,7 @@ err:
     return -1;
 }
 
-static TPMBackend *tpm_emulator_create(QemuOpts *opts)
+static TPMBackend *tpm_emulator_create(TpmTypeOptions *opts)
 {
     TPMBackend *tb = TPM_BACKEND(object_new(TYPE_TPM_EMULATOR));
 
@@ -664,10 +659,9 @@ static TPMBackend *tpm_emulator_create(QemuOpts *opts)
 static TpmTypeOptions *tpm_emulator_get_tpm_options(TPMBackend *tb)
 {
     TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
-    TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
+    TpmTypeOptions *options;
 
-    options->type = TPM_TYPE_EMULATOR;
-    options->u.emulator.data = QAPI_CLONE(TPMEmulatorOptions, tpm_emu->options);
+    options = QAPI_CLONE(TpmTypeOptions, tpm_emu->options);
 
     return options;
 }
@@ -972,7 +966,6 @@ static void tpm_emulator_inst_init(Object *obj)
 
     trace_tpm_emulator_inst_init();
 
-    tpm_emu->options = g_new0(TPMEmulatorOptions, 1);
     tpm_emu->cur_locty_number = ~0;
     qemu_mutex_init(&tpm_emu->mutex);
     tpm_emu->vmstate =
@@ -990,7 +983,7 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
 {
     ptm_res res;
 
-    if (!tpm_emu->options->chardev) {
+    if (!tpm_emu->data_ioc) {
         /* was never properly initialized */
         return;
     }
@@ -1015,7 +1008,7 @@ static void tpm_emulator_inst_finalize(Object *obj)
 
     qemu_chr_fe_deinit(&tpm_emu->ctrl_chr, false);
 
-    qapi_free_TPMEmulatorOptions(tpm_emu->options);
+    qapi_free_TpmTypeOptions(tpm_emu->options);
 
     if (tpm_emu->migration_blocker) {
         migrate_del_blocker(tpm_emu->migration_blocker);
diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
index 5a2f74db1b..2ce39b2167 100644
--- a/backends/tpm/tpm_passthrough.c
+++ b/backends/tpm/tpm_passthrough.c
@@ -41,7 +41,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(TPMPassthruState, TPM_PASSTHROUGH)
 struct TPMPassthruState {
     TPMBackend parent;
 
-    TPMPassthroughOptions *options;
+    TpmTypeOptions *options;
     const char *tpm_dev;
     int tpm_fd;
     bool tpm_executing;
@@ -214,8 +214,8 @@ static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
     char *dev;
     char path[PATH_MAX];
 
-    if (tpm_pt->options->cancel_path) {
-        fd = qemu_open_old(tpm_pt->options->cancel_path, O_WRONLY);
+    if (tpm_pt->options->u.passthrough.cancel_path) {
+        fd = qemu_open_old(tpm_pt->options->u.passthrough.cancel_path, O_WRONLY);
         if (fd < 0) {
             error_report("tpm_passthrough: Could not open TPM cancel path: %s",
                          strerror(errno));
@@ -245,30 +245,18 @@ static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
     if (fd < 0) {
         error_report("tpm_passthrough: Could not guess TPM cancel path");
     } else {
-        tpm_pt->options->cancel_path = g_strdup(path);
+        tpm_pt->options->u.passthrough.cancel_path = g_strdup(path);
     }
 
     return fd;
 }
 
 static int
-tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, QemuOpts *opts)
+tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, TpmTypeOptions *opts)
 {
-    const char *value;
+    tpm_pt->options = opts;
 
-    value = qemu_opt_get(opts, "cancel-path");
-    if (value) {
-        tpm_pt->options->cancel_path = g_strdup(value);
-        tpm_pt->options->has_cancel_path = true;
-    }
-
-    value = qemu_opt_get(opts, "path");
-    if (value) {
-        tpm_pt->options->has_path = true;
-        tpm_pt->options->path = g_strdup(value);
-    }
-
-    tpm_pt->tpm_dev = value ? value : TPM_PASSTHROUGH_DEFAULT_DEVICE;
+    tpm_pt->tpm_dev = opts->u.passthrough.has_path ? opts->u.passthrough.path : TPM_PASSTHROUGH_DEFAULT_DEVICE;
     tpm_pt->tpm_fd = qemu_open_old(tpm_pt->tpm_dev, O_RDWR);
     if (tpm_pt->tpm_fd < 0) {
         error_report("Cannot access TPM device using '%s': %s",
@@ -290,11 +278,11 @@ tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, QemuOpts *opts)
     return 0;
 }
 
-static TPMBackend *tpm_passthrough_create(QemuOpts *opts)
+static TPMBackend *tpm_passthrough_create(TpmTypeOptions *tto)
 {
     Object *obj = object_new(TYPE_TPM_PASSTHROUGH);
 
-    if (tpm_passthrough_handle_device_opts(TPM_PASSTHROUGH(obj), opts)) {
+    if (tpm_passthrough_handle_device_opts(TPM_PASSTHROUGH(obj), tto)) {
         object_unref(obj);
         return NULL;
     }
@@ -320,9 +308,7 @@ static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
 {
     TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
 
-    options->type = TPM_TYPE_PASSTHROUGH;
-    options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions,
-                                             TPM_PASSTHROUGH(tb)->options);
+    options = QAPI_CLONE(TpmTypeOptions, TPM_PASSTHROUGH(tb)->options);
 
     return options;
 }
@@ -346,7 +332,6 @@ static void tpm_passthrough_inst_init(Object *obj)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
 
-    tpm_pt->options = g_new0(TPMPassthroughOptions, 1);
     tpm_pt->tpm_fd = -1;
     tpm_pt->cancel_fd = -1;
 }
@@ -363,7 +348,7 @@ static void tpm_passthrough_inst_finalize(Object *obj)
     if (tpm_pt->cancel_fd >= 0) {
         qemu_close(tpm_pt->cancel_fd);
     }
-    qapi_free_TPMPassthroughOptions(tpm_pt->options);
+    qapi_free_TpmTypeOptions(tpm_pt->options);
 }
 
 static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index fb40e30ff6..d00a833a21 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -17,8 +17,8 @@
 
 #ifdef CONFIG_TPM
 
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
-int tpm_init(void);
+void tpm_config_parse(const char *optarg);
+void tpm_init(void);
 void tpm_cleanup(void);
 
 typedef enum TPMVersion {
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 8fd3269c11..bcef275688 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -57,7 +57,7 @@ struct TPMBackendClass {
     /* get a descriptive text of the backend to display to the user */
     const char *desc;
 
-    TPMBackend *(*create)(QemuOpts *opts);
+    TPMBackend *(*create)(TpmTypeOptions *tto);
 
     /* start up the TPM on the backend - optional */
     int (*startup_tpm)(TPMBackend *t, size_t buffersize);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 01b789a79e..e99447ad68 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -863,7 +863,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
 
         switch (ti->options->type) {
         case TPM_TYPE_PASSTHROUGH:
-            tpo = ti->options->u.passthrough.data;
+            tpo = &ti->options->u.passthrough;
             monitor_printf(mon, "%s%s%s%s",
                            tpo->has_path ? ",path=" : "",
                            tpo->has_path ? tpo->path : "",
@@ -871,7 +871,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
                            tpo->has_cancel_path ? tpo->cancel_path : "");
             break;
         case TPM_TYPE_EMULATOR:
-            teo = ti->options->u.emulator.data;
+            teo = &ti->options->u.emulator;
             monitor_printf(mon, ",chardev=%s", teo->chardev);
             break;
         case TPM_TYPE__MAX:
diff --git a/qapi/tpm.json b/qapi/tpm.json
index 4e2ea9756a..d8cbd5ea0e 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -99,39 +99,23 @@
 { 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
   'if': 'CONFIG_TPM' }
 
-##
-# @TPMPassthroughOptionsWrapper:
-#
-# Since: 1.5
-##
-{ 'struct': 'TPMPassthroughOptionsWrapper',
-  'data': { 'data': 'TPMPassthroughOptions' },
-  'if': 'CONFIG_TPM' }
-
-##
-# @TPMEmulatorOptionsWrapper:
-#
-# Since: 2.11
-##
-{ 'struct': 'TPMEmulatorOptionsWrapper',
-  'data': { 'data': 'TPMEmulatorOptions' },
-  'if': 'CONFIG_TPM' }
-
 ##
 # @TpmTypeOptions:
 #
 # A union referencing different TPM backend types' configuration options
 #
+# @id: identifier of the backend
 # @type: - 'passthrough' The configuration options for the TPM passthrough type
 #        - 'emulator' The configuration options for TPM emulator backend type
 #
 # Since: 1.5
 ##
 { 'union': 'TpmTypeOptions',
-  'base': { 'type': 'TpmType' },
+  'base': { 'type': 'TpmType',
+            'id': 'str' },
   'discriminator': 'type',
-  'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
-            'emulator': 'TPMEmulatorOptionsWrapper' },
+  'data': { 'passthrough' : 'TPMPassthroughOptions',
+            'emulator': 'TPMEmulatorOptions' },
   'if': 'CONFIG_TPM' }
 
 ##
diff --git a/softmmu/tpm.c b/softmmu/tpm.c
index 578563f05a..e7417df4ce 100644
--- a/softmmu/tpm.c
+++ b/softmmu/tpm.c
@@ -17,14 +17,26 @@
 #include "qapi/error.h"
 #include "qapi/qapi-commands-tpm.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-tpm.h"
 #include "sysemu/tpm_backend.h"
 #include "sysemu/tpm.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/help_option.h"
 
 static QLIST_HEAD(, TPMBackend) tpm_backends =
     QLIST_HEAD_INITIALIZER(tpm_backends);
 
+typedef struct TpmTypeOptionsQueueEntry {
+        TpmTypeOptions *tto;
+        QSIMPLEQ_ENTRY(TpmTypeOptionsQueueEntry) entry;
+} TpmTypeOptionsQueueEntry;
+
+typedef QSIMPLEQ_HEAD(, TpmTypeOptionsQueueEntry) TpmTypeOptionsQueue;
+
+static TpmTypeOptionsQueue tto_queue = QSIMPLEQ_HEAD_INITIALIZER(tto_queue);
+
 static const TPMBackendClass *
 tpm_be_find_by_type(enum TpmType type)
 {
@@ -84,63 +96,31 @@ TPMBackend *qemu_find_tpm_be(const char *id)
     return NULL;
 }
 
-static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
+static void tpm_init_tpmdev(TpmTypeOptions *tto)
 {
-    /*
-     * Use of error_report() in a function with an Error ** parameter
-     * is suspicious.  It is okay here.  The parameter only exists to
-     * make the function usable with qemu_opts_foreach().  It is not
-     * actually used.
-     */
-    const char *value;
-    const char *id;
     const TPMBackendClass *be;
     TPMBackend *drv;
-    Error *local_err = NULL;
-    int i;
 
     if (!QLIST_EMPTY(&tpm_backends)) {
         error_report("Only one TPM is allowed.");
-        return 1;
+        exit(1);
     }
 
-    id = qemu_opts_id(opts);
-    if (id == NULL) {
-        error_report(QERR_MISSING_PARAMETER, "id");
-        return 1;
-    }
-
-    value = qemu_opt_get(opts, "type");
-    if (!value) {
-        error_report(QERR_MISSING_PARAMETER, "type");
-        tpm_display_backend_drivers();
-        return 1;
-    }
-
-    i = qapi_enum_parse(&TpmType_lookup, value, -1, NULL);
-    be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
+    be = tto->type >= 0 ? tpm_be_find_by_type(tto->type) : NULL;
     if (be == NULL) {
         error_report(QERR_INVALID_PARAMETER_VALUE,
                      "type", "a TPM backend type");
         tpm_display_backend_drivers();
-        return 1;
-    }
-
-    /* validate backend specific opts */
-    if (!qemu_opts_validate(opts, be->opts, &local_err)) {
-        error_report_err(local_err);
-        return 1;
+        exit(1);
     }
 
-    drv = be->create(opts);
+    drv = be->create(tto);
     if (!drv) {
-        return 1;
+        exit(1);
     }
 
-    drv->id = g_strdup(id);
+    drv->id = g_strdup(tto->id);
     QLIST_INSERT_HEAD(&tpm_backends, drv, list);
-
-    return 0;
 }
 
 /*
@@ -161,33 +141,35 @@ void tpm_cleanup(void)
  * Initialize the TPM. Process the tpmdev command line options describing the
  * TPM backend.
  */
-int tpm_init(void)
+void tpm_init(void)
 {
-    if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
-                          tpm_init_tpmdev, NULL, NULL)) {
-        return -1;
-    }
+    while (!QSIMPLEQ_EMPTY(&tto_queue)) {
+        TpmTypeOptionsQueueEntry *ttoqe = QSIMPLEQ_FIRST(&tto_queue);
 
-    return 0;
+        QSIMPLEQ_REMOVE_HEAD(&tto_queue, entry);
+        tpm_init_tpmdev(ttoqe->tto);
+        g_free(ttoqe);
+    }
 }
 
 /*
  * Parse the TPM configuration options.
  * To display all available TPM backends the user may use '-tpmdev help'
  */
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+void tpm_config_parse(const char *optarg)
 {
-    QemuOpts *opts;
+    Visitor *v;
+    TpmTypeOptionsQueueEntry *toqe;
 
-    if (!strcmp(optarg, "help")) {
+    if (is_help_option(optarg)) {
         tpm_display_backend_drivers();
-        return -1;
-    }
-    opts = qemu_opts_parse_noisily(opts_list, optarg, true);
-    if (!opts) {
-        return -1;
+        return;
     }
-    return 0;
+    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
+    toqe = g_new(TpmTypeOptionsQueueEntry, 1);
+    visit_type_TpmTypeOptions(v, NULL, &toqe->tto, &error_fatal);
+    visit_free(v);
+    QSIMPLEQ_INSERT_TAIL(&tto_queue, toqe, entry);
 }
 
 /*
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5115221efe..980f998c35 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -328,16 +328,6 @@ static QemuOptsList qemu_object_opts = {
     },
 };
 
-static QemuOptsList qemu_tpmdev_opts = {
-    .name = "tpmdev",
-    .implied_opt_name = "type",
-    .head = QTAILQ_HEAD_INITIALIZER(qemu_tpmdev_opts.head),
-    .desc = {
-        /* options are defined in the TPM backends */
-        { /* end of list */ }
-    },
-};
-
 static QemuOptsList qemu_overcommit_opts = {
     .name = "overcommit",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_overcommit_opts.head),
@@ -1934,9 +1924,7 @@ static void qemu_create_late_backends(void)
 
     object_option_foreach_add(object_create_late);
 
-    if (tpm_init() < 0) {
-        exit(1);
-    }
+    tpm_init();
 
     qemu_opts_foreach(qemu_find_opts("mon"),
                       mon_init_func, NULL, &error_fatal);
@@ -2658,7 +2646,6 @@ void qemu_init(int argc, char **argv)
     qemu_add_opts(&qemu_boot_opts);
     qemu_add_opts(&qemu_add_fd_opts);
     qemu_add_opts(&qemu_object_opts);
-    qemu_add_opts(&qemu_tpmdev_opts);
     qemu_add_opts(&qemu_overcommit_opts);
     qemu_add_opts(&qemu_msg_opts);
     qemu_add_opts(&qemu_name_opts);
@@ -2906,9 +2893,7 @@ void qemu_init(int argc, char **argv)
                 break;
 #ifdef CONFIG_TPM
             case QEMU_OPTION_tpmdev:
-                if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) {
-                    exit(1);
-                }
+                tpm_config_parse(optarg);
                 break;
 #endif
             case QEMU_OPTION_mempath:
-- 
2.35.3



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

* [PATCH v3 2/2] tpm: add backend for mssim
  2022-12-19 13:13 [PATCH v3 0/2] tpm: add mssim backend James Bottomley
  2022-12-19 13:13 ` [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format James Bottomley
@ 2022-12-19 13:13 ` James Bottomley
  2022-12-19 13:51 ` [PATCH v3 0/2] tpm: add mssim backend Stefan Berger
  2022-12-19 15:16 ` Stefan Berger
  3 siblings, 0 replies; 11+ messages in thread
From: James Bottomley @ 2022-12-19 13:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Berger, Daniel P . Berrangé, Markus Armbruster

From: James Bottomley <James.Bottomley@HansenPartnership.com>

The Microsoft Simulator (mssim) is the reference emulation platform
for the TCG TPM 2.0 specification.

https://github.com/Microsoft/ms-tpm-20-ref.git

It exports a fairly simple network socket based protocol on two
sockets, one for command (default 2321) and one for control (default
2322).  This patch adds a simple backend that can speak the mssim
protocol over the network.  It also allows the two sockets to be
specified on the command line.  The benefits are twofold: firstly it
gives us a backend that actually speaks a standard TPM emulation
protocol instead of the linux specific TPM driver format of the
current emulated TPM backend and secondly, using the microsoft
protocol, the end point of the emulator can be anywhere on the
network, facilitating the cloud use case where a central TPM service
can be used over a control network.

The implementation does basic control commands like power off/on, but
doesn't implement cancellation or startup.  The former because
cancellation is pretty much useless on a fast operating TPM emulator
and the latter because this emulator is designed to be used with OVMF
which itself does TPM startup and I wanted to validate that.

To run this, simply download an emulator based on the MS specification
(package ibmswtpm2 on openSUSE) and run it, then add these two lines
to the qemu command and it will use the emulator.

    -tpmdev mssim,id=tpm0 \
    -device tpm-crb,tpmdev=tpm0 \

to use a remote emulator replace the first line with

    -tpmdev "{'type':'mssim','id':'tpm0','command':{'type':inet,'host':'remote','port':'2321'}}"

tpm-tis also works as the backend.

Signed-off-by: James Bottomley <jejb@linux.ibm.com>

---

v2: convert to SocketAddr json and use qio_channel_socket_connect_sync()
v3: gate control power off by migration state keep control socket disconnected
    to test outside influence and add docs.
---
 MAINTAINERS              |   6 +
 backends/tpm/Kconfig     |   5 +
 backends/tpm/meson.build |   1 +
 backends/tpm/tpm_mssim.c | 264 +++++++++++++++++++++++++++++++++++++++
 backends/tpm/tpm_mssim.h |  43 +++++++
 docs/specs/tpm.rst       |  35 ++++++
 monitor/hmp-cmds.c       |   7 ++
 qapi/tpm.json            |  25 +++-
 8 files changed, 383 insertions(+), 3 deletions(-)
 create mode 100644 backends/tpm/tpm_mssim.c
 create mode 100644 backends/tpm/tpm_mssim.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6966490c94..b0f5cceda1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3043,9 +3043,15 @@ F: include/hw/acpi/tpm.h
 F: include/sysemu/tpm*
 F: qapi/tpm.json
 F: backends/tpm/
+X: backends/tpm/tpm_mssim.*
 F: tests/qtest/*tpm*
 T: git https://github.com/stefanberger/qemu-tpm.git tpm-next
 
+MSSIM TPM Backend
+M: James Bottomley <jejb@linux.ibm.com>
+S: Maintained
+F: backends/tpm/tpm_mssim.*
+
 Checkpatch
 S: Odd Fixes
 F: scripts/checkpatch.pl
diff --git a/backends/tpm/Kconfig b/backends/tpm/Kconfig
index 5d91eb89c2..d6d6fa53e9 100644
--- a/backends/tpm/Kconfig
+++ b/backends/tpm/Kconfig
@@ -12,3 +12,8 @@ config TPM_EMULATOR
     bool
     default y
     depends on TPM_BACKEND
+
+config TPM_MSSIM
+    bool
+    default y
+    depends on TPM_BACKEND
diff --git a/backends/tpm/meson.build b/backends/tpm/meson.build
index 7f2503f84e..c7c3c79125 100644
--- a/backends/tpm/meson.build
+++ b/backends/tpm/meson.build
@@ -3,4 +3,5 @@ if have_tpm
   softmmu_ss.add(files('tpm_util.c'))
   softmmu_ss.add(when: 'CONFIG_TPM_PASSTHROUGH', if_true: files('tpm_passthrough.c'))
   softmmu_ss.add(when: 'CONFIG_TPM_EMULATOR', if_true: files('tpm_emulator.c'))
+  softmmu_ss.add(when: 'CONFIG_TPM_MSSIM', if_true: files('tpm_mssim.c'))
 endif
diff --git a/backends/tpm/tpm_mssim.c b/backends/tpm/tpm_mssim.c
new file mode 100644
index 0000000000..1b990bfa30
--- /dev/null
+++ b/backends/tpm/tpm_mssim.c
@@ -0,0 +1,264 @@
+/*
+ * Emulator TPM driver which connects over the mssim protocol
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Copyright (c) 2022
+ * Author: James Bottomley <jejb@linux.ibm.com>
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/sockets.h"
+
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-tpm.h"
+
+#include "io/channel-socket.h"
+
+#include "sysemu/runstate.h"
+#include "sysemu/tpm_backend.h"
+#include "sysemu/tpm_util.h"
+
+#include "qom/object.h"
+
+#include "tpm_int.h"
+#include "tpm_mssim.h"
+
+#define ERROR_PREFIX "TPM mssim Emulator: "
+
+#define TYPE_TPM_MSSIM "tpm-mssim"
+OBJECT_DECLARE_SIMPLE_TYPE(TPMmssim, TPM_MSSIM)
+
+struct TPMmssim {
+    TPMBackend parent;
+
+    TpmTypeOptions *opts;
+
+    QIOChannelSocket *cmd_qc, *ctrl_qc;
+};
+
+static int tpm_send_ctrl(TPMmssim *t, uint32_t cmd, Error **errp)
+{
+    int ret;
+
+    qio_channel_socket_connect_sync(t->ctrl_qc, t->opts->u.mssim.control, errp);
+    cmd = htonl(cmd);
+    ret = qio_channel_write_all(QIO_CHANNEL(t->ctrl_qc), (char *)&cmd, sizeof(cmd), errp);
+    if (ret != 0)
+        goto out;
+    ret = qio_channel_read_all(QIO_CHANNEL(t->ctrl_qc), (char *)&cmd, sizeof(cmd), errp);
+    if (ret != 0)
+        goto out;
+    if (cmd != 0) {
+        error_setg(errp, ERROR_PREFIX "Incorrect ACK recieved on control channel 0x%x\n", cmd);
+        ret = -1;
+    }
+ out:
+    qio_channel_close(QIO_CHANNEL(t->ctrl_qc), errp);
+    return ret;
+}
+
+static void tpm_mssim_instance_init(Object *obj)
+{
+}
+
+static void tpm_mssim_instance_finalize(Object *obj)
+{
+    TPMmssim *t = TPM_MSSIM(obj);
+
+    if (t->ctrl_qc && !runstate_check(RUN_STATE_INMIGRATE))
+        tpm_send_ctrl(t, TPM_SIGNAL_POWER_OFF, NULL);
+
+    object_unref(OBJECT(t->ctrl_qc));
+    object_unref(OBJECT(t->cmd_qc));
+}
+
+static void tpm_mssim_cancel_cmd(TPMBackend *tb)
+{
+        return;
+}
+
+static TPMVersion tpm_mssim_get_version(TPMBackend *tb)
+{
+    return TPM_VERSION_2_0;
+}
+
+static size_t tpm_mssim_get_buffer_size(TPMBackend *tb)
+{
+    /* TCG standard profile max buffer size */
+    return 4096;
+}
+
+static TpmTypeOptions *tpm_mssim_get_opts(TPMBackend *tb)
+{
+    TPMmssim *t = TPM_MSSIM(tb);
+    TpmTypeOptions *opts;
+
+    opts = QAPI_CLONE(TpmTypeOptions, t->opts);
+
+    return opts;
+}
+
+static void tpm_mssim_handle_request(TPMBackend *tb, TPMBackendCmd *cmd,
+                                     Error **errp)
+{
+    TPMmssim *t = TPM_MSSIM(tb);
+    uint32_t header, len;
+    uint8_t locality = cmd->locty;
+    struct iovec iov[4];
+    int ret;
+
+    header = htonl(TPM_SEND_COMMAND);
+    len = htonl(cmd->in_len);
+
+    iov[0].iov_base = &header;
+    iov[0].iov_len = sizeof(header);
+    iov[1].iov_base = &locality;
+    iov[1].iov_len = sizeof(locality);
+    iov[2].iov_base = &len;
+    iov[2].iov_len = sizeof(len);
+    iov[3].iov_base = (void *)cmd->in;
+    iov[3].iov_len = cmd->in_len;
+
+    ret = qio_channel_writev_all(QIO_CHANNEL(t->cmd_qc), iov, 4, errp);
+    if (ret != 0)
+        goto fail;
+
+    ret = qio_channel_read_all(QIO_CHANNEL(t->cmd_qc), (char *)&len, sizeof(len), errp);
+    if (ret != 0)
+        goto fail;
+    len = ntohl(len);
+    if (len > cmd->out_len) {
+        error_setg(errp, "receive size is too large");
+        goto fail;
+    }
+    ret = qio_channel_read_all(QIO_CHANNEL(t->cmd_qc), (char *)cmd->out, len, errp);
+    if (ret != 0)
+        goto fail;
+    /* ACK packet */
+    ret = qio_channel_read_all(QIO_CHANNEL(t->cmd_qc), (char *)&header, sizeof(header), errp);
+    if (ret != 0)
+        goto fail;
+    if (header != 0) {
+        error_setg(errp, "incorrect ACK received on command channel 0x%x", len);
+        goto fail;
+    }
+
+    return;
+
+ fail:
+    error_prepend(errp, ERROR_PREFIX);
+    tpm_util_write_fatal_error_response(cmd->out, cmd->out_len);
+}
+
+static TPMBackend *tpm_mssim_create(TpmTypeOptions *opts)
+{
+    TPMBackend *be = TPM_BACKEND(object_new(TYPE_TPM_MSSIM));
+    TPMmssim *t = TPM_MSSIM(be);
+    int sock;
+    Error *errp = NULL;
+    TPMmssimOptions *mo = &opts->u.mssim;
+
+    t->opts = opts;
+    if (!mo->has_command) {
+            mo->has_command = true;
+            mo->command = g_new0(SocketAddress, 1);
+            mo->command->type = SOCKET_ADDRESS_TYPE_INET;
+            mo->command->u.inet.host = g_strdup("localhost");
+            mo->command->u.inet.port = g_strdup("2321");
+    }
+    if (!mo->has_control) {
+            int port;
+
+            mo->has_control = true;
+            mo->control = g_new0(SocketAddress, 1);
+            mo->control->type = SOCKET_ADDRESS_TYPE_INET;
+            mo->control->u.inet.host = g_strdup(mo->command->u.inet.host);
+            /* in the reference implementation, the control port is
+             * always one above the command port */
+            port = atoi(mo->command->u.inet.port) + 1;
+            mo->control->u.inet.port = g_strdup_printf("%d", port);
+    }
+
+    t->cmd_qc = qio_channel_socket_new();
+    t->ctrl_qc = qio_channel_socket_new();
+
+    if (qio_channel_socket_connect_sync(t->cmd_qc, mo->command, &errp) < 0)
+        goto fail;
+
+    if (qio_channel_socket_connect_sync(t->ctrl_qc, mo->control, &errp) < 0)
+        goto fail;
+    qio_channel_close(QIO_CHANNEL(t->ctrl_qc), &errp);
+
+    if (!runstate_check(RUN_STATE_INMIGRATE)) {
+        /* reset the TPM using a power cycle sequence, in case someone
+         * has previously powered it up */
+        sock = tpm_send_ctrl(t, TPM_SIGNAL_POWER_OFF, &errp);
+        if (sock != 0)
+            goto fail;
+        sock = tpm_send_ctrl(t, TPM_SIGNAL_POWER_ON, &errp);
+        if (sock != 0)
+            goto fail;
+        sock = tpm_send_ctrl(t, TPM_SIGNAL_NV_ON, &errp);
+        if (sock != 0)
+            goto fail;
+    }
+
+    return be;
+
+ fail:
+    object_unref(OBJECT(t->ctrl_qc));
+    object_unref(OBJECT(t->cmd_qc));
+    t->ctrl_qc = NULL;
+    t->cmd_qc = NULL;
+    error_prepend(&errp, ERROR_PREFIX);
+    error_report_err(errp);
+    object_unref(OBJECT(be));
+
+    return NULL;
+}
+
+static const QemuOptDesc tpm_mssim_cmdline_opts[] = {
+    TPM_STANDARD_CMDLINE_OPTS,
+    {
+        .name = "command",
+        .type = QEMU_OPT_STRING,
+        .help = "Command socket (default localhost:2321)",
+    },
+    {
+        .name = "control",
+        .type = QEMU_OPT_STRING,
+        .help = "control socket (default localhost:2322)",
+    },
+};
+
+static void tpm_mssim_class_init(ObjectClass *klass, void *data)
+{
+    TPMBackendClass *cl = TPM_BACKEND_CLASS(klass);
+
+    cl->type = TPM_TYPE_MSSIM;
+    cl->opts = tpm_mssim_cmdline_opts;
+    cl->desc = "TPM mssim emulator backend driver";
+    cl->create = tpm_mssim_create;
+    cl->cancel_cmd = tpm_mssim_cancel_cmd;
+    cl->get_tpm_version = tpm_mssim_get_version;
+    cl->get_buffer_size = tpm_mssim_get_buffer_size;
+    cl->get_tpm_options = tpm_mssim_get_opts;
+    cl->handle_request = tpm_mssim_handle_request;
+}
+
+static const TypeInfo tpm_mssim_info = {
+    .name = TYPE_TPM_MSSIM,
+    .parent = TYPE_TPM_BACKEND,
+    .instance_size = sizeof(TPMmssim),
+    .class_init = tpm_mssim_class_init,
+    .instance_init = tpm_mssim_instance_init,
+    .instance_finalize = tpm_mssim_instance_finalize,
+};
+
+static void tpm_mssim_register(void)
+{
+    type_register_static(&tpm_mssim_info);
+}
+
+type_init(tpm_mssim_register)
diff --git a/backends/tpm/tpm_mssim.h b/backends/tpm/tpm_mssim.h
new file mode 100644
index 0000000000..04a270338a
--- /dev/null
+++ b/backends/tpm/tpm_mssim.h
@@ -0,0 +1,43 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * The code below is copied from the Microsoft/TCG Reference implementation
+ *
+ *  https://github.com/Microsoft/ms-tpm-20-ref.git
+ *
+ * In file TPMCmd/Simulator/include/TpmTcpProtocol.h
+ */
+
+#define TPM_SIGNAL_POWER_ON         1
+#define TPM_SIGNAL_POWER_OFF        2
+#define TPM_SIGNAL_PHYS_PRES_ON     3
+#define TPM_SIGNAL_PHYS_PRES_OFF    4
+#define TPM_SIGNAL_HASH_START       5
+#define TPM_SIGNAL_HASH_DATA        6
+        // {uint32_t BufferSize, uint8_t[BufferSize] Buffer}
+#define TPM_SIGNAL_HASH_END         7
+#define TPM_SEND_COMMAND            8
+        // {uint8_t Locality, uint32_t InBufferSize, uint8_t[InBufferSize] InBuffer} ->
+        //     {uint32_t OutBufferSize, uint8_t[OutBufferSize] OutBuffer}
+
+#define TPM_SIGNAL_CANCEL_ON        9
+#define TPM_SIGNAL_CANCEL_OFF       10
+#define TPM_SIGNAL_NV_ON            11
+#define TPM_SIGNAL_NV_OFF           12
+#define TPM_SIGNAL_KEY_CACHE_ON     13
+#define TPM_SIGNAL_KEY_CACHE_OFF    14
+
+#define TPM_REMOTE_HANDSHAKE        15
+#define TPM_SET_ALTERNATIVE_RESULT  16
+
+#define TPM_SIGNAL_RESET            17
+#define TPM_SIGNAL_RESTART          18
+
+#define TPM_SESSION_END             20
+#define TPM_STOP                    21
+
+#define TPM_GET_COMMAND_RESPONSE_SIZES  25
+
+#define TPM_ACT_GET_SIGNALED        26
+
+#define TPM_TEST_FAILURE_MODE       30
diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..1398735956 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -270,6 +270,38 @@ available as a module (assuming a TPM 2 is passed through):
   /sys/devices/LNXSYSTEM:00/LNXSYBUS:00/MSFT0101:00/tpm/tpm0/pcr-sha256/9
   ...
 
+The QEMU TPM Microsoft Simulator Device
+---------------------------------------
+
+The TCG provides a reference implementation for TPM 2.0 written by
+Microsoft (See `ms-tpm-20-ref`_ on github).  The reference implementation
+starts a network server and listens for TPM commands on port 2321 and
+TPM Platform control commands on port 2322, although these can be
+altered.  The QEMU mssim TPM backend talks to this implementation.  By
+default it connects to the default ports on localhost:
+
+.. code-block:: console
+
+  qemu-system-x86_64 <qemu-options> \
+    -tpmdev mssim,id=tpm0 \
+    -device tpm-crb,tpmdev=tpm0
+
+
+Although it can also communicate with a remote host, which must be
+specified as a SocketAddress via json on the command line for each of
+the command and control ports:
+
+.. code-block:: console
+
+  qemu-system-x86_64 <qemu-options> \
+    -tpmdev "{'type':'mssim','id':'tpm0','command':{'type':'inet','host':'remote','port':'2321'},'control':{'type':'inet','host':'remote','port':'2322'}}" \
+    -device tpm-crb,tpmdev=tpm0
+
+
+The mssim backend supports snapshotting and migration, but the state
+of the Microsoft Simulator server must be preserved (or the server
+kept running) outside of QEMU for restore to be successful.
+
 The QEMU TPM emulator device
 ----------------------------
 
@@ -526,3 +558,6 @@ the following:
 
 .. _SWTPM protocol:
    https://github.com/stefanberger/swtpm/blob/master/man/man3/swtpm_ioctls.pod
+
+.. _ms-tpm-20-ref:
+   https://github.com/microsoft/ms-tpm-20-ref
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index e99447ad68..319f9eeeb6 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -841,6 +841,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
     unsigned int c = 0;
     TPMPassthroughOptions *tpo;
     TPMEmulatorOptions *teo;
+    TPMmssimOptions *tmo;
 
     info_list = qmp_query_tpm(&err);
     if (err) {
@@ -874,6 +875,12 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
             teo = &ti->options->u.emulator;
             monitor_printf(mon, ",chardev=%s", teo->chardev);
             break;
+        case TPM_TYPE_MSSIM:
+            tmo = &ti->options->u.mssim;
+            monitor_printf(mon, ",command=%s:%s,control=%s:%s",
+                           tmo->command->u.inet.host, tmo->command->u.inet.port,
+                           tmo->control->u.inet.host, tmo->control->u.inet.port);
+            break;
         case TPM_TYPE__MAX:
             break;
         }
diff --git a/qapi/tpm.json b/qapi/tpm.json
index d8cbd5ea0e..b773bde2ff 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -5,6 +5,7 @@
 ##
 # = TPM (trusted platform module) devices
 ##
+{ 'include': 'sockets.json' }
 
 ##
 # @TpmModel:
@@ -49,7 +50,7 @@
 #
 # Since: 1.5
 ##
-{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator' ],
+{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator', 'mssim' ],
   'if': 'CONFIG_TPM' }
 
 ##
@@ -64,7 +65,7 @@
 # Example:
 #
 # -> { "execute": "query-tpm-types" }
-# <- { "return": [ "passthrough", "emulator" ] }
+# <- { "return": [ "passthrough", "emulator", "mssim" ] }
 #
 ##
 { 'command': 'query-tpm-types', 'returns': ['TpmType'],
@@ -99,6 +100,22 @@
 { 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
   'if': 'CONFIG_TPM' }
 
+##
+# @TPMmssimOptions:
+#
+# Information for the mssim emulator connection
+#
+# @command: command socket for the TPM emulator
+# @control: control socket for the TPM emulator
+#
+# Since: 7.2.0
+##
+{ 'struct': 'TPMmssimOptions',
+  'data': {
+      '*command': 'SocketAddress',
+      '*control': 'SocketAddress' },
+  'if': 'CONFIG_TPM' }
+
 ##
 # @TpmTypeOptions:
 #
@@ -107,6 +124,7 @@
 # @id: identifier of the backend
 # @type: - 'passthrough' The configuration options for the TPM passthrough type
 #        - 'emulator' The configuration options for TPM emulator backend type
+#        - 'mssim' The configuration options for TPM emulator mssim type
 #
 # Since: 1.5
 ##
@@ -115,7 +133,8 @@
             'id': 'str' },
   'discriminator': 'type',
   'data': { 'passthrough' : 'TPMPassthroughOptions',
-            'emulator': 'TPMEmulatorOptions' },
+            'emulator': 'TPMEmulatorOptions',
+            'mssim': 'TPMmssimOptions' },
   'if': 'CONFIG_TPM' }
 
 ##
-- 
2.35.3



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

* Re: [PATCH v3 0/2] tpm: add mssim backend
  2022-12-19 13:13 [PATCH v3 0/2] tpm: add mssim backend James Bottomley
  2022-12-19 13:13 ` [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format James Bottomley
  2022-12-19 13:13 ` [PATCH v3 2/2] tpm: add backend for mssim James Bottomley
@ 2022-12-19 13:51 ` Stefan Berger
  2022-12-19 13:55   ` James Bottomley
  2022-12-19 15:16 ` Stefan Berger
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2022-12-19 13:51 UTC (permalink / raw)
  To: James Bottomley, qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster



On 12/19/22 08:13, James Bottomley wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> The requested feedback was to convert the tpmdev handler to being json
> based, which requires rethreading all the backends.  The good news is
> this reduced quite a bit of code (especially as I converted it to
> error_fatal handling as well, which removes the return status
> threading).  The bad news is I can't test any of the conversions.
> swtpm still isn't building on opensuse and, apparently, passthrough

The package seems to be available: https://software.opensuse.org/package/swtpm


I'll get to looking at this in more depth once I am back in office.

    Stefan


> doesn't like my native TPM because it doesn't allow cancellation.
> 
> v3 pulls out more unneeded code in the visitor conversion, makes
> migration work on external state preservation of the simulator and
> adds documentation
> 
> James
> 
> ---
> 
> James Bottomley (2):
>    tpm: convert tpmdev options processing to new visitor format
>    tpm: add backend for mssim
> 
>   MAINTAINERS                    |   6 +
>   backends/tpm/Kconfig           |   5 +
>   backends/tpm/meson.build       |   1 +
>   backends/tpm/tpm_emulator.c    |  35 ++---
>   backends/tpm/tpm_mssim.c       | 264 +++++++++++++++++++++++++++++++++
>   backends/tpm/tpm_mssim.h       |  43 ++++++
>   backends/tpm/tpm_passthrough.c |  37 ++---
>   docs/specs/tpm.rst             |  35 +++++
>   include/sysemu/tpm.h           |   4 +-
>   include/sysemu/tpm_backend.h   |   2 +-
>   monitor/hmp-cmds.c             |  11 +-
>   qapi/tpm.json                  |  37 ++---
>   softmmu/tpm.c                  |  90 +++++------
>   softmmu/vl.c                   |  19 +--
>   14 files changed, 449 insertions(+), 140 deletions(-)
>   create mode 100644 backends/tpm/tpm_mssim.c
>   create mode 100644 backends/tpm/tpm_mssim.h
> 


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

* Re: [PATCH v3 0/2] tpm: add mssim backend
  2022-12-19 13:51 ` [PATCH v3 0/2] tpm: add mssim backend Stefan Berger
@ 2022-12-19 13:55   ` James Bottomley
  2022-12-19 14:15     ` Stefan Berger
  0 siblings, 1 reply; 11+ messages in thread
From: James Bottomley @ 2022-12-19 13:55 UTC (permalink / raw)
  To: Stefan Berger, qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster

On Mon, 2022-12-19 at 08:51 -0500, Stefan Berger wrote:
> 
> 
> On 12/19/22 08:13, James Bottomley wrote:
> > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > 
> > The requested feedback was to convert the tpmdev handler to being
> > json based, which requires rethreading all the backends.  The good
> > news is this reduced quite a bit of code (especially as I converted
> > it to error_fatal handling as well, which removes the return status
> > threading).  The bad news is I can't test any of the conversions.
> > swtpm still isn't building on opensuse and, apparently, passthrough
> 
> The package seems to be available:
> https://software.opensuse.org/package/swtpm

It's not building for any of the platforms I currently have.

I think I've tested most of the option processing, though, before it
tells me it can't connect.

> I'll get to looking at this in more depth once I am back in office.

That's great, thanks ... it would certainly be better to test option
processing on a working platform.

James



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

* Re: [PATCH v3 0/2] tpm: add mssim backend
  2022-12-19 13:55   ` James Bottomley
@ 2022-12-19 14:15     ` Stefan Berger
  2022-12-19 14:17       ` James Bottomley
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2022-12-19 14:15 UTC (permalink / raw)
  To: jejb, qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster



On 12/19/22 08:55, James Bottomley wrote:
> On Mon, 2022-12-19 at 08:51 -0500, Stefan Berger wrote:
>>
>>
>> On 12/19/22 08:13, James Bottomley wrote:
>>> From: James Bottomley <James.Bottomley@HansenPartnership.com>
>>>
>>> The requested feedback was to convert the tpmdev handler to being
>>> json based, which requires rethreading all the backends.  The good
>>> news is this reduced quite a bit of code (especially as I converted
>>> it to error_fatal handling as well, which removes the return status
>>> threading).  The bad news is I can't test any of the conversions.
>>> swtpm still isn't building on opensuse and, apparently, passthrough
>>
>> The package seems to be available:
>> https://software.opensuse.org/package/swtpm
> 
> It's not building for any of the platforms I currently have.

You would have to tell me what is failing. I have been building it for several platforms for a while and the build works, including OpenSuSE Tumbleweed:


https://app.travis-ci.com/github/stefanberger/swtpm-distro-compile/builds/258769183

There have been issues with what seems to be seccomp policy on 2 of these platforms for a while but this is unrelated to SuSE and build issues -- obviously.

    Stefan

> 
> I think I've tested most of the option processing, though, before it
> tells me it can't connect.
> 
>> I'll get to looking at this in more depth once I am back in office.
> 
> That's great, thanks ... it would certainly be better to test option
> processing on a working platform.
> 
> James
> 


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

* Re: [PATCH v3 0/2] tpm: add mssim backend
  2022-12-19 14:15     ` Stefan Berger
@ 2022-12-19 14:17       ` James Bottomley
  0 siblings, 0 replies; 11+ messages in thread
From: James Bottomley @ 2022-12-19 14:17 UTC (permalink / raw)
  To: Stefan Berger, qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster

On Mon, 2022-12-19 at 09:15 -0500, Stefan Berger wrote:
> 
> 
> On 12/19/22 08:55, James Bottomley wrote:
> > On Mon, 2022-12-19 at 08:51 -0500, Stefan Berger wrote:
> > > 
> > > 
> > > On 12/19/22 08:13, James Bottomley wrote:
> > > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > > 
> > > > The requested feedback was to convert the tpmdev handler to
> > > > being json based, which requires rethreading all the backends. 
> > > > The good news is this reduced quite a bit of code (especially
> > > > as I converted it to error_fatal handling as well, which
> > > > removes the return status threading).  The bad news is I can't
> > > > test any of the conversions. swtpm still isn't building on
> > > > opensuse and, apparently, passthrough
> > > 
> > > The package seems to be available:
> > > https://software.opensuse.org/package/swtpm
> > 
> > It's not building for any of the platforms I currently have.
> 
> You would have to tell me what is failing. I have been building it
> for several platforms for a while and the build works, including
> OpenSuSE Tumbleweed:
> 
> 
> https://app.travis-ci.com/github/stefanberger/swtpm-distro-compile/builds/258769183
> 
> There have been issues with what seems to be seccomp policy on 2 of
> these platforms for a while but this is unrelated to SuSE and build
> issues -- obviously.

All I know is what the build service says, which is the URL I first
pointed you to:

https://build.opensuse.org/package/show/security/swtpm

I haven't dug into the problem.

James



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

* Re: [PATCH v3 0/2] tpm: add mssim backend
  2022-12-19 13:13 [PATCH v3 0/2] tpm: add mssim backend James Bottomley
                   ` (2 preceding siblings ...)
  2022-12-19 13:51 ` [PATCH v3 0/2] tpm: add mssim backend Stefan Berger
@ 2022-12-19 15:16 ` Stefan Berger
  2022-12-19 15:21   ` James Bottomley
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2022-12-19 15:16 UTC (permalink / raw)
  To: James Bottomley, qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster



On 12/19/22 08:13, James Bottomley wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> The requested feedback was to convert the tpmdev handler to being json
> based, which requires rethreading all the backends.  The good news is
> this reduced quite a bit of code (especially as I converted it to
> error_fatal handling as well, which removes the return status
> threading).  The bad news is I can't test any of the conversions.
> swtpm still isn't building on opensuse and, apparently, passthrough
> doesn't like my native TPM because it doesn't allow cancellation.

For passthrough you can use /dev/null in place of the cancel file. Libvirt does that also:

https://github.com/stefanberger/libvirt-tpm/blob/master/src/util/virtpm.c#L88

    Stefan


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

* Re: [PATCH v3 0/2] tpm: add mssim backend
  2022-12-19 15:16 ` Stefan Berger
@ 2022-12-19 15:21   ` James Bottomley
  0 siblings, 0 replies; 11+ messages in thread
From: James Bottomley @ 2022-12-19 15:21 UTC (permalink / raw)
  To: Stefan Berger, qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster

On Mon, 2022-12-19 at 10:16 -0500, Stefan Berger wrote:
> 
> 
> On 12/19/22 08:13, James Bottomley wrote:
> > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > 
> > The requested feedback was to convert the tpmdev handler to being
> > json
> > based, which requires rethreading all the backends.  The good news
> > is
> > this reduced quite a bit of code (especially as I converted it to
> > error_fatal handling as well, which removes the return status
> > threading).  The bad news is I can't test any of the conversions.
> > swtpm still isn't building on opensuse and, apparently, passthrough
> > doesn't like my native TPM because it doesn't allow cancellation.
> 
> For passthrough you can use /dev/null in place of the cancel file.
> Libvirt does that also:
> 
> https://github.com/stefanberger/libvirt-tpm/blob/master/src/util/virtpm.c#L88

OK, so passthrough works with the visitor conversion.  If /dev/null is
the default for no cancel path, the backend shouldn't really beat the
end user up about not specifying it if it can't find the cancel path
for the chosen host TPM.

James



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

* Re: [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format
  2022-12-19 13:13 ` [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format James Bottomley
@ 2022-12-21 16:32   ` Daniel P. Berrangé
  2022-12-21 20:31     ` James Bottomley
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-12-21 16:32 UTC (permalink / raw)
  To: James Bottomley; +Cc: qemu-devel, Stefan Berger, Markus Armbruster

On Mon, Dec 19, 2022 at 08:13:43AM -0500, James Bottomley wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Instead of processing the tpmdev options using the old qemu options,
> convert to the new visitor format which also allows the passing of
> json on the command line.
> 
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  backends/tpm/tpm_emulator.c    | 35 ++++++-------
>  backends/tpm/tpm_passthrough.c | 37 +++++---------
>  include/sysemu/tpm.h           |  4 +-
>  include/sysemu/tpm_backend.h   |  2 +-
>  monitor/hmp-cmds.c             |  4 +-
>  qapi/tpm.json                  | 26 ++--------
>  softmmu/tpm.c                  | 90 ++++++++++++++--------------------
>  softmmu/vl.c                   | 19 +------
>  8 files changed, 73 insertions(+), 144 deletions(-)
> 

> diff --git a/qapi/tpm.json b/qapi/tpm.json
> index 4e2ea9756a..d8cbd5ea0e 100644
> --- a/qapi/tpm.json
> +++ b/qapi/tpm.json
> @@ -99,39 +99,23 @@
>  { 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
>    'if': 'CONFIG_TPM' }
>  
> -##
> -# @TPMPassthroughOptionsWrapper:
> -#
> -# Since: 1.5
> -##
> -{ 'struct': 'TPMPassthroughOptionsWrapper',
> -  'data': { 'data': 'TPMPassthroughOptions' },
> -  'if': 'CONFIG_TPM' }
> -
> -##
> -# @TPMEmulatorOptionsWrapper:
> -#
> -# Since: 2.11
> -##
> -{ 'struct': 'TPMEmulatorOptionsWrapper',
> -  'data': { 'data': 'TPMEmulatorOptions' },
> -  'if': 'CONFIG_TPM' }
> -
>  ##
>  # @TpmTypeOptions:
>  #
>  # A union referencing different TPM backend types' configuration options
>  #
> +# @id: identifier of the backend
>  # @type: - 'passthrough' The configuration options for the TPM passthrough type
>  #        - 'emulator' The configuration options for TPM emulator backend type
>  #
>  # Since: 1.5
>  ##
>  { 'union': 'TpmTypeOptions',
> -  'base': { 'type': 'TpmType' },
> +  'base': { 'type': 'TpmType',
> +            'id': 'str' },
>    'discriminator': 'type',
> -  'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
> -            'emulator': 'TPMEmulatorOptionsWrapper' },
> +  'data': { 'passthrough' : 'TPMPassthroughOptions',
> +            'emulator': 'TPMEmulatorOptions' },
>    'if': 'CONFIG_TPM' }

This isn't a valid change todo, as it affects the public facing
data structure for the  query-tpm command.

I understand why you're doing it though, to get rid fo the
extra nesting, which is a hangover from earlier QAPI days
where we couldn't cope with flat unions.

Instead of changing TpmTypeOptions, you'll need to introduce
a new TpmTypeCreateOptions that eliminates the wrapping, and
use that in the CLI creation path, leaving the query-tpm
command unchanged.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



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

* Re: [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format
  2022-12-21 16:32   ` Daniel P. Berrangé
@ 2022-12-21 20:31     ` James Bottomley
  0 siblings, 0 replies; 11+ messages in thread
From: James Bottomley @ 2022-12-21 20:31 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: qemu-devel, Stefan Berger, Markus Armbruster

On Wed, 2022-12-21 at 16:32 +0000, Daniel P. Berrangé wrote:
> This isn't a valid change todo, as it affects the public facing
> data structure for the  query-tpm command.
> 
> I understand why you're doing it though, to get rid fo the
> extra nesting, which is a hangover from earlier QAPI days
> where we couldn't cope with flat unions.
> 
> Instead of changing TpmTypeOptions, you'll need to introduce
> a new TpmTypeCreateOptions that eliminates the wrapping, and
> use that in the CLI creation path, leaving the query-tpm
> command unchanged.


Well, it makes the diffstat less favourable, but how about this (I'm
also assuming I don't need to wrapper the new mssim options)?

James

---

 backends/tpm/tpm_emulator.c    | 24 ++++-----
 backends/tpm/tpm_passthrough.c | 27 +++-------
 include/sysemu/tpm.h           |  4 +-
 include/sysemu/tpm_backend.h   |  2 +-
 qapi/tpm.json                  | 19 +++++++
 softmmu/tpm.c                  | 90 ++++++++++++++--------------------
 softmmu/vl.c                   | 19 +------
 7 files changed, 76 insertions(+), 109 deletions(-)

diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 49cc3d749d..cb6bf9d7c2 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -584,33 +584,28 @@ err_exit:
     return -1;
 }
 
-static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
+static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, TpmCreateOptions *opts)
 {
-    const char *value;
     Error *err = NULL;
     Chardev *dev;
 
-    value = qemu_opt_get(opts, "chardev");
-    if (!value) {
-        error_report("tpm-emulator: parameter 'chardev' is missing");
-        goto err;
-    }
+    tpm_emu->options = QAPI_CLONE(TPMEmulatorOptions, &opts->u.emulator);
+    tpm_emu->data_ioc = NULL;
 
-    dev = qemu_chr_find(value);
+    dev = qemu_chr_find(opts->u.emulator.chardev);
     if (!dev) {
-        error_report("tpm-emulator: tpm chardev '%s' not found", value);
+        error_report("tpm-emulator: tpm chardev '%s' not found",
+                opts->u.emulator.chardev);
         goto err;
     }
 
     if (!qemu_chr_fe_init(&tpm_emu->ctrl_chr, dev, &err)) {
         error_prepend(&err, "tpm-emulator: No valid chardev found at '%s':",
-                      value);
+                      opts->u.emulator.chardev);
         error_report_err(err);
         goto err;
     }
 
-    tpm_emu->options->chardev = g_strdup(value);
-
     if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
         goto err;
     }
@@ -649,7 +644,7 @@ err:
     return -1;
 }
 
-static TPMBackend *tpm_emulator_create(QemuOpts *opts)
+static TPMBackend *tpm_emulator_create(TpmCreateOptions *opts)
 {
     TPMBackend *tb = TPM_BACKEND(object_new(TYPE_TPM_EMULATOR));
 
@@ -972,7 +967,6 @@ static void tpm_emulator_inst_init(Object *obj)
 
     trace_tpm_emulator_inst_init();
 
-    tpm_emu->options = g_new0(TPMEmulatorOptions, 1);
     tpm_emu->cur_locty_number = ~0;
     qemu_mutex_init(&tpm_emu->mutex);
     tpm_emu->vmstate =
@@ -990,7 +984,7 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
 {
     ptm_res res;
 
-    if (!tpm_emu->options->chardev) {
+    if (!tpm_emu->data_ioc) {
         /* was never properly initialized */
         return;
     }
diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
index 5a2f74db1b..f9771eaf1f 100644
--- a/backends/tpm/tpm_passthrough.c
+++ b/backends/tpm/tpm_passthrough.c
@@ -252,23 +252,11 @@ static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
 }
 
 static int
-tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, QemuOpts *opts)
+tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, TpmCreateOptions *opts)
 {
-    const char *value;
+    tpm_pt->options = QAPI_CLONE(TPMPassthroughOptions, &opts->u.passthrough);
 
-    value = qemu_opt_get(opts, "cancel-path");
-    if (value) {
-        tpm_pt->options->cancel_path = g_strdup(value);
-        tpm_pt->options->has_cancel_path = true;
-    }
-
-    value = qemu_opt_get(opts, "path");
-    if (value) {
-        tpm_pt->options->has_path = true;
-        tpm_pt->options->path = g_strdup(value);
-    }
-
-    tpm_pt->tpm_dev = value ? value : TPM_PASSTHROUGH_DEFAULT_DEVICE;
+    tpm_pt->tpm_dev = opts->u.passthrough.has_path ? opts->u.passthrough.path : TPM_PASSTHROUGH_DEFAULT_DEVICE;
     tpm_pt->tpm_fd = qemu_open_old(tpm_pt->tpm_dev, O_RDWR);
     if (tpm_pt->tpm_fd < 0) {
         error_report("Cannot access TPM device using '%s': %s",
@@ -290,11 +278,11 @@ tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, QemuOpts *opts)
     return 0;
 }
 
-static TPMBackend *tpm_passthrough_create(QemuOpts *opts)
+static TPMBackend *tpm_passthrough_create(TpmCreateOptions *tco)
 {
     Object *obj = object_new(TYPE_TPM_PASSTHROUGH);
 
-    if (tpm_passthrough_handle_device_opts(TPM_PASSTHROUGH(obj), opts)) {
+    if (tpm_passthrough_handle_device_opts(TPM_PASSTHROUGH(obj), tco)) {
         object_unref(obj);
         return NULL;
     }
@@ -321,8 +309,8 @@ static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
     TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
 
     options->type = TPM_TYPE_PASSTHROUGH;
-    options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions,
-                                             TPM_PASSTHROUGH(tb)->options);
+
+    options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions, TPM_PASSTHROUGH(tb)->options);
 
     return options;
 }
@@ -346,7 +334,6 @@ static void tpm_passthrough_inst_init(Object *obj)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
 
-    tpm_pt->options = g_new0(TPMPassthroughOptions, 1);
     tpm_pt->tpm_fd = -1;
     tpm_pt->cancel_fd = -1;
 }
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index fb40e30ff6..d00a833a21 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -17,8 +17,8 @@
 
 #ifdef CONFIG_TPM
 
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
-int tpm_init(void);
+void tpm_config_parse(const char *optarg);
+void tpm_init(void);
 void tpm_cleanup(void);
 
 typedef enum TPMVersion {
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 8fd3269c11..846a9e39fa 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -57,7 +57,7 @@ struct TPMBackendClass {
     /* get a descriptive text of the backend to display to the user */
     const char *desc;
 
-    TPMBackend *(*create)(QemuOpts *opts);
+    TPMBackend *(*create)(TpmCreateOptions *tco);
 
     /* start up the TPM on the backend - optional */
     int (*startup_tpm)(TPMBackend *t, size_t buffersize);
diff --git a/qapi/tpm.json b/qapi/tpm.json
index 4e2ea9756a..2b491c28b4 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -134,6 +134,25 @@
             'emulator': 'TPMEmulatorOptionsWrapper' },
   'if': 'CONFIG_TPM' }
 
+##
+# @TpmCreateOptions:
+#
+# A union referencing different TPM backend types' configuration options
+#   without the wrapper to be usable by visitors.
+#
+# @type: - 'passthrough' The configuration options for the TPM passthrough type
+#        - 'emulator' The configuration options for TPM emulator backend type
+#
+# Since: 7.2
+##
+{ 'union': 'TpmCreateOptions',
+  'base': { 'type': 'TpmType',
+            'id' : 'str' },
+  'discriminator': 'type',
+  'data': { 'passthrough' : 'TPMPassthroughOptions',
+            'emulator': 'TPMEmulatorOptions' },
+  'if': 'CONFIG_TPM' }
+
 ##
 # @TPMInfo:
 #
diff --git a/softmmu/tpm.c b/softmmu/tpm.c
index 578563f05a..699c46d3ae 100644
--- a/softmmu/tpm.c
+++ b/softmmu/tpm.c
@@ -17,14 +17,26 @@
 #include "qapi/error.h"
 #include "qapi/qapi-commands-tpm.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-tpm.h"
 #include "sysemu/tpm_backend.h"
 #include "sysemu/tpm.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/help_option.h"
 
 static QLIST_HEAD(, TPMBackend) tpm_backends =
     QLIST_HEAD_INITIALIZER(tpm_backends);
 
+typedef struct TpmCreateOptionsQueueEntry {
+        TpmCreateOptions *tco;
+        QSIMPLEQ_ENTRY(TpmCreateOptionsQueueEntry) entry;
+} TpmCreateOptionsQueueEntry;
+
+typedef QSIMPLEQ_HEAD(, TpmCreateOptionsQueueEntry) TpmCreateOptionsQueue;
+
+static TpmCreateOptionsQueue tco_queue = QSIMPLEQ_HEAD_INITIALIZER(tco_queue);
+
 static const TPMBackendClass *
 tpm_be_find_by_type(enum TpmType type)
 {
@@ -84,63 +96,31 @@ TPMBackend *qemu_find_tpm_be(const char *id)
     return NULL;
 }
 
-static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
+static void tpm_init_tpmdev(TpmCreateOptions *tco)
 {
-    /*
-     * Use of error_report() in a function with an Error ** parameter
-     * is suspicious.  It is okay here.  The parameter only exists to
-     * make the function usable with qemu_opts_foreach().  It is not
-     * actually used.
-     */
-    const char *value;
-    const char *id;
     const TPMBackendClass *be;
     TPMBackend *drv;
-    Error *local_err = NULL;
-    int i;
 
     if (!QLIST_EMPTY(&tpm_backends)) {
         error_report("Only one TPM is allowed.");
-        return 1;
+        exit(1);
     }
 
-    id = qemu_opts_id(opts);
-    if (id == NULL) {
-        error_report(QERR_MISSING_PARAMETER, "id");
-        return 1;
-    }
-
-    value = qemu_opt_get(opts, "type");
-    if (!value) {
-        error_report(QERR_MISSING_PARAMETER, "type");
-        tpm_display_backend_drivers();
-        return 1;
-    }
-
-    i = qapi_enum_parse(&TpmType_lookup, value, -1, NULL);
-    be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
+    be = tco->type >= 0 ? tpm_be_find_by_type(tco->type) : NULL;
     if (be == NULL) {
         error_report(QERR_INVALID_PARAMETER_VALUE,
                      "type", "a TPM backend type");
         tpm_display_backend_drivers();
-        return 1;
-    }
-
-    /* validate backend specific opts */
-    if (!qemu_opts_validate(opts, be->opts, &local_err)) {
-        error_report_err(local_err);
-        return 1;
+        exit(1);
     }
 
-    drv = be->create(opts);
+    drv = be->create(tco);
     if (!drv) {
-        return 1;
+        exit(1);
     }
 
-    drv->id = g_strdup(id);
+    drv->id = g_strdup(tco->id);
     QLIST_INSERT_HEAD(&tpm_backends, drv, list);
-
-    return 0;
 }
 
 /*
@@ -161,33 +141,35 @@ void tpm_cleanup(void)
  * Initialize the TPM. Process the tpmdev command line options describing the
  * TPM backend.
  */
-int tpm_init(void)
+void tpm_init(void)
 {
-    if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
-                          tpm_init_tpmdev, NULL, NULL)) {
-        return -1;
-    }
+    while (!QSIMPLEQ_EMPTY(&tco_queue)) {
+        TpmCreateOptionsQueueEntry *tcoqe = QSIMPLEQ_FIRST(&tco_queue);
 
-    return 0;
+        QSIMPLEQ_REMOVE_HEAD(&tco_queue, entry);
+        tpm_init_tpmdev(tcoqe->tco);
+        g_free(tcoqe);
+    }
 }
 
 /*
  * Parse the TPM configuration options.
  * To display all available TPM backends the user may use '-tpmdev help'
  */
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+void tpm_config_parse(const char *optarg)
 {
-    QemuOpts *opts;
+    Visitor *v;
+    TpmCreateOptionsQueueEntry *tcqe;
 
-    if (!strcmp(optarg, "help")) {
+    if (is_help_option(optarg)) {
         tpm_display_backend_drivers();
-        return -1;
-    }
-    opts = qemu_opts_parse_noisily(opts_list, optarg, true);
-    if (!opts) {
-        return -1;
+        return;
     }
-    return 0;
+    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
+    tcqe = g_new(TpmCreateOptionsQueueEntry, 1);
+    visit_type_TpmCreateOptions(v, NULL, &tcqe->tco, &error_fatal);
+    visit_free(v);
+    QSIMPLEQ_INSERT_TAIL(&tco_queue, tcqe, entry);
 }
 
 /*
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5115221efe..980f998c35 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -328,16 +328,6 @@ static QemuOptsList qemu_object_opts = {
     },
 };
 
-static QemuOptsList qemu_tpmdev_opts = {
-    .name = "tpmdev",
-    .implied_opt_name = "type",
-    .head = QTAILQ_HEAD_INITIALIZER(qemu_tpmdev_opts.head),
-    .desc = {
-        /* options are defined in the TPM backends */
-        { /* end of list */ }
-    },
-};
-
 static QemuOptsList qemu_overcommit_opts = {
     .name = "overcommit",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_overcommit_opts.head),
@@ -1934,9 +1924,7 @@ static void qemu_create_late_backends(void)
 
     object_option_foreach_add(object_create_late);
 
-    if (tpm_init() < 0) {
-        exit(1);
-    }
+    tpm_init();
 
     qemu_opts_foreach(qemu_find_opts("mon"),
                       mon_init_func, NULL, &error_fatal);
@@ -2658,7 +2646,6 @@ void qemu_init(int argc, char **argv)
     qemu_add_opts(&qemu_boot_opts);
     qemu_add_opts(&qemu_add_fd_opts);
     qemu_add_opts(&qemu_object_opts);
-    qemu_add_opts(&qemu_tpmdev_opts);
     qemu_add_opts(&qemu_overcommit_opts);
     qemu_add_opts(&qemu_msg_opts);
     qemu_add_opts(&qemu_name_opts);
@@ -2906,9 +2893,7 @@ void qemu_init(int argc, char **argv)
                 break;
 #ifdef CONFIG_TPM
             case QEMU_OPTION_tpmdev:
-                if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) {
-                    exit(1);
-                }
+                tpm_config_parse(optarg);
                 break;
 #endif
             case QEMU_OPTION_mempath:
-- 
2.35.3




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

end of thread, other threads:[~2022-12-21 20:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-19 13:13 [PATCH v3 0/2] tpm: add mssim backend James Bottomley
2022-12-19 13:13 ` [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format James Bottomley
2022-12-21 16:32   ` Daniel P. Berrangé
2022-12-21 20:31     ` James Bottomley
2022-12-19 13:13 ` [PATCH v3 2/2] tpm: add backend for mssim James Bottomley
2022-12-19 13:51 ` [PATCH v3 0/2] tpm: add mssim backend Stefan Berger
2022-12-19 13:55   ` James Bottomley
2022-12-19 14:15     ` Stefan Berger
2022-12-19 14:17       ` James Bottomley
2022-12-19 15:16 ` Stefan Berger
2022-12-19 15:21   ` James Bottomley

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.