From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40215) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THEq2-0003x4-TB for qemu-devel@nongnu.org; Thu, 27 Sep 2012 10:13:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1THEps-0003JM-Fu for qemu-devel@nongnu.org; Thu, 27 Sep 2012 10:13:14 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:43735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THEps-0003Ic-Aq for qemu-devel@nongnu.org; Thu, 27 Sep 2012 10:13:04 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 27 Sep 2012 10:12:56 -0400 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8RECpcG051668 for ; Thu, 27 Sep 2012 10:12:52 -0400 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q8RECoHR014016 for ; Thu, 27 Sep 2012 08:12:51 -0600 Message-ID: <50645EDF.8060105@linux.vnet.ibm.com> Date: Thu, 27 Sep 2012 10:12:47 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1338838668-7544-1-git-send-email-stefanb@linux.vnet.ibm.com> <1338838668-7544-2-git-send-email-stefanb@linux.vnet.ibm.com> In-Reply-To: <1338838668-7544-2-git-send-email-stefanb@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V19 1/7] Support for TPM command line options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Berger Cc: mst@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, andreas.niederl@iaik.tugraz.at On 06/04/2012 03:37 PM, Stefan Berger wrote: > This patch adds support for TPM command line options. > The command line options supported here are > > ./qemu-... -tpmdev passthrough,path=,id= > -device tpm-tis,tpmdev= > > and > > ./qemu-... -tpmdev ? > > where the latter works similar to -soundhw ? and shows a list of > available TPM backends (for example 'passthrough'). > > Using the type parameter, the backend is chosen, i.e., 'passthrough' for the > passthrough driver. The interpretation of the other parameters along > with determining whether enough parameters were provided is pushed into > the backend driver, which needs to implement the interface function > 'create' and return a TPMDriver structure if the VM can be started or 'NULL' > if not enough or bad parameters were provided. > > Monitor support for 'info tpm' has been added. It for example prints the > following: > > (qemu) info tpm > TPM devices: > tpm0: model=tpm-tis > \ tpm0: type=passthrough,path=/dev/tpm0 > > Signed-off-by: Stefan Berger > --- > hmp-commands.hx | 2 + > hmp.c | 28 +++++++ > hmp.h | 1 + > hw/tpm_tis.h | 78 ++++++++++++++++++++ > monitor.c | 8 ++ > qapi-schema.json | 29 ++++++++ > qemu-config.c | 20 +++++ > qemu-options.hx | 33 +++++++++ > qmp-commands.hx | 5 ++ > tpm.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tpm.h | 81 +++++++++++++++++++++ > vl.c | 17 +++++ > 12 files changed, 515 insertions(+), 0 deletions(-) > create mode 100644 hw/tpm_tis.h > create mode 100644 tpm.c > create mode 100644 tpm.h > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 18cb415..08f6942 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1401,6 +1401,8 @@ show device tree > show qdev device model list > @item info roms > show roms > +@item info tpm > +show the TPM device > @end table > ETEXI > > diff --git a/hmp.c b/hmp.c > index bb0952e..9aedaff 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -548,6 +548,34 @@ void hmp_info_block_jobs(Monitor *mon) > } > } > > +void hmp_info_tpm(Monitor *mon) > +{ > + TPMInfoList *info_list, *info; > + Error *err = NULL; > + unsigned int c = 0; > + > + info_list = qmp_query_tpm(&err); > + if (err) { > + monitor_printf(mon, "TPM device not supported\n"); > + error_free(err); > + return; > + } > + > + monitor_printf(mon, "TPM device:\n"); > + > + for (info = info_list; info; info = info->next) { > + TPMInfo *ti = info->value; > + monitor_printf(mon, " tpm%d: model=%s\n", > + c, ti->model); > + monitor_printf(mon, " \\ %s: type=%s%s%s\n", > + ti->id, ti->type, > + ti->has_path ? ",path=" : "", > + ti->has_path ? ti->path : ""); > + c++; > + } > + qapi_free_TPMInfoList(info_list); > +} > + > void hmp_quit(Monitor *mon, const QDict *qdict) > { > monitor_suspend(mon); > diff --git a/hmp.h b/hmp.h > index 443b812..8e2a858 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -33,6 +33,7 @@ void hmp_info_spice(Monitor *mon); > void hmp_info_balloon(Monitor *mon); > void hmp_info_pci(Monitor *mon); > void hmp_info_block_jobs(Monitor *mon); > +void hmp_info_tpm(Monitor *mon); > void hmp_quit(Monitor *mon, const QDict *qdict); > void hmp_stop(Monitor *mon, const QDict *qdict); > void hmp_system_reset(Monitor *mon, const QDict *qdict); > diff --git a/hw/tpm_tis.h b/hw/tpm_tis.h > new file mode 100644 > index 0000000..5e1f731 > --- /dev/null > +++ b/hw/tpm_tis.h > @@ -0,0 +1,78 @@ > +/* > + * tpm_tis.c - QEMU's TPM TIS interface emulator > + * > + * Copyright (C) 2006,2010,2011 IBM Corporation > + * > + * Authors: > + * Stefan Berger > + * David Safford > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + * Implementation of the TIS interface according to specs found at > + * http://www.trustedcomputiggroup.org > + * > + */ > +#ifndef HW_TPM_TIS_H > +#define HW_TPM_TIS_H > + > +#include "isa.h" > +#include "qemu-common.h" > + > +#define TPM_TIS_ADDR_BASE 0xFED40000 > + > +#define TPM_TIS_NUM_LOCALITIES 5 /* per spec */ > +#define TPM_TIS_LOCALITY_SHIFT 12 > +#define TPM_TIS_NO_LOCALITY 0xff > + > +#define TPM_TIS_IS_VALID_LOCTY(x) ((x) < TPM_TIS_NUM_LOCALITIES) > + > +#define TPM_TIS_IRQ 5 > + > +#define TPM_TIS_BUFFER_MAX 4096 > + > + > +typedef struct TPMSizedBuffer { > + uint32_t size; > + uint8_t *buffer; > +} TPMSizedBuffer; > + > +typedef enum { > + TPM_TIS_STATUS_IDLE = 0, > + TPM_TIS_STATUS_READY, > + TPM_TIS_STATUS_COMPLETION, > + TPM_TIS_STATUS_EXECUTION, > + TPM_TIS_STATUS_RECEPTION, > +} TPMTISStatus; > + > +/* locality data -- all fields are persisted */ > +typedef struct TPMLocality { > + TPMTISStatus status; > + uint8_t access; > + uint8_t sts; > + uint32_t inte; > + uint32_t ints; > + > + uint16_t w_offset; > + uint16_t r_offset; > + TPMSizedBuffer w_buffer; > + TPMSizedBuffer r_buffer; > +} TPMLocality; > + > +typedef struct TPMTISState { > + QEMUBH *bh; > + uint32_t offset; > + uint8_t buf[TPM_TIS_BUFFER_MAX]; > + > + uint8_t active_locty; > + uint8_t aborting_locty; > + uint8_t next_locty; > + > + TPMLocality loc[TPM_TIS_NUM_LOCALITIES]; > + > + qemu_irq irq; > + uint32_t irq_num; > +} TPMTISState; > + > +#endif /* HW_TPM_TIS_H */ > diff --git a/monitor.c b/monitor.c > index 12a6fe2..aec06ce 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -47,6 +47,7 @@ > #include "migration.h" > #include "kvm.h" > #include "acl.h" > +#include "tpm.h" > #include "qint.h" > #include "qfloat.h" > #include "qlist.h" > @@ -2602,6 +2603,13 @@ static mon_cmd_t info_cmds[] = { > .mhandler.info = do_trace_print_events, > }, > { > + .name = "tpm", > + .args_type = "", > + .params = "", > + .help = "show the TPM device", > + .mhandler.info = hmp_info_tpm, > + }, > + { > .name = NULL, > }, > }; > diff --git a/qapi-schema.json b/qapi-schema.json > index 2ca7195..8799322 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1755,3 +1755,32 @@ > # Since: 0.14.0 > ## > { 'command': 'device_del', 'data': {'id': 'str'} } > + > +## > +# @TPMInfo: > +# > +# Information about the TPM > +# > +# @model: The TPM frontend model, i.e., tpm-tis > +# > +# @id: The ID of the TPM > +# > +# @type: The type of TPM backend, i.e., passthrough > +# > +# @path: #optional Path to the TPM backend device > +# > +# Since: 1.1 > +## > +{ 'type': 'TPMInfo', > + 'data': {'model': 'str', 'id': 'str', 'type': 'str', '*path': 'str' } } > + > +## > +# @query-tpm > +# > +# Return information about the TPM device. > +# > +# Returns: @TPMInfo on success > +# > +# Since: 1.1 > +## > +{ 'command': 'query-tpm', 'returns': ['TPMInfo'] } > diff --git a/qemu-config.c b/qemu-config.c > index be84a03..edc8d5d 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -613,6 +613,25 @@ QemuOptsList qemu_boot_opts = { > }, > }; > > +static QemuOptsList qemu_tpmdev_opts = { > + .name = "tpmdev", > + .implied_opt_name = "type", > + .head = QTAILQ_HEAD_INITIALIZER(qemu_tpmdev_opts.head), > + .desc = { > + { > + .name = "type", > + .type = QEMU_OPT_STRING, > + .help = "Type of TPM backend", > + }, > + { > + .name = "path", > + .type = QEMU_OPT_STRING, > + .help = "Persistent storage for TPM state", > + }, > + { /* end of list */ } > + }, > +}; > + > static QemuOptsList *vm_config_groups[32] = { > &qemu_drive_opts, > &qemu_chardev_opts, > @@ -628,6 +647,7 @@ static QemuOptsList *vm_config_groups[32] = { > &qemu_machine_opts, > &qemu_boot_opts, > &qemu_iscsi_opts, > + &qemu_tpmdev_opts, > NULL, > }; > > diff --git a/qemu-options.hx b/qemu-options.hx > index 8b66264..143d92d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2012,6 +2012,39 @@ ETEXI > > DEFHEADING() > > +#ifdef CONFIG_TPM > +DEFHEADING(TPM device options:) > + > +DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \ > + "-tpmdev [],id=str[,option][,option][,...]\n", > + QEMU_ARCH_ALL) > +STEXI > + > +The general form of a TPM device option is: > +@table @option > + > +@item -tpmdev @var{backend} ,id=@var{id} [,@var{options}] > +@findex -tpmdev > +Backend type must be: > + > +The specific backend type will determine the applicable options. > +The @code{-tpmdev} options requires a @code{-device} option. > + > +Options to each backend are described below. > + > +Use ? to print all available TPM backend types. > +@example > +qemu -tpmdev ? > +@end example > + > +@end table > + > +ETEXI > + > +DEFHEADING() > + > +#endif > + > DEFHEADING(Linux/Multiboot boot specific:) > STEXI > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index db980fa..82f1a6d 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2142,3 +2142,8 @@ EQMP > .args_type = "implements:s?,abstract:b?", > .mhandler.cmd_new = qmp_marshal_input_qom_list_types, > }, > + { > + .name = "query-tpm", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_query_tpm, > + }, > diff --git a/tpm.c b/tpm.c > new file mode 100644 > index 0000000..778de42 > --- /dev/null > +++ b/tpm.c > @@ -0,0 +1,213 @@ > +/* > + * TPM configuration > + * > + * Copyright (C) 2011 IBM Corporation > + * > + * Authors: > + * Stefan Berger > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + * Based on net.c > + */ > +#include "config.h" > + > +#include "monitor.h" > +#include "qerror.h" > +#include "tpm.h" > +#include "qmp-commands.h" > + > +static QLIST_HEAD(, TPMBackend) tpm_backends = > + QLIST_HEAD_INITIALIZER(tpm_backends); > + > +#ifdef CONFIG_TPM > + > +static const TPMDriverOps *bes[] = { I think bes[] would be more descriptive if it were named be_drivers[] or be_ops[]? > + NULL, > +}; > + > +const TPMDriverOps *tpm_get_backend_driver(const char *id) > +{ > + int i; > + > + for (i = 0; bes[i] != NULL; i++) { > + if (!strcmp(bes[i]->id, id)) { > + break; > + } > + } > + > + return bes[i]; > +} > + > +/* > + * Walk the list of available TPM backend drivers and display them on the > + * screen. > + */ > +void tpm_display_backend_drivers(void) > +{ > + int i; > + > + fprintf(stderr, "Supported TPM types (choose only one):\n"); > + > + for (i = 0; bes[i] != NULL; i++) { > + fprintf(stderr, "%12s %s\n", bes[i]->id, bes[i]->desc()); > + } > + fprintf(stderr, "\n"); > +} > + > +/* > + * Find the TPM with the given Id > + */ > +TPMBackend *qemu_find_tpm(const char *id) > +{ > + TPMBackend *drv; > + > + QLIST_FOREACH(drv, &tpm_backends, list) { > + if (!strcmp(drv->id, id)) { > + return drv; > + } > + } > + > + return NULL; > +} > + > +static int configure_tpm(QemuOpts *opts) > +{ > + const char *value; > + const char *id; > + const TPMDriverOps *be; > + TPMBackend *drv; > + > + if (!QLIST_EMPTY(&tpm_backends)) { > + error_report("Only one TPM is allowed.\n"); > + return 1; > + } A list of tpm_backends is maintained and walked in a few places, but only one is allowed to be added to the list. Will it ever make sense to enable multiple backends at one time? > + > + id = qemu_opts_id(opts); > + if (id == NULL) { > + qerror_report(QERR_MISSING_PARAMETER, "id"); > + return 1; > + } > + > + value = qemu_opt_get(opts, "type"); > + if (!value) { > + qerror_report(QERR_MISSING_PARAMETER, "type"); > + tpm_display_backend_drivers(); > + return 1; > + } > + > + be = tpm_get_backend_driver(value); The "type" value is being passed but the tpm_get_backend_driver() defines the parameter as "id". Maybe "id" could be renamed to "type" for consistency. See similar comment further down in this email. > + if (be == NULL) { > + qerror_report(QERR_INVALID_PARAMETER_VALUE, "type", > + "a TPM backend type"); > + tpm_display_backend_drivers(); > + return 1; > + } > + > + drv = be->create(opts, id); > + if (!drv) { > + return 1; > + } > + > + QLIST_INSERT_HEAD(&tpm_backends, drv, list); > + > + return 0; > +} > + > +static int tpm_init_tpmdev(QemuOpts *opts, void *dummy) > +{ > + return configure_tpm(opts); > +} > + > +/* > + * Walk the list of TPM backend drivers that are in use and call their > + * destroy function to have them cleaned up. > + */ > +void tpm_cleanup(void) > +{ > + TPMBackend *drv, *next; > + > + QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) { > + QLIST_REMOVE(drv, list); > + drv->ops->destroy(drv); > + } > +} > + > +/* > + * Initialize the TPM. Process the tpmdev command line options describing the > + * TPM backend. > + */ > +int tpm_init(void) > +{ > + if (qemu_opts_foreach(qemu_find_opts("tpmdev"), > + tpm_init_tpmdev, NULL, 1) != 0) { > + return -1; > + } > + > + atexit(tpm_cleanup); > + > + return 0; > +} > + > +/* > + * Parse the TPM configuration options. > + * It is possible to pass an option '-tpmdev none' to not activate any TPM. > + * To display all available TPM backends the user may use '-tpmdev ?' > + */ > +int tpm_config_parse(QemuOptsList *opts_list, const char *optarg) > +{ > + QemuOpts *opts; > + > + if (strcmp("none", optarg) != 0) { What's the point of supporting "-tpmdev none"? > + if (*optarg == '?') { > + tpm_display_backend_drivers(); > + return -1; > + } > + opts = qemu_opts_parse(opts_list, optarg, 1); > + if (!opts) { > + return -1; > + } > + } > + return 0; > +} > + > +#endif /* CONFIG_TPM */ > + > +static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv) > +{ > + TPMInfo *res = g_new0(TPMInfo, 1); > + > + res->model = g_strdup(drv->fe_model); > + res->id = g_strdup(drv->id); > + if (drv->path) { > + res->path = g_strdup(drv->path); > + } > + res->type = g_strdup(drv->ops->id); > + > + return res; > +} > + > +/* > + * Walk the list of active TPM backends and collect information about them > + * following the schema description in qapi-schema.json. > + */ > +TPMInfoList *qmp_query_tpm(Error **errp) > +{ > + TPMBackend *drv; > + TPMInfoList *info, *head = NULL, *cur_item = NULL; > + > + QLIST_FOREACH(drv, &tpm_backends, list) { > + info = g_new0(TPMInfoList, 1); > + info->value = qmp_query_tpm_inst(drv); > + > + if (!cur_item) { > + head = cur_item = info; > + } else { > + cur_item->next = info; > + cur_item = info; > + } > + } > + > + return head; > +} > diff --git a/tpm.h b/tpm.h > new file mode 100644 > index 0000000..7fdd1db > --- /dev/null > +++ b/tpm.h > @@ -0,0 +1,81 @@ > +/* > + * TPM configuration > + * > + * Copyright (C) 2011 IBM Corporation > + * > + * Authors: > + * Stefan Berger > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > +#ifndef QEMU_TPM_H > +#define QEMU_TPM_H > + > +#include "memory.h" > +#include "hw/tpm_tis.h" > + > +struct TPMDriverOps; > +typedef struct TPMDriverOps TPMDriverOps; > + > +typedef struct TPMBackend { > + char *id; For consistency, this could be named "type" instead of "id" since it corresponds to -tpmdev's type. > + const char *fe_model; > + char *path; > + const TPMDriverOps *ops; > + > + QLIST_ENTRY(TPMBackend) list; > +} TPMBackend; > + > +/* overall state of the TPM interface */ > +typedef struct TPMState { > + ISADevice busdev; > + MemoryRegion mmio; > + > + union { > + TPMTISState tis; > + } s; > + > + uint8_t command_locty; It would be easier to read if locality was spelled out fully, instead of locty. But if that causes lines to be too long then maybe it's not worth it. > + TPMLocality *cmd_locty; There's a cmd_locty and a command_locty. command_locty is the locality number and cmd_locty is the locality data. Could these variable names be updated to be more unique and descriptive? > + > + char *backend; > + TPMBackend *be_driver; > +} TPMState; > + > +typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty); > + > +struct TPMDriverOps { > + const char *id; > + /* get a descriptive text of the backend to display to the user */ > + const char *(*desc)(void); > + > + TPMBackend *(*create)(QemuOpts *opts, const char *id); > + void (*destroy)(TPMBackend *t); > + > + /* initialize the backend */ > + int (*init)(TPMBackend *t, TPMState *s, TPMRecvDataCB *datacb); > + /* start up the TPM on the backend */ > + int (*startup_tpm)(TPMBackend *t); > + /* returns true if nothing will ever answer TPM requests */ > + bool (*had_startup_error)(TPMBackend *t); > + > + size_t (*realloc_buffer)(TPMSizedBuffer *sb); > + > + void (*deliver_request)(TPMBackend *t); > + > + void (*reset)(TPMBackend *t); > + > + bool (*get_tpm_established_flag)(TPMBackend *t); > +}; > + > +#define TPM_DEFAULT_DEVICE_MODEL "tpm-tis" > + > +int tpm_config_parse(QemuOptsList *opts_list, const char *optarg); > +int tpm_init(void); > +void tpm_cleanup(void); > +TPMBackend *qemu_find_tpm(const char *id); > +void tpm_display_backend_drivers(void); > +const TPMDriverOps *tpm_get_backend_driver(const char *id); > + > +#endif /* QEMU_TPM_H */ > diff --git a/vl.c b/vl.c > index 23ab3a3..3e784f5 100644 > --- a/vl.c > +++ b/vl.c > @@ -139,6 +139,7 @@ int main(int argc, char **argv) > #include "block.h" > #include "blockdev.h" > #include "block-migration.h" > +#include "tpm.h" > #include "dma.h" > #include "audio/audio.h" > #include "migration.h" > @@ -2674,6 +2675,13 @@ int main(int argc, char **argv, char **envp) > ram_size = value; > break; > } > +#ifdef CONFIG_TPM > + case QEMU_OPTION_tpmdev: > + if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) { > + exit(1); > + } > + break; > +#endif > case QEMU_OPTION_mempath: > mem_path = optarg; > break; > @@ -3405,6 +3413,12 @@ int main(int argc, char **argv, char **envp) > exit(1); > } > > +#ifdef CONFIG_TPM > + if (tpm_init() < 0) { > + exit(1); > + } > +#endif > + > /* init the bluetooth world */ > if (foreach_device_config(DEV_BT, bt_parse)) > exit(1); > @@ -3657,6 +3671,9 @@ int main(int argc, char **argv, char **envp) > pause_all_vcpus(); > net_cleanup(); > res_free(); > +#ifdef CONFIG_TPM > + tpm_cleanup(); > +#endif > > return 0; > } > -- Regards, Corey Bryant