Patchwork [V13,1/7] Support for TPM command line options

login
register
mail settings
Submitter Stefan Berger
Date Dec. 12, 2011, 7:12 p.m.
Message ID <1323717136-21661-2-git-send-email-stefanb@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/130947/
State New
Headers show

Comments

Stefan Berger - Dec. 12, 2011, 7:12 p.m.
This patch adds support for TPM command line options.
The command line options supported here are

./qemu-... -tpmdev passthrough,path=<path to TPM device>,id=<id>
           -device tpm-tis,tpmdev=<id>

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 <stefanb@linux.vnet.ibm.com>

---

v12:
 - use all 4 bytes of the message length indicator

v10:
 - tpm_display_backend_drivers always prints to stderr

v9:
 - prefixing all functions with tpm_tis_ and all constants with TPM_TIS_
 - splitting off of -tpm command line support into its own patch
 - only support TPM passthrough for now

v8:
 - adjusting formatting of backend drivers output to accomodate better
   formatting of 'passthrough' backend output

v6:
 - use #idef CONFIG_TPM to surround TPM calls
 - use QLIST_FOREACH_SAFE rather than QLIST_FOREACH in tpm_cleanup
 - commented backend ops in tpm.h
 - moving to IRQ 5 (11 collided with network cards)

v5:
 - fixing typo reported by Serge Hallyn
 - Adapting code to split command line parameters supporting
   -tpmdev ... -device tpm-tis,tpmdev=...
 - moved code out of arch_init.c|h into tpm.c|h
 - increasing reserved memory for ACPI tables to 128kb (from 64kb)
 - the backend interface has a create() function for interpreting the command
   line parameters and returning a TPMDevice structure; previoulsy
   this function was called handle_options()
 - the backend interface has a destroy() function for cleaning up after
   the create() function was called
 - added support for 'info tpm' in monitor

v4:
 - coding style fixes

v3:
 - added hw/tpm_tis.h to this patch so Qemu compiles at this stage
---
 hmp-commands.hx |    2 +
 hw/tpm_tis.h    |   91 ++++++++++++++++++++++++++++++
 monitor.c       |   10 +++
 qemu-config.c   |   20 +++++++
 qemu-options.hx |   34 +++++++++++
 tpm.c           |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tpm.h           |   90 +++++++++++++++++++++++++++++
 vl.c            |   15 +++++
 8 files changed, 429 insertions(+), 0 deletions(-)
 create mode 100644 hw/tpm_tis.h
 create mode 100644 tpm.c
 create mode 100644 tpm.h
Anthony Liguori - Dec. 12, 2011, 11:16 p.m.
On 12/12/2011 01:12 PM, Stefan Berger wrote:
> This patch adds support for TPM command line options.
> The command line options supported here are
>
> ./qemu-... -tpmdev passthrough,path=<path to TPM device>,id=<id>
>             -device tpm-tis,tpmdev=<id>
>
> 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<stefanb@linux.vnet.ibm.com>
>
> ---
>
> v12:
>   - use all 4 bytes of the message length indicator
>
> v10:
>   - tpm_display_backend_drivers always prints to stderr
>
> v9:
>   - prefixing all functions with tpm_tis_ and all constants with TPM_TIS_
>   - splitting off of -tpm command line support into its own patch
>   - only support TPM passthrough for now
>
> v8:
>   - adjusting formatting of backend drivers output to accomodate better
>     formatting of 'passthrough' backend output
>
> v6:
>   - use #idef CONFIG_TPM to surround TPM calls
>   - use QLIST_FOREACH_SAFE rather than QLIST_FOREACH in tpm_cleanup
>   - commented backend ops in tpm.h
>   - moving to IRQ 5 (11 collided with network cards)
>
> v5:
>   - fixing typo reported by Serge Hallyn
>   - Adapting code to split command line parameters supporting
>     -tpmdev ... -device tpm-tis,tpmdev=...
>   - moved code out of arch_init.c|h into tpm.c|h
>   - increasing reserved memory for ACPI tables to 128kb (from 64kb)
>   - the backend interface has a create() function for interpreting the command
>     line parameters and returning a TPMDevice structure; previoulsy
>     this function was called handle_options()
>   - the backend interface has a destroy() function for cleaning up after
>     the create() function was called
>   - added support for 'info tpm' in monitor
>
> v4:
>   - coding style fixes
>
> v3:
>   - added hw/tpm_tis.h to this patch so Qemu compiles at this stage
> ---
>   hmp-commands.hx |    2 +
>   hw/tpm_tis.h    |   91 ++++++++++++++++++++++++++++++
>   monitor.c       |   10 +++
>   qemu-config.c   |   20 +++++++
>   qemu-options.hx |   34 +++++++++++
>   tpm.c           |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   tpm.h           |   90 +++++++++++++++++++++++++++++
>   vl.c            |   15 +++++
>   8 files changed, 429 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 fdbed15..4506f56 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1352,6 +1352,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/hw/tpm_tis.h b/hw/tpm_tis.h
> new file mode 100644
> index 0000000..e6a9a4b
> --- /dev/null
> +++ b/hw/tpm_tis.h
> @@ -0,0 +1,91 @@
> +/*
> + * tpm_tis.c - QEMU's TPM TIS interface emulator
> + *
> + * Copyright (C) 2006,2010,2011 IBM Corporation
> + *
> + * Authors:
> + *  Stefan Berger<stefanb@us.ibm.com>
> + *  David Safford<safford@us.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + *
> + *
> + * Implementation of the TIS interface according to specs found at
> + * http://www.trustedcomputiggroup.org
> + *
> + */
> +#ifndef _HW_TPM_TIS_H
> +#define _HW_TPM_TIS_H

