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

[Stefan Berger]

On 02/20/2012 05:02 PM, Michael S. Tsirkin wrote:
> On Wed, Dec 14, 2011 at 08:43:17AM -0500, Stefan Berger wrote:
> > +/*
> > + * 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;
> > +
> > +    tpm_tis_show_buffer(&tis->loc[locty].w_buffer, "tpm_tis: To TPM");
> > +
> > +    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].status = TPM_TIS_STATUS_EXECUTION;
> > +    tis->loc[locty].sts&= ~TPM_TIS_STS_EXPECT;
> > +
> > +    s->to_tpm_execute = true;
> > +    qemu_cond_signal(&s->to_tpm_cond);
> > +}
> What happens IIUC is that frondend sets to_tpm_execute
> and signals a condition, and backend clears it
> and waits on a condition.
>
> So how about moving all the signalling
> and locking out to backend, and have frontend
> invoke a callback to signal it?
>
> The whole threading thing then becomes a work-around
> for a backend that does not support select,
> instead of spilling out into frontend?

How do I get the lock calls (qemu_mutex_lock(&s->state_lock)) out of the 
frontend? Do you want me to add callbacks to the backend interface for 
locking (s->be_driver->ops->state_lock(s)) and one for unlocking 
(s->be_driver->ops->state_unlock(tpm_be)) of the state that really 
belongs to the front-end (state is 's') and invoke it as shown in 
parenthesis and still keep s->state_lock around? Ideally the locks would 
end up being 'nop's' if select() was available, but in the end all 
backend will need to support that lock.

[The lock protects the common structure so that the thread in the 
backend can deliver the response to a request while the OS for example 
polls the hardware interface for its current state.]


   Stefan