Re: [Qemu-devel] [PATCH V8 07/14] Implementation of the libtpms-based backend

[Stefan Berger]

On 09/01/2011 01:27 PM, Michael S. Tsirkin wrote:
> On Wed, Aug 31, 2011 at 10:35:58AM -0400, Stefan Berger wrote:
> > This patch provides the glue for the TPM TIS interface (frontend) to
> > the libtpms that provides the actual TPM functionality.
> >
> > Some details:
> >
> > This part of the patch provides support for the spawning of a thread
> > that will interact with the libtpms-based TPM. It expects a signal
> > from the frontend to wake and pick up the TPM command that is supposed
> > to be processed and delivers the response packet using a callback
> > function provided by the frontend.
> >
> > The backend connects itself to the frontend by filling out an interface
> > structure with pointers to the function implementing support for various
> > operations.
> >
> > In this part a structure with callback functions is registered with
> > libtpms. Those callback functions are invoked by libtpms for example to
> > store the TPM's state.
> >
> > The libtpms-based backend implements functionality to write into a
> > Qemu block storage device rather than to plain files. With that we
> > can support VM snapshotting and we also get the possibility to use
> > encrypted QCoW2 for free. Thanks to Anthony for pointing this out.
> > The storage part of the driver has been split off into its own patch.
> >
> > v6:
> >    - cache a copy of the last permanent state blob
> >    - move some functions into tpm_builtin.h
> >    - reworked parts of the error path handling where the TPM is
> >      now used to process commands under error conditions and the callbacks
> >      make the TPM aware of the error conditions. Only as the last resort
> >      fault messages are sent by the backend driver circumventing the TPM.
> >    - add out_len variable used in the thread
> >
> > v5:
> >    - check access() to TPM's state file and report error if file is not
> >      accessible
> >
> > v3:
> >    - temporarily deactivate the building of the tpm_builtin.c until
> >      subsequent patch completely converts it to the libtpms based driver
> >
> > v2:
> >    - fixes to adhere to the qemu coding style
> >
> >
> > Signed-off-by: Stefan Berger<address@hidden>
> >
> > ---
> >   configure        |    1
> >   hw/tpm_builtin.c |  450 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> >   hw/tpm_builtin.h |   56 ++++++
> >   3 files changed, 482 insertions(+), 25 deletions(-)
> >
> > Index: qemu-git/hw/tpm_builtin.c
> > ===================================================================
> > --- qemu-git.orig/hw/tpm_builtin.c
> > +++ qemu-git/hw/tpm_builtin.c
> > @@ -1,5 +1,5 @@
> >   /*
> > - *  builtin 'null' TPM driver
> > + *  builtin TPM driver based on libtpms
> Just wondering - might a stub driver be useful for
> basic testing on systems without TPM hardware?
'systems without TPM hardware' -- we're not relying on underlying TPM 
provided by the host. But I assume that's not what you meant.

A 'null' driver, which responds to every command with an error response, 
is added in patch 13.

> The namespace comment applies to this and all other patches.
In patch 6 I am adding a skeleton backend driver that I am transforming 
into the libtpms-based backend in patch 7. I didn't name the file 
tpm_skeleton.c but already tpm_builtin.c and all functions already start 
with the prefix tpm_builtin. This presumably makes it easier to review 
since the 'meat' is added in part 7 and unnecessary function name 
changes are avoided. The null driver later on and the passthrough driver 
(posted by Andreas Niederl) all can be derived rather easily from that 
initial skeleton driver. In those we do adhere to the namespace requirement.

  Stefan