Macros shouldn't start with '_' as that namespace is reserved in C.

> +#include "isa.h"
> +#include "qemu-thread.h"
> +
> +#include<stdint.h>

Just include "qemu-common.h"

> +#define TPM_TIS_ADDR_BASE           0xFED40000
> +
> +#define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
> +#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;
> +
> +enum tpm_tis_state {
> +    TPM_TIS_STATE_IDLE = 0,
> +    TPM_TIS_STATE_READY,
> +    TPM_TIS_STATE_COMPLETION,
> +    TPM_TIS_STATE_EXECUTION,
> +    TPM_TIS_STATE_RECEPTION,
> +};

Typed enums still need to be CamelCase and typedefed.

> +/* locality data  -- all fields are persisted */
> +typedef struct TPMLocality {
> +    enum tpm_tis_state state;
> +    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 {
> +    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;
> +
> +/* utility functions */
> +
> +static inline uint32_t tpm_tis_get_size_from_buffer(const TPMSizedBuffer *sb)
> +{
> +    return (sb->buffer[2]<<  24) |
> +           (sb->buffer[3]<<  16) |
> +           (sb->buffer[4]<<   8) |
> +            sb->buffer[5];
> +}

I would strongly suggest not making this static inline.

> +
> +#endif /* _HW_TPM_TIS_H */
> diff --git a/monitor.c b/monitor.c
> index 7334401..28873c3 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"
> @@ -2735,6 +2736,15 @@ static mon_cmd_t info_cmds[] = {
>           .help       = "show available trace-events&  their state",
>           .mhandler.info = do_trace_print_events,
>       },
> +#if defined(CONFIG_TPM)
> +    {
> +        .name       = "tpm",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the TPM device",
> +        .mhandler.info = do_info_tpm,
> +    },
> +#endif

Please don't make monitor commands conditional.  Make it fail in a predictable 
fashion if tpm isn't configured.

>       {
>           .name       = NULL,
>       },
> diff --git a/qemu-config.c b/qemu-config.c
> index 18f3020..cc4c31d 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -549,6 +549,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,
> @@ -563,6 +582,7 @@ static QemuOptsList *vm_config_groups[32] = {
>       &qemu_option_rom_opts,
>       &qemu_machine_opts,
>       &qemu_boot_opts,
> +&qemu_tpmdev_opts,

I assume this is my mailer or is the whitespace munged here?

>       NULL,
>   };
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b3db10c..58a5dfa 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1903,6 +1903,40 @@ ETEXI
>
>   DEFHEADING()
>
> +DEFHEADING(TPM device options:)
> +
> +#ifndef _WIN32
> +# ifdef CONFIG_TPM
> +DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
> +    "-tpmdev [<type>],id=str[,option][,option][,...]\n",
> +    QEMU_ARCH_ALL)
> +# endif
> +#endif
> +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()
> +
>   DEFHEADING(Linux/Multiboot boot specific:)
>   STEXI
>
> diff --git a/tpm.c b/tpm.c
> new file mode 100644
> index 0000000..fcab023
> --- /dev/null
> +++ b/tpm.c
> @@ -0,0 +1,167 @@
> +/*
> + * TPM configuration
> + *
> + * Copyright (C) 2011 IBM Corporation
> + *
> + * Authors:
> + *  Stefan Berger<stefanb@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
>

v2 or later please.

> + * Based on net.c
> + */
> +#include "config.h"
> +
> +#include "tpm.h"
> +#include "monitor.h"
> +#include "qerror.h"
> +
> +static const TPMDriverOps *bes[] = {
> +    NULL,
> +};
> +
> +static QLIST_HEAD(, TPMBackend) tpm_backends =
> +    QLIST_HEAD_INITIALIZER(tpm_backends);
> +
> +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];
> +}
> +
> +void tpm_display_backend_drivers(void)
> +{
> +    int i;
> +
> +    fprintf(stderr, "Supported TPM types (choose only one):\n");

Having fprintfs to stderr is a sign something is wrong.

In this case, we should have a programatic way to query the backends (like via 
qmp_query_tpm) and then in the option handling code, we should use that function 
and print to the screen from there.

> +    for (i = 0; bes[i] != NULL; i++) {
> +        fprintf(stderr, "%12s   %s\n", bes[i]->id, bes[i]->desc());
> +    }
> +    fprintf(stderr, "\n");
> +}
> +
> +TPMBackend *qemu_find_tpm(const char *id)
> +{
> +    TPMBackend *drv;
> +
> +    QLIST_FOREACH(drv,&tpm_backends, list) {
> +        if (!strcmp(drv->id, id)) {
> +            return drv;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void do_info_tpm(Monitor *mon)
> +{
> +    TPMBackend *drv;
> +    unsigned int c = 0;
> +
> +    monitor_printf(mon, "TPM device:\n");
> +
> +    QLIST_FOREACH(drv,&tpm_backends, list) {
> +        monitor_printf(mon, " tpm%d: model=%s\n",
> +                       c, drv->fe_model);
> +        monitor_printf(mon, "  \\ %s: type=%s%s%s\n",
> +                       drv->id, drv->ops->id,
> +                       drv->parameters ? "," : "",
> +                       drv->parameters ? drv->parameters : "");
> +        c++;
> +    }
> +}

We should do this through sure QAPI now that it's in the the tree with a proper 
schema entry and an implementation in hmp.c.

> +
> +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;
> +    }
> +
> +    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);
> +    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);
> +}
> +
> +void tpm_cleanup(void)
> +{
> +    TPMBackend *drv, *next;
> +
> +    QLIST_FOREACH_SAFE(drv,&tpm_backends, list, next) {
> +        QLIST_REMOVE(drv, list);
> +        drv->ops->destroy(drv);
> +    }
> +}
> +
> +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;
> +}
> +
> +void tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
> +{
> +    QemuOpts *opts;
> +
> +    if (strcmp("none", optarg) != 0) {
> +        if (*optarg == '?') {
> +            tpm_display_backend_drivers();
> +            exit(0);

Don't exit from something other than vl.c.  Return an error code and let vl.c exit.

> +        }
> +        opts = qemu_opts_parse(opts_list, optarg, 1);
> +        if (!opts) {
> +            exit(1);
> +        }
> +    }
> +}
> diff --git a/tpm.h b/tpm.h
> new file mode 100644
> index 0000000..85c2a35
> --- /dev/null
> +++ b/tpm.h
> @@ -0,0 +1,90 @@

