From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56113) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RzypP-000602-GO for qemu-devel@nongnu.org; Tue, 21 Feb 2012 18:09:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RzypO-0005hH-5n for qemu-devel@nongnu.org; Tue, 21 Feb 2012 18:08:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:4281) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RzypN-0005hD-QD for qemu-devel@nongnu.org; Tue, 21 Feb 2012 18:08:58 -0500 Date: Wed, 22 Feb 2012 01:08:46 +0200 From: "Michael S. Tsirkin" Message-ID: <20120221230845.GD9062@redhat.com> References: <1323870202-25742-1-git-send-email-stefanb@linux.vnet.ibm.com> <1323870202-25742-3-git-send-email-stefanb@linux.vnet.ibm.com> <20120220220201.GD19278@redhat.com> <4F42E899.3010907@linux.vnet.ibm.com> <20120221031854.GA2502@redhat.com> <4F437DBE.90901@linux.vnet.ibm.com> <20120221121810.GA6975@redhat.com> <4F43B2B6.1040109@linux.vnet.ibm.com> <20120221195812.GB9062@redhat.com> <4F441B08.6000306@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F441B08.6000306@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH V14 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Berger Cc: qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at 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?