From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40093) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCuk9-0001Wb-Ou for qemu-devel@nongnu.org; Wed, 28 Mar 2012 11:25:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCuk7-0005no-Lt for qemu-devel@nongnu.org; Wed, 28 Mar 2012 11:25:01 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:55813) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCuk7-0005nC-FN for qemu-devel@nongnu.org; Wed, 28 Mar 2012 11:24:59 -0400 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 28 Mar 2012 09:24:54 -0600 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id B23381FF004C for ; Wed, 28 Mar 2012 09:24:49 -0600 (MDT) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q2SFOjQ8123184 for ; Wed, 28 Mar 2012 09:24:47 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q2SFOKG5010090 for ; Wed, 28 Mar 2012 09:24:21 -0600 Message-ID: <4F732D20.9030203@linux.vnet.ibm.com> Date: Wed, 28 Mar 2012 11:24:16 -0400 From: Stefan Berger MIME-Version: 1.0 References: <1332879879-29460-1-git-send-email-stefanb@linux.vnet.ibm.com> <1332879879-29460-2-git-send-email-stefanb@linux.vnet.ibm.com> <4F7232B8.2080001@codemonkey.ws> In-Reply-To: <4F7232B8.2080001@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V15 1/7] Support for TPM command line options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: mst@redhat.com, qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at On 03/27/2012 05:35 PM, Anthony Liguori wrote: > On 03/27/2012 03:24 PM, Stefan Berger wrote: >> This patch adds support for TPM command line options. >> The command line options supported here are >> [...] >> Monitor support for 'info tpm' has been added. It for example prints the >> following: >> >> (qemu) info tpm >> TPM devices: >> tpm0: model=tpm-tis >> \ tpm0: type=passthrough,path=/dev/tpm0 >> >> Signed-off-by: Stefan Berger >> --- [...] >> >> --- /dev/null >> +++ b/hw/tpm_tis.h >> @@ -0,0 +1,81 @@ >> +/* >> + * tpm_tis.c - QEMU's TPM TIS interface emulator >> + * >> + * Copyright (C) 2006,2010,2011 IBM Corporation >> + * >> + * Authors: >> + * Stefan Berger >> + * David Safford >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation, version 2 of the >> + * License. > > Or later please. We're sticking with GPLv2 as our effective license > but asking that all new code is GPLv2 or later. Forgot that one. Fixed. > >> + * >> + * >> + * Implementation of the TIS interface according to specs found at >> + * http://www.trustedcomputiggroup.org >> + * >> + */ >> +#ifndef HW_TPM_TIS_H >> +#define HW_TPM_TIS_H >> + >> +#include "isa.h" >> +#include "qemu-thread.h" > > This shouldn't be needed anymore. Indeed. Removed.+ >> +typedef struct TPMTISState { >> + uint32_t offset; >> + uint8_t buf[TPM_TIS_BUFFER_MAX]; >> + >> + uint8_t active_locty; >> + uint8_t aborting_locty; >> + uint8_t next_locty; >> + >> + TPMLocality loc[TPM_TIS_NUM_LOCALITIES]; >> + >> + qemu_irq irq; >> + uint32_t irq_num; > > I'm a bit confused here. If TPMTISState has an irq, shouldn't it be a > DeviceState? > So I guess you would expect the irq to be part of TPMState?diff --git a/qapi-schema.json b/qapi-schema.json >> index 0d11d6e..4ad6d29 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -1701,3 +1701,32 @@ >> # Since: 1.1 >> ## >> { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} } >> + >> +## >> +# @TPMInfo: >> +# >> +# Information about the TPM >> +# >> +# @model: The TPM frontend model, i.e., tpm-tis >> +# >> +# @id: The ID of the TPM >> +# >> +# @type: The type of TPM backend, i.e., passthrough >> +# >> +# @parameters: Additional parameters of the TPM backend device > > This is in some sort of key=value format? Why not specify those > parameters properly in the schema as optional items? > Yes, it's in key=value format and it is optional. So, I fixed this now with #optional and '*parameters'. Should it make a difference in the code, except for the auto-generated code? + >> +/* overall state of the TPM interface */ >> +typedef struct TPMState { >> + ISADevice busdev; >> + MemoryRegion mmio; >> + >> + union { >> + TPMTISState tis; >> + } s; >> + >> + uint8_t command_locty; >> + TPMLocality *cmd_locty; >> + >> + char *backend; >> + TPMBackend *be_driver; >> +} TPMState; > > I'm a bit confused at what the relationship between TPMTISState and > TPMSTate. The rational is that TPMState could accomodate different types of front-ends in the union with the part in the union being private to the front-end. Right now there is only a TIS emulator, there could be a virtio (with the restriction that it would have to support TPMLocality). Obviously there is a backend driver necessary for this split between front- and backend to work, so this is what the 'char *backend' and TPMBackend *be_driver are good for. cmd_locty is shared between front and backend and is set by the frontend and read by the backend. Regards, Stefan