qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V8 01/14] Support for TPM command line options


From: Stefan Berger
Subject: Re: [Qemu-devel] [PATCH V8 01/14] Support for TPM command line options
Date: Thu, 01 Sep 2011 21:01:32 -0400
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.18) Gecko/20110621 Fedora/3.1.11-1.fc14 Lightning/1.0b3pre Thunderbird/3.1.11

On 09/01/2011 01:14 PM, Michael S. Tsirkin wrote:
On Wed, Aug 31, 2011 at 10:35:52AM -0400, Stefan Berger wrote:
This patch adds support for TPM command line options.
The command line supported here (considering the libtpms based
backend) are

./qemu-... -tpm builtin,path=<path to blockstorage file>

and

./qemu-... -tpmdev builtin,path=<path to blockstorage file>,id=<id>
            -device tpm-tis,tpmdev=<id>
do we really need both?
I had chatted with Anthony about this. I am following the existing pattern is use for example for -netdev / -net.

and

./qemu-... -tpmdev ?

where the latter works similar to -soundhw ? and shows a list of
available TPM backends ('builtin').

To show the available TPM models do:

./qemu-... -tpm model=?
Can we live with -tpmdev for backend and plain device_add for frontend?
Can you give a more specific example? Is device_add a function call or a command line parameter in this context?
Frontend would be connected to backend using a tpmdev matching the id
of the frontend...

qemu-... -tpmdev builtin,path=<path to blockstorage file>,id=<id>
         -device tpm-tis,tpmdev=<id>


Isn't that what I am doing?

In case of -tpm, 'type' (above 'builtin') and 'model' are interpreted in tpm.c.
In case of -tpmdev 'type' and 'id' are interpreted in tpm.c
Using the type parameter, the backend is chosen, i.e., 'builtin' for the
libtpms-based builtin TPM. 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.

Since SeaBIOS will now use 128kb for ACPI tables the amount of reserved
memory for ACPI tables needs to be increased -- increasing it to 128kb.
Increasing from which value to which?
From 64kb to 128kb.
Monitor support for 'info tpm' has been added. It for example prints the
following:

TPM devices:
   builtin: model=tpm-tis,id=tpm0
This mixes frontend and backend properties.

There's currently only one frontend 'model' and that's the 'tpm-tis'. In case someone would want to write a virtio equivalent it would show the that the 'builtin' backend is connected to the 'virtio' frontend model. If above is not correct, how should it look like?
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

Signed-off-by: Stefan Berger<address@hidden>

---
  Makefile.target |    1
  hmp-commands.hx |    2
  hw/pc.c         |    7 +
  hw/tpm_tis.h    |   75 +++++++++++++++
  monitor.c       |   10 ++
  qemu-config.c   |   46 +++++++++
  qemu-options.hx |   80 ++++++++++++++++
  tpm.c           |  279 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  tpm.h           |  112 ++++++++++++++++++++++
  vl.c            |   18 +++
  10 files changed, 629 insertions(+), 1 deletion(-)

Index: qemu-git/qemu-options.hx
===================================================================
--- qemu-git.orig/qemu-options.hx
+++ qemu-git/qemu-options.hx
@@ -1760,6 +1760,86 @@ ETEXI

  DEFHEADING()

