Re: [Qemu-devel] [PATCH V19 0/7] Qemu Trusted Platform Module (TPM) integration

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 06/04/2012 03:37 PM, Stefan Berger wrote:
> From: root <root@d941f-5.watson.ibm.com>
>
> The following series of patches adds TPM (Trusted Platform Module) support
> to Qemu. An emulator for the TIS (TPM Interface Spec) interface is
> added that provides the basis for accessing a 'backend' implementing the actual
> TPM functionality. The TIS emulator serves as a 'frontend' enabling for
> example Linux's TPM TIS (tpm_tis) driver.
>
> In this series I am posting a backend implementation that makes use of the
> host's TPM through a passthrough driver, which on Linux is accessed
> using /dev/tpm0.
>
> v19:
>   - applies to checkout of 8cc9b43 (Jun 4)
>
> v18:
>   - applies to checkout of 563987d (May 1)
>   - removed some dead variable in 7/7
>
> v17:
>   - applies to checkout of 6507470 (Apr 30)
>   - split up path and fd into two optional parameters
>
> v16:
>   - applied to checkout of 42fe1c2 (Apr 27)
>   - followed Anthony's suggestions for v15
>   - changed qemu-options.hx and vl.c to not show anything TPM-related if
>     --enable-tpm-passthrough was not used on configure line
>
> v15:
>   - applies to checkout of 8a22565 (Mar 27)
>   - replacing g_malloc's with g_new; no more checks for NULL after allocs
>   - introducing usage of bottom half in TIS frontend to deliver responses
>   - get rid of locks since global lock is held by all threads entering TIS
>     code
>   - cleanups
>
> v14:
>   - applies to checkout of da5361c (Dec 12)
>   - implemented Anthony Liguori's suggestions
>   - dropping the version log on individual patches
>
> v13:
>   - applies to checkout of 61a5872 (Dec 12)
>   - only allowing character devices as fd parameter
>   - fixing error path in tpm_tis_init
>
> v12:
>   - applies to checkout of ebffe2a (Oct 11)
>   - added documentation for fd parameter
>   - nits
>
> v11:
>   - applies to checkout of 46f3069 (Sep 28)
>   - some filing on the documentation
>   - small nits fixed
>
> v10:
>   - applies to checkout of 1ce9ce6 (Sep 27)
>   - addressed Michael Tsirkin's comments on v9
>
> v9:
>   - addressed Michael Tsirkin's and other reviewers' comments
>   - only posting Andreas Niederl's passthrough driver as the backend driver
>
> v8:
>   - applies to checkout of f0fb8b7 (Aug 30)
>   - fixing compilation error pointed out by Andreas Niederl
>   - adding patch that allows to feed an initial state into the libtpms TPM
>   - following memory API changes (glib) where necessary
>
> v7:
>   - applies to checkout of b9c6cbf (Aug 9)
>   - measuring the modules if multiboot is used
>   - coding style fixes
>
> v6:
>   - applies to checkout of 75ef849 (July 2nd)
>   - some fixes and improvements to existing patches; see individual patches
>   - added a patch with a null driver responding to all TPM requests with
>     a response indicating failure; this backend has no dependencies and
>     can alwayy be built;
>   - added a patch to support the hashing of kernel, ramfs and command line
>     if those were passed to Qemu using -kernel, -initrd and -append
>     respectively. Measurements are taken, logged, and passed to SeaBIOS using
>     the firmware interface.
>   - libtpms revision 7 now requires 83kb of block storage due to having more
>     NVRAM space
>
> v5:
>   - applies to checkout of 1fddfba1
>   - adding support for split command line using the -tpmdev ... -device ...
>     options while keeping the -tpm option
>   - support for querying the device models using -tpm model=?
>   - support for monitor 'info tpm'
>   - adding documentation of command line options for man page and web page
>   - increasing room for ACPI tables that qemu reserves to 128kb (from 64kb)
>   - adding (experimental) support for block migration
>   - adding (experimental) support for taking measurements when kernel,
>     initrd and kernel command line are directly passed to Qemu
>
> v4:
>   - applies to checkout of d2d979c6
>   - more coding style fixes
>   - adding patch for supporting blob encryption (in addition to the existing
>     QCoW2-level encryption)
>     - this allows for graceful termination of a migration if the target
>       is detected to have a wrong key
>     - tested with big and little endian hosts
>   - main thread releases mutex while checking for work to do on behalf of
>     backend
>   - introducing file locking (fcntl) on the block layer for serializing access
>     to shared (QCoW2) files (used during migration)
>
> v3:
>   - Building a null driver at patch 5/8 that responds to all requests
>     with an error response; subsequently this driver is transformed to the
>     libtpms-based driver for real TPM functionality
>   - Reworked the threading; dropped the patch for qemu_thread_join; the
>     main thread synchronizing with the TPM thread termination may need
>     to write data to the block storage while waiting for the thread to
>     terminate; did not previously show a problem but is safer
>   - A lot of testing based on recent git checkout 4b4a72e5 (4/10):
>     - migration of i686 VM from x86_64 host to i686 host to ppc64 host while
>       running tests inside the VM
>     - tests with S3 suspend/resume
>     - tests with snapshots
>     - multiple-hour tests with VM suspend/resume (using virsh save/restore)
>       while running a TPM test suite inside the VM
>     All tests passed; [not all of them were done on the ppc64 host]
>
> v2:
>   - splitting some of the patches into smaller ones for easier review
>   - fixes in individual patches
>
> Regards,
>      Stefan
>
>
> Stefan Berger (7):
>    Support for TPM command line options
>    Add TPM (frontend) hardware interface (TPM TIS) to Qemu
>    Add a debug register
>    Build the TPM frontend code
>    Add a TPM Passthrough backend driver implementation
>    Introduce --enable-tpm-passthrough configure option
>    Add fd parameter for TPM passthrough driver
>
>   Makefile.target      |    3 +
>   configure            |   28 ++
>   hmp-commands.hx      |    2 +
>   hmp.c                |   33 ++
>   hmp.h                |    1 +
>   hw/tpm_backend.c     |   58 ++++
>   hw/tpm_backend.h     |   43 +++
>   hw/tpm_passthrough.c |  427 ++++++++++++++++++++++++
>   hw/tpm_tis.c         |  892 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/tpm_tis.h         |   78 +++++
>   monitor.c            |    8 +
>   qapi-schema.json     |   31 ++
>   qemu-config.c        |   25 ++
>   qemu-options.hx      |   74 +++++
>   qmp-commands.hx      |    5 +
>   tpm.c                |  235 +++++++++++++
>   tpm.h                |  115 +++++++
>   vl.c                 |   19 +
>   18 files changed, 2077 insertions(+), 0 deletions(-)
>   create mode 100644 hw/tpm_backend.c
>   create mode 100644 hw/tpm_backend.h
>   create mode 100644 hw/tpm_passthrough.c
>   create mode 100644 hw/tpm_tis.c
>   create mode 100644 hw/tpm_tis.h
>   create mode 100644 tpm.c
>   create mode 100644 tpm.h
>

I reviewed this series and have sent all my comments.  In general the 
patches look good to me other than the comments I've already made.

The external interfaces (command line and monitor commands) look to be 
in good shape.  In particular -tpmdev can easily be modified to support 
future backend vTPM implementations, such a software vTPM.  And the 
monitor commands are simple and straight forward (info TPM and query-TPM 
with no arguments).

I haven't had the chance to run any tests with the patch series, but I 
plan to soon.  In the mean time I have a couple of general questions:

I understand that the passthrough driver won't support migration.  What 
happens on a migration attempt?  Does it fail gracefully?

What happens when a 2nd guest attempts to use the passthrough vTPM after 
the 1st guest is already using it?  Does it also fail gracefully?

Are there any architecture dependencies for this support?

-- 
Regards,
Corey Bryant