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

[Stefan Berger]

On 02/20/2012 10:18 PM, Michael S. Tsirkin wrote:
> On Mon, Feb 20, 2012 at 07:43:05PM -0500, Stefan Berger wrote:
> > 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
>
> Well, this is just an idea, please do not take this as
> a request or anything like that. Maybe it is a dumb one.
>
> Maybe something like what you describe.

I am starting to wonder what we're trying to achieve? We have a 
producer-consumer problem here with different threads. Both threads need 
to have some locking constructs along with the signalling (condition). 
The backend needs to be written in a certain way to work with the 
frontend, locking and signalling is a part of this. So I don't see it 
makes much sense to move all that code around, especially since there is 
only one backend right now. Maybe something really great can be done 
once there is a 2nd backend.

> Alternatively, I imagined that you can pass a copy
> or pointer of the necessary state to the backend,
> which queues the command and wakes the worker.
> In the reverse direction, backend queues a response
> and when OS polls you dequeue it and update state.

The OS doesn't necessarily need to poll. It is just one mode of 
operation of the OS, the other being interrupt-driven where the backend 
raises the interrupt once it has delivered the response to the frontend.


   Stefan

> Can this work?