Needs a copyright.

> +#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;
> +    const char *fe_model;
> +    char *parameters;
> +    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;
> +    TPMLocality *cmd_locty;
> +
> +    QemuMutex state_lock;
> +    QemuCond  from_tpm_cond;
> +    QemuCond  to_tpm_cond;
> +    bool      to_tpm_execute;
> +
> +    bool      tpm_initialized;
> +
> +    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 (*reset)(TPMBackend *t);
> +
> +    bool (*get_tpm_established_flag)(TPMBackend *t);
> +};
> +
> +static inline void tpm_dump_buffer(FILE *stream,
> +                                   unsigned char *buffer, unsigned int len)
> +{
> +    int i;
> +
> +    for (i = 0; i<  len; i++) {
> +        if (i&&  !(i % 16)) {
> +            fprintf(stream, "\n");
> +        }
> +        fprintf(stream, "%.2X ", buffer[i]);
> +    }
> +    fprintf(stream, "\n");
> +}

This definitely shouldn't be static inline and it's questionable whether it 
should exist in the first place.

> +#define TPM_DEFAULT_DEVICE_MODEL "tpm-tis"
> +
> +void 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 do_info_tpm(Monitor *mon);
> +void tpm_display_backend_drivers(void);
> +const TPMDriverOps *tpm_get_backend_driver(const char *id);

