From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YhOyd-0005ng-7M for qemu-devel@nongnu.org; Sun, 12 Apr 2015 16:59:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YhOyZ-0001Z7-UE for qemu-devel@nongnu.org; Sun, 12 Apr 2015 16:59:35 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:47353) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YhOyZ-0001Xy-Q0 for qemu-devel@nongnu.org; Sun, 12 Apr 2015 16:59:31 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 12 Apr 2015 16:59:29 -0400 Received: from b01cxnp22035.gho.pok.ibm.com (b01cxnp22035.gho.pok.ibm.com [9.57.198.25]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 894C96E8041 for ; Sun, 12 Apr 2015 16:51:16 -0400 (EDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t3CKxRZn22020214 for ; Sun, 12 Apr 2015 20:59:27 GMT Received: from d01av04.pok.ibm.com (localhost [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t3CKxQAo017369 for ; Sun, 12 Apr 2015 16:59:26 -0400 Message-ID: <552ADCAE.7070600@linux.vnet.ibm.com> Date: Sun, 12 Apr 2015 16:59:26 -0400 From: Stefan Berger MIME-Version: 1.0 References: <1427830813-639306-1-git-send-email-stefanb@linux.vnet.ibm.com> <1427830813-639306-3-git-send-email-stefanb@linux.vnet.ibm.com> <945CA011AD5F084CBEA3E851C0AB28890E8DB967@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <945CA011AD5F084CBEA3E851C0AB28890E8DB967@SHSMSX101.ccr.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] tpm: Probe for connected TPM 1.2 or TPM 2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Xu, Quan" , "qemu-devel@nongnu.org" , "mst@redhat.com" , Eric Blake Cc: Stefan Berger On 04/07/2015 04:54 AM, Xu, Quan wrote: > >> -----Original Message----- >> From: Stefan Berger [mailto:stefanb@linux.vnet.ibm.com] >> Sent: Wednesday, April 01, 2015 3:40 AM >> To: qemu-devel@nongnu.org; mst@redhat.com >> Cc: Xu, Quan; Stefan Berger; Stefan Berger >> Subject: [PATCH 2/3] tpm: Probe for connected TPM 1.2 or TPM 2 >> >> In the TPM passthrough backend driver, modify the probing code so that we can >> check whether a TPM 1.2 or TPM 2 is being used and adapt the behavior of the >> TPM TIS accordingly. >> >> Move the code that tested for a TPM 1.2 into tpm_utils.c and extend it with test >> for probing for TPM 2. Have the function return the version of TPM found. >> >> Signed-off-by: Stefan Berger >> --- >> hw/tpm/Makefile.objs | 2 +- >> hw/tpm/tpm_int.h | 6 +++ >> hw/tpm/tpm_passthrough.c | 59 +++------------------- >> hw/tpm/tpm_util.c | 126 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> hw/tpm/tpm_util.h | 28 +++++++++++ >> 5 files changed, 167 insertions(+), 54 deletions(-) create mode 100644 >> hw/tpm/tpm_util.c create mode 100644 hw/tpm/tpm_util.h >> >> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index >> 99f5983..64cecc3 100644 >> --- a/hw/tpm/Makefile.objs >> +++ b/hw/tpm/Makefile.objs >> @@ -1,2 +1,2 @@ >> common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o >> -common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o >> +common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o >> tpm_util.o >> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h index 24e12ce..edab824 >> 100644 >> --- a/hw/tpm/tpm_int.h >> +++ b/hw/tpm/tpm_int.h >> @@ -66,4 +66,10 @@ struct tpm_resp_hdr { #define >> TPM_ORD_ContinueSelfTest 0x53 >> #define TPM_ORD_GetTicks 0xf1 >> >> + >> +/* TPM2 defines */ >> +#define TPM_ST_NO_SESSIONS 0x8001 >> + >> +#define TPM_CC_ReadClock 0x00000181 >> + > Could you define TPM2 macro definitions beginning with 'TPM2_*'? Ok, will do. [...] > +/* > + * Probe for the TPM device in the back > + * Returns 0 on success with the version of the probed TPM set, 1 on failure. > + */ > +int tpm_util_test_tpmdev(int tpm_fd, enum TPMVersion *tpm_version) { > + /* > + * Sending a TPM1.2 command to a TPM2 should return a TPM1.2 > + * header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e) > + * > + * Sending a TPM2 command to a TPM 2 will give a TPM 2 tag in the > + * header. > + * Sending a TPM2 command to a TPM 1.2 will give a TPM 1.2 tag > + * in the header and an error code. > + */ > + const struct tpm_req_hdr test_req = { > + .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND), > + .len = cpu_to_be32(sizeof(test_req)), > + .ordinal = cpu_to_be32(TPM_ORD_GetTicks), > + }; > + > + const struct tpm_req_hdr test_req_tpm2 = { > + .tag = cpu_to_be16(TPM_ST_NO_SESSIONS), > + .len = cpu_to_be32(sizeof(test_req_tpm2)), > + .ordinal = cpu_to_be32(TPM_CC_ReadClock), > + }; > + uint16_t returnTag; > + int ret; > + > + /* Send TPM 2 command */ > + ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2, > + sizeof(test_req_tpm2), &returnTag); > + /* TPM 2 would respond with a tag of TPM_ST_NO_SESSIONS */ > + if (!ret && returnTag == TPM_ST_NO_SESSIONS) { > + *tpm_version = TPMVersion2_0; > + return 0; > + } > + > + /* Send TPM 1.2 command */ > + ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req, > + sizeof(test_req), &returnTag); > + if (!ret && returnTag == TPM_TAG_RSP_COMMAND) { > + *tpm_version = TPMVersion1_2; > + /* this is a TPM 1.2 */ > + return 0; > + } > + > + *tpm_version = TPMVersion_Unspec; > + > + return 1; > +} > > In my opinion, I prefer to point out tpm_version in QEMU command line options, then > tpm_util_test_tpmdev() tries to verify it. The only reason why I am not doing this was that libvirt for example will need to probe for whether the additional parameter indicating the TPM version is supported. Besides that I thought it should be possible to probe on any platform and get a reliable result. Maybe Eric has a comment. I have recently seen a discussion where an additional parameter to an existing option was to be added, but cannot remember which option that was. Stefan