+DEFHEADING(TPM device options:)
+
+#ifndef _WIN32
+# ifdef CONFIG_TPM
+DEF("tpm", HAS_ARG, QEMU_OPTION_tpm, \
+    "" \
+    "-tpm builtin,path=<path>[,model=<model>]\n" \
+    "                enable a builtin TPM with state in file in path\n" \
+    "-tpm model=?    to list available TPM device models\n" \
+    "-tpm ?          to list available TPM backend types\n",
+    QEMU_ARCH_I386)
+DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
+    "-tpmdev [builtin],id=str[,option][,option][,...]\n",
+    QEMU_ARCH_I386)
+# endif
+#endif
+STEXI
+
+The general form of a TPM device option is:
address@hidden @option
+
address@hidden -tpmdev @var{backend} ,address@hidden [,@var{options}]
address@hidden -tpmdev
+Backend type must be:
address@hidden
+
+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.
address@hidden
+qemu -tpmdev ?
address@hidden example
+
address@hidden -tpmdev builtin ,address@hidden, address@hidden
+
+Creates an instance of the built-in TPM.
+
address@hidden specifies the path to the QCoW2 image that will store
+the TPM's persistent data. @option{path} is required.
+
+To create a built-in TPM use the following two options:
address@hidden
+-tpmdev builtin,id=tpm0,path=<path_to_qcow2>  -device tpm-tis,tpmdev=tpm0
address@hidden example
+Not that the @code{-tpmdev} id is @code{tpm0} and is referenced by
address@hidden in the device option.
+
address@hidden table
+
+The short form of a TPM device option is:
address@hidden @option
+
address@hidden -tpm @var{backend-type}, address@hidden [,address@hidden
address@hidden -tpm
+
address@hidden specifies the device model. The default device model is a
address@hidden device model. @code{model} is optional.
+
+Use ? to print all available TPM models.
address@hidden
+qemu -tpm model=?
address@hidden example
+
+The other options have the same meaning as explained above.
+
+To create a built-in TPM use the following option:
address@hidden
+-tpm builtin, path=<path_to_qcow2>
address@hidden example
+
address@hidden table
+
+ETEXI
+
+
+DEFHEADING()
+
  DEFHEADING(Linux/Multiboot boot specific:)
  STEXI

Index: qemu-git/vl.c
===================================================================
--- qemu-git.orig/vl.c
+++ qemu-git/vl.c
@@ -137,6 +137,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"
@@ -2498,6 +2499,14 @@ int main(int argc, char **argv, char **e
                  ram_size = value;
                  break;
              }
+#ifdef CONFIG_TPM
+            case QEMU_OPTION_tpm:
+                tpm_config_parse(qemu_find_opts("tpm"), optarg);
+                break;
+            case QEMU_OPTION_tpmdev:
+                tpm_config_parse(qemu_find_opts("tpmdev"), optarg);
+                break;
+#endif
              case QEMU_OPTION_mempath:
                  mem_path = optarg;
                  break;
@@ -3149,6 +3158,12 @@ int main(int argc, char **argv, char **e
          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);
@@ -3394,6 +3409,9 @@ int main(int argc, char **argv, char **e
      quit_timers();
      net_cleanup();
      res_free();
+#ifdef CONFIG_TPM
+    tpm_cleanup();
+#endif

      return 0;
  }
Index: qemu-git/qemu-config.c
===================================================================
--- qemu-git.orig/qemu-config.c
+++ qemu-git/qemu-config.c
@@ -507,6 +507,50 @@ 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 qemu_tpm_opts = {
+    .name = "tpm",
+    .implied_opt_name = "type",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_tpm_opts.head),
+    .desc = {
+        {
+            .name = "type",
+            .type = QEMU_OPT_STRING,
+            .help = "Type of TPM backend",
+        },
+        {
+            .name = "model",
+            .type = QEMU_OPT_STRING,
+            .help = "Model of TPM frontend",
+        },
+        {
+            .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,
@@ -523,6 +567,8 @@ static QemuOptsList *vm_config_groups[32
      &qemu_option_rom_opts,
      &qemu_machine_opts,
      &qemu_boot_opts,
+&qemu_tpmdev_opts,
+&qemu_tpm_opts,
      NULL,
  };

Index: qemu-git/hw/tpm_tis.h
===================================================================
--- /dev/null
+++ qemu-git/hw/tpm_tis.h
@@ -0,0 +1,75 @@
+/*
+ * tpm_tis.h - include file for tpm_tis.c
+ *
+ * Copyright (C) 2006,2010,2011 IBM Corporation
+ *
+ * Author: Stefan Berger<address@hidden>
+ *         David Safford<address@hidden>
+ *
+ * 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.
+ */
+#ifndef _HW_TPM_TIS_H
+#define _HW_TPM_TIS_H
+
+#include "isa.h"
+#include "block_int.h"
+#include "qemu-thread.h"
+
+#include<stdint.h>
+
+#define TIS_ADDR_BASE       0xFED40000
+
+#define NUM_LOCALITIES      5     /* per spec */
+#define NO_LOCALITY         0xff
Please use consistent prefixes to avoid namespace
pollution. E.g. tpm_tis_ for stuff in tpm_tis.h, etc.


Ok. I'll change the functions. Also the #define's ?

[...]
+
+static int configure_tpm(QemuOpts *opts, int is_tpmdev)
+{
+    const char *value;
+    const char *id = TPM_DEFAULT_DEVICE_ID;
+    const char *model =  NULL;
+    const TPMDriverOps *be;
+    TPMBackend *drv;
+
+    if (!QLIST_EMPTY(&tpm_backends)) {
+        fprintf(stderr, "Only one TPM is allowed.\n");
+        return 1;
+    }
+
+    if (is_tpmdev) {
+        id = qemu_opts_id(opts);
+        if (id == NULL) {
+            qerror_report(QERR_MISSING_PARAMETER, "id");
+            return 1;
+        }
+    } else {
+        model = qemu_opt_get(opts, "model");
+        if (model) {
+            if (strcmp(model, "?") == 0) {
+                tpm_display_models(stdout);
+                return 1;
+            }
+            if (!tpm_check_model(model)) {
+                qerror_report(QERR_INVALID_PARAMETER_VALUE, "model",
+                              "a tpm model");
+                tpm_display_models(stderr);
+                return 1;
+            }
+        } else {
+            model = TPM_DEFAULT_DEVICE_MODEL;
+        }
+    }
+
+    value = qemu_opt_get(opts, "type");
+    if (!value) {
+        qerror_report(QERR_MISSING_PARAMETER, "type");
+        tpm_display_backend_drivers(stderr);
+        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(stderr);
+        return 1;
+    }
+
+    assert((is_tpmdev&&  model == NULL) || (!is_tpmdev&&  model != NULL));
Why isn't this using qdev for parameter passing?

Can you point me to a device that is using qdev for parameter passing. Also this part is very similar to how the networking works (net.c).

   Stefan





reply via email to

[Prev in Thread] Current Thread [Next in Thread]