qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v9 8/8] tpm: Added support for TPM emulator


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PATCH v9 8/8] tpm: Added support for TPM emulator
Date: Fri, 29 Sep 2017 11:50:37 +0200

On Fri, Sep 29, 2017 at 10:35 AM, Valluri, Amarnath
<address@hidden> wrote:
> On Thu, 2017-09-28 at 13:53 +0200, Marc-André Lureau wrote:
>> Hi
>>
>> On Thu, Sep 28, 2017 at 11:20 AM, Amarnath Valluri
>> <address@hidden> wrote:
>> >
>> > This change introduces a new TPM backend driver that can
>> > communicate with
>> > swtpm(software TPM emulator) using unix domain socket interface.
>> > QEMU talks to
>> > TPM emulator using QEMU's socket-based chardev backend device.
>> >
>> > Swtpm uses two Unix sockets for communications, one for plain TPM
>> > commands and
>> > responses, and one for out-of-band control messages. QEMU passes
>> > data socket to
>> > be used over the control channel.
>> >
>> > The swtpm and associated tools can be found here:
>> >     https://github.com/stefanberger/swtpm
>> >
>> > The swtpm's control channel protocol specification can be found
>> > here:
>> >     https://github.com/stefanberger/swtpm/wiki/Control-Channel-Spec
>> > ification
>> >
>> > Usage:
>> >     # setup TPM state directory
>> >     mkdir /tmp/mytpm
>> >     chown -R tss:root /tmp/mytpm
>> >     /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
>> >
>> >     # Ask qemu to use TPM emulator with given tpm state directory
>> >     qemu-system-x86_64 \
>> >         [...] \
>> >         -chardev socket,id=chrtpm,path=/tmp/swtpm-sock \
>> >         -tpmdev emulator,id=tpm0,chardev=chrtpm \
>> >         -device tpm-tis,tpmdev=tpm0 \
>> >         [...]
>> >
>> > Signed-off-by: Amarnath Valluri <address@hidden>
>> > ---
>> >  configure             |  13 +-
>> >  hmp.c                 |   5 +
>> >  hw/tpm/Makefile.objs  |   1 +
>> >  hw/tpm/tpm_emulator.c | 599
>> > ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  hw/tpm/tpm_ioctl.h    | 246 +++++++++++++++++++++
>> >  qapi/tpm.json         |  21 +-
>> >  qemu-options.hx       |  22 +-
>> >  vl.c                  |   1 +
>> >  8 files changed, 901 insertions(+), 7 deletions(-)
>> >  create mode 100644 hw/tpm/tpm_emulator.c
>> >  create mode 100644 hw/tpm/tpm_ioctl.h
>> >
>> > diff --git a/configure b/configure
>> > index cb0f7ed..a1b956e 100755
>> > --- a/configure
>> > +++ b/configure
>> > @@ -3467,6 +3467,12 @@ else
>> >    tpm_passthrough=no
>> >  fi
>> >
>> > +# TPM emulator is for all posix systems
>> > +if test "$mingw32" != "yes"; then
>> > +  tpm_emulator=$tpm
>> > +else
>> > +  tpm_emulator=no
>> > +fi
>> >  ##########################################
>> >  # attr probe
>> >
>> > @@ -5359,6 +5365,7 @@ echo "gcov enabled      $gcov"
>> >  echo "TPM support       $tpm"
>> >  echo "libssh2 support   $libssh2"
>> >  echo "TPM passthrough   $tpm_passthrough"
>> > +echo "TPM emulator      $tpm_emulator"
>> >  echo "QOM debugging     $qom_cast_debug"
>> >  echo "Live block migration $live_block_migration"
>> >  echo "lzo support       $lzo"
>> > @@ -5937,12 +5944,16 @@ if test "$live_block_migration" = "yes" ;
>> > then
>> >    echo "CONFIG_LIVE_BLOCK_MIGRATION=y" >> $config_host_mak
>> >  fi
>> >
>> > -# TPM passthrough support?
>> >  if test "$tpm" = "yes"; then
>> >    echo 'CONFIG_TPM=$(CONFIG_SOFTMMU)' >> $config_host_mak
>> > +  # TPM passthrough support?
>> >    if test "$tpm_passthrough" = "yes"; then
>> >      echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
>> >    fi
>> > +  # TPM emulator support?
>> > +  if test "$tpm_emulator" = "yes"; then
>> > +    echo "CONFIG_TPM_EMULATOR=y" >> $config_host_mak
>> > +  fi
>> >  fi
>> >
>> >  echo "TRACE_BACKENDS=$trace_backends" >> $config_host_mak
>> > diff --git a/hmp.c b/hmp.c
>> > index 0fb2bc7..9cd8179 100644
>> > --- a/hmp.c
>> > +++ b/hmp.c
>> > @@ -994,6 +994,7 @@ void hmp_info_tpm(Monitor *mon, const QDict
>> > *qdict)
>> >      Error *err = NULL;
>> >      unsigned int c = 0;
>> >      TPMPassthroughOptions *tpo;
>> > +    TPMEmulatorOptions *teo;
>> >
>> >      info_list = qmp_query_tpm(&err);
>> >      if (err) {
>> > @@ -1023,6 +1024,10 @@ void hmp_info_tpm(Monitor *mon, const QDict
>> > *qdict)
>> >                             tpo->has_cancel_path ? ",cancel-path="
>> > : "",
>> >                             tpo->has_cancel_path ? tpo->cancel_path
>> > : "");
>> >              break;
>> > +        case TPM_TYPE_OPTIONS_KIND_EMULATOR:
>> > +            teo = ti->options->u.emulator.data;
>> > +            monitor_printf(mon, ",chardev=%s", teo->chardev);
>> > +            break;
>> >          case TPM_TYPE_OPTIONS_KIND__MAX:
>> >              break;
>> >          }
>> > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
>> > index 64cecc3..41f0b7a 100644
>> > --- a/hw/tpm/Makefile.objs
>> > +++ b/hw/tpm/Makefile.objs
>> > @@ -1,2 +1,3 @@
>> >  common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
>> >  common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
>> > tpm_util.o
>> > +common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o tpm_util.o
>> > diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>> > new file mode 100644
>> > index 0000000..0065b0a
>> > --- /dev/null
>> > +++ b/hw/tpm/tpm_emulator.c
>> > @@ -0,0 +1,599 @@
>> > +/*
>> > + *  Emulator TPM driver
>> > + *
>> > + *  Copyright (c) 2017 Intel Corporation
>> > + *  Author: Amarnath Valluri <address@hidden>
>> > + *
>> > + *  Copyright (c) 2010 - 2013 IBM Corporation
>> > + *  Authors:
>> > + *    Stefan Berger <address@hidden>
>> > + *
>> > + *  Copyright (C) 2011 IAIK, Graz University of Technology
>> > + *    Author: Andreas Niederl
>> > + *
>> > + * 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.or
>> > g/licenses/>
>> > + *
>> > + */
>> > +
>> > +#include "qemu/osdep.h"
>> > +#include "qemu/error-report.h"
>> > +#include "qemu/sockets.h"
>> > +#include "io/channel-socket.h"
>> > +#include "sysemu/tpm_backend.h"
>> > +#include "tpm_int.h"
>> > +#include "hw/hw.h"
>> > +#include "hw/i386/pc.h"
>> > +#include "tpm_util.h"
>> > +#include "tpm_ioctl.h"
>> > +#include "migration/blocker.h"
>> > +#include "qapi/error.h"
>> > +#include "qapi/clone-visitor.h"
>> > +#include "chardev/char-fe.h"
>> > +
>> > +#include <fcntl.h>
>> > +#include <sys/types.h>
>> > +#include <sys/stat.h>
>> > +#include <stdio.h>
>> > +
>> > +#define DEBUG_TPM 0
>> > +
>> > +#define DPRINTF(fmt, ...) do { \
>> > +    if (DEBUG_TPM) { \
>> > +        fprintf(stderr, "tpm-emulator:"fmt"\n", ## __VA_ARGS__); \
>> > +    } \
>> > +} while (0)
>> > +
>> > +#define TYPE_TPM_EMULATOR "tpm-emulator"
>> > +#define TPM_EMULATOR(obj) \
>> > +    OBJECT_CHECK(TPMEmulator, (obj), TYPE_TPM_EMULATOR)
>> > +
>> > +#define TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(S, cap) (((S)->caps &
>> > (cap)) == (cap))
>> > +
>> > +static const TPMDriverOps tpm_emulator_driver;
>> > +
>> > +/* data structures */
>> > +typedef struct TPMEmulator {
>> > +    TPMBackend parent;
>> > +
>> > +    TPMEmulatorOptions *options;
>> > +    CharBackend ctrl_chr;
>> > +    QIOChannel *data_ioc;
>> > +    TPMVersion tpm_version;
>> > +    ptm_cap caps; /* capabilities of the TPM */
>> > +    uint8_t cur_locty_number; /* last set locality */
>> > +    QemuMutex state_lock;
>> The mutex isnt doing much with this code, as there should be no
>> concurrent handle_request() (thread-pool has max-thread=1).
> Yes I agree, I can remove this mutex in this case.
>>
>> (I wonder if we could replace the thread-pool with a coroutine/bh
>> doing IO instead to avoid potential thread races... or just do IO in
>> the current context, like the ctrlcmd)
> Yes we should. Can we do it as a separate PR?. As it touches both the
> backends.
>

Absolutely, this is not directly related. We should try to get your
series merged first. I have a pending series with various tpm
cleanups, I may try to tackle that too.

Thanks


-- 
Marc-André Lureau



reply via email to

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