From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56052) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMb72-0006qX-Vt for qemu-devel@nongnu.org; Wed, 18 May 2011 03:24:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QMb72-0003os-07 for qemu-devel@nongnu.org; Wed, 18 May 2011 03:24:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53858) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMb71-0003og-Nw for qemu-devel@nongnu.org; Wed, 18 May 2011 03:24:07 -0400 From: Markus Armbruster References: <20110506173224.278066589@linux.vnet.ibm.com> <20110506173244.772773627@linux.vnet.ibm.com> Date: Wed, 18 May 2011 09:23:24 +0200 In-Reply-To: <20110506173244.772773627@linux.vnet.ibm.com> (Stefan Berger's message of "Fri, 06 May 2011 13:32:26 -0400") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH V4 02/10] 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: serge@hallyn.com, qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at Stefan Berger writes: > This patch adds the main code of the TPM frontend driver, the TPM TIS > interface, to Qemu. The code is largely based on my 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. > > v3: > - prefixing functions with tis_ > - added a function to the backend interface 'early_startup_tpm' that > allows to detect the presence of the block storage and gracefully fails > Qemu if it's not available. This works with migration using shared > storage but doesn't support migration with block storage migration. > For encyrypted QCoW2 and in case of a snapshot resue the late_startup_tpm > interface function is called > > Signed-off-by: Stefan Berger > > --- > hw/tpm_tis.c | 871 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 871 insertions(+) > > Index: qemu-git/hw/tpm_tis.c > =================================================================== > --- /dev/null > +++ qemu-git/hw/tpm_tis.c [...] > +static void tis_reset(DeviceState *d) > +{ > + TPMState *s = container_of(d, TPMState, busdev.qdev); > + tis_s_reset(s); > +} > + > + > +static int tis_init(ISADevice *dev) Function not used, sure this compiles with -Werror? If not, it needs fixing. As far as I can see, the use is added in 03/10, where you add the missing qdev parts. Makes it hard to review for qdev sanity, so I did *not* do that. > +{ > + TPMState *s = DO_UPCAST(TPMState, busdev, dev); > + int iomemtype, rc; > + > + qemu_mutex_init(&s->state_lock); > + qemu_cond_init(&s->from_tpm_cond); > + qemu_cond_init(&s->to_tpm_cond); > + > + if (active_be->init(s, tis_tpm_receive_cb)) { > + goto err_exit; > + } > + > + isa_init_irq(dev, &s->irq, s->irq_num); > + > + iomemtype = cpu_register_io_memory(tis_readfn, tis_writefn, s, > + DEVICE_LITTLE_ENDIAN); > + cpu_register_physical_memory(TIS_ADDR_BASE, 0x1000 * NUM_LOCALITIES, > + iomemtype); > + > + /* > + * startup the TPM backend early to detect problems early > + */ > + rc = tis_do_early_startup_tpm(s); > + if (rc != 0 && rc != -ENOKEY) { > + fprintf(stderr,"tpm_tis: Fatal error accessing TPM's block storage.\n"); > + goto err_exit; > + } > + > + return 0; > + > + err_exit: > + return -1; > +} > + Please fix "new blank line at EOF."