From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51333) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMeNh-0003i1-Th for qemu-devel@nongnu.org; Wed, 18 May 2011 06:53:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QMeNg-0007hv-Qh for qemu-devel@nongnu.org; Wed, 18 May 2011 06:53:33 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:46053) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMeNg-0007hr-LU for qemu-devel@nongnu.org; Wed, 18 May 2011 06:53:32 -0400 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by e4.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p4IAWeEM004490 for ; Wed, 18 May 2011 06:32:40 -0400 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p4IArVUo901334 for ; Wed, 18 May 2011 06:53:31 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p4IArV7j015773 for ; Wed, 18 May 2011 06:53:31 -0400 Message-ID: <4DD3A52A.3000301@linux.vnet.ibm.com> Date: Wed, 18 May 2011 06:53:30 -0400 From: Stefan Berger MIME-Version: 1.0 References: <20110506173224.278066589@linux.vnet.ibm.com> <20110506173244.772773627@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: Markus Armbruster Cc: serge@hallyn.com, qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at On 05/18/2011 03:23 AM, Markus Armbruster wrote: > 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. > I tried to split up the patches in smaller parts and had to cut 'somewhere'. Without additional compile option it does not report any errors. >> +{ >> + 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." Will do. Thanks. Stefan