On Fri, Mar 02, 2012 at 07:02:21AM -0500, Stefan Berger wrote: > On 02/21/2012 06:08 PM, Michael S. Tsirkin wrote: > >On Tue, Feb 21, 2012 at 05:30:32PM -0500, Stefan Berger wrote: > >>On 02/21/2012 02:58 PM, Michael S. Tsirkin wrote: > >>>On Tue, Feb 21, 2012 at 10:05:26AM -0500, Stefan Berger wrote: > >>>>On 02/21/2012 07:18 AM, Michael S. Tsirkin wrote: > >>>>When the backend delivers the response it checks whether the > >>>>interface is used in interrupt mode and raises the interrupt. > >>>IMO it's the frontend that should send interrupts. > >>>Yes it kind of works for isa anyway, but e.g. pci > >>>needs to update configuration etc. > >>> > >>The code that causes the interrupt to be raised is in the frontend. > >>The function doing that is invoked via callback from the backend. > >>This should be ok? > >Absolutely. > > > >>>>The > >>>>backend enters the frontend code with a callback. In this function > >>>>also a signal is sent that may wake up the main thread that, upon > >>>>suspend, may be waiting for the last command to be processed and be > >>>>sleeping on a condition variable. > >>>> > >>>>I now added a function to the backend interface that is invoked by > >>>>the frontend to notify the backend of a TPM request. The backend > >>>>code can then either notify a thread (passthrough and libtpms > >>>>driver) or create a response right away and invoke that callback to > >>>>the front-end to deliver the response (null driver). How frontend > >>>>and backend handle notifications is isolated to the frontend and > >>>>backend with some backends (libtpms, passthough) sharing the code > >>>>for how the notification is done. > >>>> > >>>>Stefan > >>>Right. And all the locking/threading can then be internal to the backend. > >>> > >>Do you really want to replace code like this in the frontend: > >> > >>qemu_mutex_lock(&s->state_lock) > >>[...] > >>qemu_mutex_unlock(&s->state_lock) > >> > >> > >>with > >> > >> > >>s->be_driver->ops->state_lock(s->be_driver) > >>[...] > >>s->be_driver->ops->state_unlock(s->be_driver)) > >> > >> > >>where the backend starts protecting frontend data structures ? > >It's ugly I hope you can do something saner: > >ops->send_command(....) > >with command encapsulating the relevant info. > > > >>At the moment there are two backends that need threading: the > >>libtpms and passthrough backends. Both will require locking of > >>datastructures that belong to the frontend. Only the null driver > >>doesn't need a thread and the main thread can call into the backend, > >>create the response and call via callback into the frontend to > >>deliver the repsonse. If structures are protected via mutxes then > >>only the NULL driver (which we don't want anyway) may end up > >>grabbing mutexes that really aren't necessary while the two other > >>backends need them. I don't see the mitextes as problematic. The > >>frontend at least protects its data structures for the callbacks and > >>other API calls it offers and they simply are thread-safe. > >> > >> Stefan > >Worst case, you can take a qemu mutex. Is tpm very > >performance-sensitive to make contention on that > >lock a problem? > > > > Having instrumented the code with qemu_mutex_trylock() and a counter > for failed attempts with subsequent qemu_mutex_lock() practically > shows no lock contention at all for either polling or IRQ mode being > used by the Linux driver. > > IRQ mode: 4 failed lock attempts out of 1726208 locks -> 0.00% > polling mode: 2 failed lock attempts out of 1545216 locks -> 0.00% > > I used the libtpms based backend with and ran IMA and a test suite > in the VM. > > > Stefan
so maybe you can just do qemu_mutex_lock_iothread similar to what kvm does. -- MST