Please document these functions.

> +#endif /* _QEMU_TPM_H */
> diff --git a/vl.c b/vl.c
> index 5372a96..12915d0 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"
> @@ -2550,6 +2551,11 @@ int main(int argc, char **argv, char **envp)
>                   ram_size = value;
>                   break;
>               }
> +#ifdef CONFIG_TPM
> +            case QEMU_OPTION_tpmdev:
> +                tpm_config_parse(qemu_find_opts("tpmdev"), optarg);
> +                break;
> +#endif

Don't make options conditional.

Regards,

Anthony Liguori

>               case QEMU_OPTION_mempath:
>                   mem_path = optarg;
>                   break;
> @@ -3243,6 +3249,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);
> @@ -3487,6 +3499,9 @@ int main(int argc, char **argv, char **envp)
>       pause_all_vcpus();
>       net_cleanup();
>       res_free();
> +#ifdef CONFIG_TPM
> +    tpm_cleanup();
> +#endif
>
>       return 0;
>   }
Stefan Berger - Dec. 13, 2011, 2:16 a.m.
On 12/12/2011 06:16 PM, Anthony Liguori wrote:
> On 12/12/2011 01:12 PM, Stefan Berger wrote:
>> @@ -2735,6 +2736,15 @@ static mon_cmd_t info_cmds[] = {
>>           .help       = "show available trace-events&  their state",
>>           .mhandler.info = do_trace_print_events,
>>       },
>> +#if defined(CONFIG_TPM)
>> +    {
>> +        .name       = "tpm",
>> +        .args_type  = "",
>> +        .params     = "",
>> +        .help       = "show the TPM device",
>> +        .mhandler.info = do_info_tpm,
>> +    },
>> +#endif
>
> Please don't make monitor commands conditional.  Make it fail in a 
> predictable fashion if tpm isn't configured.
>

