Re: [Qemu-devel] [PATCH V13 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

[Stefan Berger]

On 12/12/2011 06:23 PM, Anthony Liguori wrote:
> On 12/12/2011 01:12 PM, Stefan Berger wrote:
> > This patch adds the main code of the TPM frontend driver, the TPM TIS
> > interface, to Qemu. The code is largely based on the previous 
> > implementation
> > for Xen but has been significantly extended to meet the standard's
> > requirements, such as the support for changing of localities and all the
> > functionality of the available flags.
> >
> > Communication with the backend (i.e., for Xen or the libtpms-based one)
> > is cleanly separated through an interface which the backend driver needs
> > to implement.
> >
> > The TPM TIS driver's backend was previously chosen in the code added
> > to arch_init. The frontend holds a pointer to the chosen backend 
> > (interface).
> >
> > Communication with the backend is largely based on signals and 
> > conditions.
> > Whenever the frontend has collected a complete packet, it will signal
> > the backend, which then starts processing the command. Once the result
> > has been returned, the backend invokes a callback function
> > (tis_tpm_receive_cb()).
> >
> > The one tricky part is support for VM suspend while the TPM is 
> > processing
> > a command. In this case the frontend driver is waiting for the backend
> > to return the result of the last command before shutting down. It waits
> > on a condition for a signal from the backend, which is delivered in
> > tis_tpm_receive_cb().
> >
> > Testing the proper functioning of the different flags and localities
> > cannot be done from user space when running in Linux for example, since
> > access to the address space of the TPM TIS interface is not possible. 
> > Also
> > the Linux driver itself does not exercise all functionality. So, for
> > testing there is a fairly extensive test suite as part of the SeaBIOS 
> > patches
> > since from within the BIOS one can have full access to all the TPM's 
> > registers.
> >
> >
> > Signed-off-by: Stefan Berger<address@hidden>
[...]

> > +
> > +/*
> > + * Send a TPM request.
> > + * Call this with the state_lock held so we can sync with the receive
> > + * callback.
> > + */
> > +static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
> > +{
> > +    TPMTISState *tis =&s->s.tis;
> > +#ifdef DEBUG_TIS
> > +    tpm_tis_show_buffer(&tis->loc[locty].w_buffer, "tpm_tis: To TPM");
> > +#endif
> > +    s->command_locty = locty;
> > +    s->cmd_locty     =&tis->loc[locty];
> > +
> > +    /* w_offset serves as length indicator for length of data;
> > +       it's reset when the response comes back */
> > +    tis->loc[locty].state = TPM_TIS_STATE_EXECUTION;
> > +    tis->loc[locty].sts&= ~TPM_TIS_STS_EXPECT;
> > +
> > +    s->to_tpm_execute = true;
> > +    qemu_cond_signal(&s->to_tpm_cond);
>
> The locking seems to presume that the device model is re-entrant which 
> it's not today.  Am I missing something here?

The TPM TIS frontend communicates with the TPM backend via a condition 
notifying it when a complete buffer with a TPM request has been 
received. The TPM backend is running as a thread, created via 
qemu_thread_create(). This is the design that was driven by the 
libtpms-based implementation.

   Stefan