Re: [Qemu-devel] [PATCH V8 13/14] Add a TPM backend null driver implementation

[Michael S. Tsirkin]

On Wed, Aug 31, 2011 at 10:36:04AM -0400, Stefan Berger wrote:
> This patch adds a TPM null driver implementation acting as a backend for
> the TIS hardware emulation. The NULL driver responds to all commands with
> a TPM fault response.
> 
> To use this null driver, use either
> 
> -tpm null
> 
> or
> 
> -tpmdev null,id=tpm0 -device tpm-tis,tpmdev=tpm0
> 
> as parameters on the command line.
> 
> If TPM support is chosen via './configure --enable-tpm ...' TPM support is now
> always compiled into Qemu and at least the null driver will be available on
> emulators for x86_64 and i386.

Why limit this to intel platforms then?

> v8:
>  - initializing 'in' variable
> 
> Signed-off-by: Stefan Berger <address@hidden>
> 
> ---
>  Makefile.target |    2 
>  configure       |    8 -
>  hw/tpm_null.c   |  327 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx |   13 +-
>  tpm.c           |    1 
>  tpm.h           |    1 
>  6 files changed, 341 insertions(+), 11 deletions(-)
> 
> Index: qemu-git/hw/tpm_null.c
> ===================================================================
> --- /dev/null
> +++ qemu-git/hw/tpm_null.c
> @@ -0,0 +1,327 @@
> +/*
> + *  builtin 'null' TPM driver

Is this the same one an earlier patch removed?

> + *
> + *  Copyright (c) 2010, 2011 IBM Corporation
> + *  Copyright (c) 2010, 2011 Stefan Berger

Why dual copyright btw?

> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>
> + */
> +
> +#include "qemu-common.h"
> +#include "tpm.h"
> +#include "hw/hw.h"
> +#include "hw/tpm_tis.h"
> +#include "hw/pc.h"
> +
> +
> +//#define DEBUG_TPM
> +//#define DEBUG_TPM_SR /* suspend - resume */

don't use C++ comments please.

> +
> +
> +/* data structures */
> +
> +typedef struct ThreadParams {
> +    TPMState *tpm_state;
> +
> +    TPMRecvDataCB *recv_data_callback;
> +} ThreadParams;
> +
> +
> +/* local variables */
> +
> +static QemuThread thread;
> +
> +static bool thread_terminate;
> +static bool thread_running;
> +
> +static ThreadParams tpm_thread_params;

this lacks consistency in naming.
prefix everything with tpm_null?

> +
> +static const unsigned char tpm_std_fatal_error_response[10] = {
> +    0x00, 0xc4, 0x00, 0x00, 0x00, 0x0A, 0x00, 0x00, 0x00, 0x09 /* TPM_FAIL */
> +};
> +
> +static char dev_description[80];

Use symbolic constants above?