This will then require tpm.c to always be compiled. You'll find the 
CONFIG_TPM there then.
@@ -563,6 +582,7 @@ static QemuOptsList *vm_config_groups[32] = {
>> &qemu_option_rom_opts,
>> &qemu_machine_opts,
>> &qemu_boot_opts,
>> +&qemu_tpmdev_opts,
>
> I assume this is my mailer or is the whitespace munged here?
>

Must be your mailer.@@ -0,0 +1,167 @@
>> +/*
>> + * TPM configuration
>> + *
>> + * Copyright (C) 2011 IBM Corporation
>> + *
>> + * Authors:
>> + *  Stefan Berger<stefanb@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  
>> See
>> + * the COPYING file in the top-level directory.
>>
>
> v2 or later please.

ok.

>> +
>> +void tpm_display_backend_drivers(void)
>> +{
>> +    int i;
>> +
>> +    fprintf(stderr, "Supported TPM types (choose only one):\n");
>
> Having fprintfs to stderr is a sign something is wrong.
>
> In this case, we should have a programatic way to query the backends 
> (like via qmp_query_tpm) and then in the option handling code, we 
> should use that function and print to the screen from there.
>

Will try to convert but keep this function ?

>> +
>> +void do_info_tpm(Monitor *mon)
>> +{
>> +    TPMBackend *drv;
>> +    unsigned int c = 0;
>> +
>> +    monitor_printf(mon, "TPM device:\n");
>> +
>> +    QLIST_FOREACH(drv,&tpm_backends, list) {
>> +        monitor_printf(mon, " tpm%d: model=%s\n",
>> +                       c, drv->fe_model);
>> +        monitor_printf(mon, "  \\ %s: type=%s%s%s\n",
>> +                       drv->id, drv->ops->id,
>> +                       drv->parameters ? "," : "",
>> +                       drv->parameters ? drv->parameters : "");
>> +        c++;
>> +    }
>> +}
>
> We should do this through sure QAPI now that it's in the the tree with 
> a proper schema entry and an implementation in hmp.c.

True...

>> +void tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
>> +{
>> +    QemuOpts *opts;
>> +
>> +    if (strcmp("none", optarg) != 0) {
>> +        if (*optarg == '?') {
>> +            tpm_display_backend_drivers();
>> +            exit(0);
>
> Don't exit from something other than vl.c.  Return an error code and 
> let vl.c exit.

I implemented this following along the lines of

qemu-system-x86_64 -soundhw ?

which then also shows and error code 0. What error code should it return 
to the shell ?


>
>> +        }
>> +        opts = qemu_opts_parse(opts_list, optarg, 1);
>> +        if (!opts) {
>> +            exit(1);
>> +        }
>> +    }
>> +}
>> diff --git a/tpm.h b/tpm.h
>> new file mode 100644
>> index 0000000..85c2a35
>> --- /dev/null
>> +++ b/tpm.h
>> @@ -0,0 +1,90 @@
>
> Needs a copyright.

Ok.

>> +static inline void tpm_dump_buffer(FILE *stream,
>> +                                   unsigned char *buffer, unsigned 
>> int len)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i<  len; i++) {
>> +        if (i&&  !(i % 16)) {
>> +            fprintf(stream, "\n");
>> +        }
>> +        fprintf(stream, "%.2X ", buffer[i]);
>> +    }
>> +    fprintf(stream, "\n");
>> +}
>
> This definitely shouldn't be static inline and it's questionable 
> whether it should exist in the first place.

Do you have an alternative for this function? Assuming it's useful for 
debugging, should I just move it into tpm.c ?


>
>> +#define TPM_DEFAULT_DEVICE_MODEL "tpm-tis"
>> +
>> +void 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 do_info_tpm(Monitor *mon);
>> +void tpm_display_backend_drivers(void);
>> +const TPMDriverOps *tpm_get_backend_driver(const char *id);
>
> Please document these functions.

Will document them in tpm.c where their implementation is.


@@ -2550,6 +2551,11 @@ int main(int argc, char **argv, char **envp)
>>                   ram_size = value;
>>                   break;
>>               }
>> +#ifdef CONFIG_TPM
>> +            case QEMU_OPTION_tpmdev:
>> +                tpm_config_parse(qemu_find_opts("tpmdev"), optarg);
>> +                break;
>> +#endif
>
> Don't make options conditional.

Can I have an #ifdef-#else-#endif construct there along the lines of 
CONFIG_SDL with an exit(1) in the #else branch?


    Stefan

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index fdbed15..4506f56 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1352,6 +1352,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/hw/tpm_tis.h b/hw/tpm_tis.h
new file mode 100644
index 0000000..e6a9a4b
--- /dev/null
+++ b/hw/tpm_tis.h
@@ -0,0 +1,91 @@ 
+/*
+ * tpm_tis.c - QEMU's TPM TIS interface emulator
+ *
+ * Copyright (C) 2006,2010,2011 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger <stefanb@us.ibm.com>
+ *  David Safford <safford@us.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ *
+ * 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-thread.h"
+
+#include <stdint.h>
+
+#define TPM_TIS_ADDR_BASE           0xFED40000
+
+#define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
+#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;
+
+enum tpm_tis_state {
+    TPM_TIS_STATE_IDLE = 0,
+    TPM_TIS_STATE_READY,
+    TPM_TIS_STATE_COMPLETION,
+    TPM_TIS_STATE_EXECUTION,
+    TPM_TIS_STATE_RECEPTION,
+};
+
+/* locality data  -- all fields are persisted */
+typedef struct TPMLocality {
+    enum tpm_tis_state state;
+    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 {
+    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;
+
+/* utility functions */
+
+static inline uint32_t tpm_tis_get_size_from_buffer(const TPMSizedBuffer *sb)
+{
+    return (sb->buffer[2] << 24) |
+           (sb->buffer[3] << 16) |
+           (sb->buffer[4] <<  8) |
+            sb->buffer[5];
+}
+
+#endif /* _HW_TPM_TIS_H */
diff --git a/monitor.c b/monitor.c
index 7334401..28873c3 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"
@@ -2735,6 +2736,15 @@  static mon_cmd_t info_cmds[] = {
         .help       = "show available trace-events & their state",
         .mhandler.info = do_trace_print_events,
     },
+#if defined(CONFIG_TPM)
+    {
+        .name       = "tpm",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the TPM device",
+        .mhandler.info = do_info_tpm,
+    },
+#endif
     {
         .name       = NULL,
     },
diff --git a/qemu-config.c b/qemu-config.c
index 18f3020..cc4c31d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -549,6 +549,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,
@@ -563,6 +582,7 @@  static QemuOptsList *vm_config_groups[32] = {
     &qemu_option_rom_opts,
     &qemu_machine_opts,
     &qemu_boot_opts,
+    &qemu_tpmdev_opts,
     NULL,
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index b3db10c..58a5dfa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1903,6 +1903,40 @@  ETEXI
 
 DEFHEADING()
 
+DEFHEADING(TPM device options:)
+
+#ifndef _WIN32
+# ifdef CONFIG_TPM
+DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
+    "-tpmdev [<type>],id=str[,option][,option][,...]\n",
+    QEMU_ARCH_ALL)
+# endif
+#endif
+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()
+
 DEFHEADING(Linux/Multiboot boot specific:)
 STEXI
 
diff --git a/tpm.c b/tpm.c
new file mode 100644
index 0000000..fcab023
--- /dev/null
+++ b/tpm.c
@@ -0,0 +1,167 @@ 
+/*
+ * TPM configuration
+ *
+ * Copyright (C) 2011 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger    <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Based on net.c
+ */
+#include "config.h"
+
+#include "tpm.h"
+#include "monitor.h"
+#include "qerror.h"
+
+static const TPMDriverOps *bes[] = {
+    NULL,
+};
+
+static QLIST_HEAD(, TPMBackend) tpm_backends =
+    QLIST_HEAD_INITIALIZER(tpm_backends);
+
+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];
+}
+
+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");
+}
+
+TPMBackend *qemu_find_tpm(const char *id)
+{
+    TPMBackend *drv;
+
+    QLIST_FOREACH(drv, &tpm_backends, list) {
+        if (!strcmp(drv->id, id)) {
+            return drv;
+        }
+    }
+
+    return NULL;
+}
+
+void do_info_tpm(Monitor *mon)
+{
+    TPMBackend *drv;
+    unsigned int c = 0;
+
+    monitor_printf(mon, "TPM device:\n");
+
+    QLIST_FOREACH(drv, &tpm_backends, list) {
+        monitor_printf(mon, " tpm%d: model=%s\n",
+                       c, drv->fe_model);
+        monitor_printf(mon, "  \\ %s: type=%s%s%s\n",
+                       drv->id, drv->ops->id,
+                       drv->parameters ? "," : "",
+                       drv->parameters ? drv->parameters : "");
+        c++;
+    }
+}
+
+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;
+    }
+
+    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);
+    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);
+}
+
+void tpm_cleanup(void)
+{
+    TPMBackend *drv, *next;
+
+    QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
+        QLIST_REMOVE(drv, list);
+        drv->ops->destroy(drv);
+    }
+}
+
+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;
+}
+
+void tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+{
+    QemuOpts *opts;
+
+    if (strcmp("none", optarg) != 0) {
+        if (*optarg == '?') {
+            tpm_display_backend_drivers();
+            exit(0);
+        }
+        opts = qemu_opts_parse(opts_list, optarg, 1);
+        if (!opts) {
+            exit(1);
+        }
+    }
+}
diff --git a/tpm.h b/tpm.h
new file mode 100644
index 0000000..85c2a35
--- /dev/null
+++ b/tpm.h
@@ -0,0 +1,90 @@ 
+#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;
+    const char *fe_model;
+    char *parameters;
+    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;
+    TPMLocality *cmd_locty;
+
+    QemuMutex state_lock;
+    QemuCond  from_tpm_cond;
+    QemuCond  to_tpm_cond;
+    bool      to_tpm_execute;
+
+    bool      tpm_initialized;
+
+    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 (*reset)(TPMBackend *t);
+
+    bool (*get_tpm_established_flag)(TPMBackend *t);
+};
+
+static inline void tpm_dump_buffer(FILE *stream,
+                                   unsigned char *buffer, unsigned int len)
+{
+    int i;
+
+    for (i = 0; i < len; i++) {
+        if (i && !(i % 16)) {
+            fprintf(stream, "\n");
+        }
+        fprintf(stream, "%.2X ", buffer[i]);
+    }
+    fprintf(stream, "\n");
+}
+
+#define TPM_DEFAULT_DEVICE_MODEL "tpm-tis"
+
+void 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 do_info_tpm(Monitor *mon);
+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 5372a96..12915d0 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"
@@ -2550,6 +2551,11 @@  int main(int argc, char **argv, char **envp)
                 ram_size = value;
                 break;
             }
+#ifdef CONFIG_TPM
+            case QEMU_OPTION_tpmdev:
+                tpm_config_parse(qemu_find_opts("tpmdev"), optarg);
+                break;
+#endif
             case QEMU_OPTION_mempath:
                 mem_path = optarg;
                 break;
@@ -3243,6 +3249,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);
@@ -3487,6 +3499,9 @@  int main(int argc, char **argv, char **envp)
     pause_all_vcpus();
     net_cleanup();
     res_free();
+#ifdef CONFIG_TPM
+    tpm_cleanup();
+#endif
 
     return 0;
 }