[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V8 13/14] Add a TPM backend null driver implemen
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH V8 13/14] Add a TPM backend null driver implementation |
Date: |
Thu, 1 Sep 2011 20:40:28 +0300 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- Re: [Qemu-devel] [PATCH V8 13/14] Add a TPM backend null driver implementation,
Michael S. Tsirkin <=