> +
> +
> +static void *tpm_null_main_loop(void *d)
> +{
> +    ThreadParams *thr_parms = d;
> +    uint32_t in_len;
> +    uint8_t *in, *out;
> +    uint8_t locty;
> +
> +#ifdef DEBUG_TPM
> +    fprintf(stderr, "tpm: THREAD IS STARTING\n");
> +#endif
> +
> +    /* start command processing */
> +    while (!thread_terminate) {
> +        /* receive and handle commands */
> +        in_len = 0;
> +        do {
> +#ifdef DEBUG_TPM
> +            fprintf(stderr, "tpm: waiting for commands...\n");
> +#endif
> +
> +            if (thread_terminate) {
> +                break;
> +            }
> +
> +            qemu_mutex_lock(&thr_parms->tpm_state->state_lock);
> +
> +            /* in case we were to slow and missed the signal, the
> +               to_tpm_execute boolean tells us about a pending command */
> +            if (!thr_parms->tpm_state->to_tpm_execute) {
> +                qemu_cond_wait(&thr_parms->tpm_state->to_tpm_cond,
> +                               &thr_parms->tpm_state->state_lock);
> +            }
> +
> +            thr_parms->tpm_state->to_tpm_execute = false;
> +
> +            qemu_mutex_unlock(&thr_parms->tpm_state->state_lock);
> +
> +            if (thread_terminate) {
> +                break;
> +            }
> +
> +            locty = thr_parms->tpm_state->command_locty;
> +
> +            in = thr_parms->tpm_state->loc[locty].w_buffer.buffer;
> +            in_len = thr_parms->tpm_state->loc[locty].w_offset;
> +
> +            out = thr_parms->tpm_state->loc[locty].r_buffer.buffer;
> +
> +            memcpy(out, tpm_std_fatal_error_response,
> +                   sizeof(tpm_std_fatal_error_response));
> +
> +            out[1] = (in_len > 2 && in[1] >= 0xc1 && in[1] <= 0xc3)
> +                   ? in[1] + 3
> +                   : 0xc4;
> +#ifdef DEBUG_TPM
> +            fprintf(stderr, "tpm_null: sending fault response to VM\n");
> +#endif
> +            thr_parms->recv_data_callback(thr_parms->tpm_state, locty);
> +        } while (in_len > 0);
> +    }
> +
> +#ifdef DEBUG_TPM
> +    fprintf(stderr, "tpm: THREAD IS ENDING\n");
> +#endif
> +
> +    thread_running = false;
> +
> +    return NULL;
> +}
> +
> +
> +static void tpm_null_terminate_tpm_thread(void)
> +{
> +    if (!thread_running) {
> +        return;
> +    }
> +
> +#if defined DEBUG_TPM || defined DEBUG_TPM_SR
> +    fprintf(stderr, "tpm: TERMINATING RUNNING TPM THREAD\n");
> +#endif
> +
> +    if (!thread_terminate) {
> +        thread_terminate = true;
> +
> +        qemu_mutex_lock(&tpm_thread_params.tpm_state->state_lock);
> +        qemu_cond_signal(&tpm_thread_params.tpm_state->to_tpm_cond);
> +        qemu_mutex_unlock(&tpm_thread_params.tpm_state->state_lock);
> +
> +        memset(&thread, 0, sizeof(thread));
> +    }
> +}
> +
> +
> +static void tpm_null_tpm_atexit(void)
> +{
> +    tpm_null_terminate_tpm_thread();
> +}
> +
> +
> +/**
> + * Start the TPM (thread). If it had been started before, then terminate
> + * and start it again.
> + */
> +static int tpm_null_startup_tpm(void)
> +{
> +    /* terminate a running TPM */
> +    tpm_null_terminate_tpm_thread();
> +
> +    /* reset the flag so the thread keeps on running */
> +    thread_terminate = false;
> +
> +    qemu_thread_create(&thread, tpm_null_main_loop, &tpm_thread_params);
> +
> +    thread_running = true;
> +
> +    return 0;
> +}
> +
> +
> +static int tpm_null_do_startup_tpm(void)
> +{
> +    return tpm_null_startup_tpm();
> +}
> +
> +
> +static int tpm_null_early_startup_tpm(void)
> +{
> +    return tpm_null_do_startup_tpm();
> +}
> +
> +
> +static int tpm_null_late_startup_tpm(void)
> +{
> +    return tpm_null_do_startup_tpm();
> +}
> +
> +
> +static void tpm_null_reset(void)
> +{
> +#if defined DEBUG_TPM || defined DEBUG_TPM_SR
> +    fprintf(stderr, "tpm: CALL TO TPM_RESET!\n");
> +#endif
> +
> +    tpm_null_terminate_tpm_thread();
> +}
> +
> +
> +/*
> + * Since the null driver does not have much persistent storage
> + * there is not much to do here...
> + */
> +static int tpm_null_instantiate_with_volatile_data(TPMState *s)
> +{
> +    if (thread_running) {
> +#ifdef DEBUG_TPM_SR
> +        fprintf(stderr, "tpm: This is resume of a SNAPSHOT\n");
> +#endif
> +        tis_reset_for_snapshot_resume(s);
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int tpm_null_init(TPMState *s, TPMRecvDataCB *recv_data_cb)
> +{
> +    tpm_thread_params.tpm_state = s;
> +    tpm_thread_params.recv_data_callback = recv_data_cb;
> +
> +    atexit(tpm_null_tpm_atexit);
> +
> +    return 0;
> +}
> +
> +
> +static bool tpm_null_get_tpm_established_flag(void)
> +{
> +    return false;
> +}
> +
> +
> +static bool tpm_null_get_startup_error(void)
> +{
> +    return false;
> +}
> +
> +
> +/**
> + * This function is called by tpm_tis.c once the TPM has processed
> + * the last command and returned the response to the TIS.
> + */
> +static int tpm_null_save_volatile_data(void)
> +{
> +    return 0;
> +}
> +
> +
> +static size_t tpm_null_realloc_buffer(TPMSizedBuffer *sb)
> +{
> +    size_t wanted_size = 4096;
> +
> +    if (sb->size != wanted_size) {
> +        sb->buffer = g_realloc(sb->buffer, wanted_size);
> +        if (sb->buffer != NULL) {
> +            sb->size = wanted_size;
> +        } else {
> +            sb->size = 0;
> +        }
> +    }
> +    return sb->size;
> +}
> +
> +
> +static const char *tpm_null_create_desc(void)
> +{
> +    static int done;
> +
> +    if (!done) {
> +        snprintf(dev_description, sizeof(dev_description),
> +                 "Null TPM backend driver");
> +        done = 1;
> +    }
> +
> +    return dev_description;
> +}
> +
> +
> +static TPMBackend *tpm_null_create(QemuOpts *opts, const char *id,
> +                                      const char *model)
> +{
> +    TPMBackend *driver;
> +
> +    driver = g_malloc(sizeof(TPMBackend));
> +    if (!driver) {
> +        fprintf(stderr, "Could not allocate memory.\n");
> +        return NULL;
> +    }
> +    driver->id = g_strdup(id);
> +    if (model) {
> +        driver->model = g_strdup(model);
> +    }
> +    driver->ops = &tpm_null_driver;
> +
> +    return driver;
> +}
> +
> +
> +static void tpm_null_destroy(TPMBackend *driver)
> +{
> +    g_free(driver->id);
> +    g_free(driver->model);
> +    g_free(driver);
> +}
> +
> +
> +TPMDriverOps tpm_null_driver = {
> +    .id                       = "null",
> +    .desc                     = tpm_null_create_desc,
> +    .job_for_main_thread      = NULL,
> +    .create                   = tpm_null_create,
> +    .destroy                  = tpm_null_destroy,
> +    .init                     = tpm_null_init,
> +    .early_startup_tpm        = tpm_null_early_startup_tpm,
> +    .late_startup_tpm         = tpm_null_late_startup_tpm,
> +    .realloc_buffer           = tpm_null_realloc_buffer,
> +    .reset                    = tpm_null_reset,
> +    .had_startup_error        = tpm_null_get_startup_error,
> +    .save_volatile_data       = tpm_null_save_volatile_data,
> +    .load_volatile_data       = tpm_null_instantiate_with_volatile_data,
> +    .get_tpm_established_flag = tpm_null_get_tpm_established_flag,
> +};
> Index: qemu-git/Makefile.target
> ===================================================================
> --- qemu-git.orig/Makefile.target
> +++ qemu-git/Makefile.target
> @@ -233,7 +233,7 @@ obj-i386-y += debugcon.o multiboot.o
>  obj-i386-y += pc_piix.o
>  obj-i386-$(CONFIG_KVM) += kvmclock.o
>  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> -obj-i386-$(CONFIG_TPM) += tpm_tis.o sha1.o
> +obj-i386-$(CONFIG_TPM) += tpm_tis.o sha1.o tpm_null.o
>  obj-i386-$(CONFIG_TPM_BUILTIN) += tpm_builtin.o
>  
>  ifdef CONFIG_TPM_BUILTIN
> Index: qemu-git/tpm.c
> ===================================================================
> --- qemu-git.orig/tpm.c
> +++ qemu-git/tpm.c
> @@ -24,6 +24,7 @@
>  #if defined(TARGET_I386) || defined(TARGET_X86_64)
>  
>  static const TPMDriverOps *bes[] = {
> +    &tpm_null_driver,
>  #ifdef CONFIG_TPM_BUILTIN
>      &tpm_builtin,
>  #endif
> Index: qemu-git/tpm.h
> ===================================================================
> --- qemu-git.orig/tpm.h
> +++ qemu-git/tpm.h
> @@ -141,6 +141,7 @@ void tpm_measure_buffer(const void *buff
>                          TPMMeasureType type, uint8_t pcrindex,
>                          const void *data, uint32_t data_len);
>  
> +extern TPMDriverOps tpm_null_driver;
>  extern TPMDriverOps tpm_builtin;
>  
>  #endif /* _HW_TPM_CONFIG_H */
> Index: qemu-git/qemu-options.hx
> ===================================================================
> --- qemu-git.orig/qemu-options.hx
> +++ qemu-git/qemu-options.hx
> @@ -1769,6 +1769,8 @@ DEF("tpm", HAS_ARG, QEMU_OPTION_tpm, \
>      "-tpm builtin,path=<path>[,model=<model>][,key=<aes key>]\n" \
>      "                enable a builtin TPM with state in file in path\n" \
>      "                and encrypt the TPM's state with the given AES key\n" \
> +    "-tpm null       enable a TPM null driver that responds with a fault\n" \
> +    "                message to every TPM request\n" \
>      "-tpm model=?    to list available TPM device models\n" \
>      "-tpm ?          to list available TPM backend types\n",
>      QEMU_ARCH_I386)
> @@ -1784,8 +1786,9 @@ The general form of a TPM device option 
>  
>  @item -tpmdev @var{backend} ,address@hidden [,@var{options}]
>  @findex -tpmdev
> -Backend type must be:
> address@hidden
> +Backend type must be one of:
> address@hidden,
> address@hidden
>  
>  The specific backend type will determine the applicable options.
>  The @code{-tpmdev} options requires a @code{-device} option.
> @@ -1826,6 +1829,12 @@ using AES-CBC encryption scheme supply t
>  @example
>  -tpmdev 
> builtin,id=tpm0,path=<path_to_qcow2>,key=aes-cbc:0x1234567890abcdef01234567890abcdef
>  -device tpm-tis,tpmdev=tpm0
>  @end example
> +
> address@hidden -tpmdev null
> +
> +Creates an instance of a TPM null driver that responds to every command
> +with a fault message.
> +
>  @end table
>  
>  The short form of a TPM device option is:
> Index: qemu-git/configure
> ===================================================================
> --- qemu-git.orig/configure
> +++ qemu-git/configure
> @@ -2593,8 +2593,6 @@ EOF
>    libtpms=no
>    if compile_prog "" "-ltpms" ; then
>      libtpms=yes
> -  else
> -    tpm_need_pkgs="libtpms development package"
>    fi
>  fi
>  
> @@ -3598,12 +3596,6 @@ if test "$tpm" = "yes"; then
>    if test "$has_tpm" = "1"; then
>        if test "$libtpms" = "yes" ; then
>            echo "CONFIG_TPM_BUILTIN=y" >> $config_target_mak
> -      else
> -          echo
> -          echo "TPM support cannot be added since no TPM backend can be 
> compiled."
> -          echo "Please install the $tpm_need_pkgs."
> -          echo
> -          exit 1
>        fi
>        echo "CONFIG_TPM=y" >> $config_host_mak
>    fi

Hmm, so now we remove some code added earlier?
Pls don't do this for new code: this makes review harder, not easier.
It makes sense for old code to ensure bisectability.

-- 
MST