From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36324) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YhOpa-0002JK-6V for qemu-devel@nongnu.org; Sun, 12 Apr 2015 16:50:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YhOpW-0006TJ-Vc for qemu-devel@nongnu.org; Sun, 12 Apr 2015 16:50:14 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:43738) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YhOpW-0006SN-Qm for qemu-devel@nongnu.org; Sun, 12 Apr 2015 16:50:10 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 12 Apr 2015 16:50:07 -0400 Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id CF5DD6E8045 for ; Sun, 12 Apr 2015 16:41:53 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by b01cxnp23034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t3CKo44H30671086 for ; Sun, 12 Apr 2015 20:50:04 GMT Received: from d01av03.pok.ibm.com (localhost [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t3CKo3T0005184 for ; Sun, 12 Apr 2015 16:50:04 -0400 Message-ID: <552ADA7B.2050402@linux.vnet.ibm.com> Date: Sun, 12 Apr 2015 16:50:03 -0400 From: Stefan Berger MIME-Version: 1.0 References: <1428649159-30879-1-git-send-email-quan.xu@intel.com> <1428649159-30879-7-git-send-email-quan.xu@intel.com> In-Reply-To: <1428649159-30879-7-git-send-email-quan.xu@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 6/6] Qemu-Xen-vTPM: Add a parameter indicating whether the command that was a selftest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Quan Xu , stefano.stabellini@eu.citrix.com, eblake@redhat.com Cc: wei.liu2@citrix.com, qemu-devel@nongnu.org, xen-devel@lists.xen.org, aliguori@amazon.com, pbonzini@redhat.com, dgdegra@tycho.nsa.gov On 04/10/2015 02:59 AM, Quan Xu wrote: > and whether it completed successfully. Move tpm_passthrough_is_selftest() into > tpm_util.c and rename it to tpm_util_is_selftest(). > > Signed-off-by: Quan Xu > --- > hw/tpm/Makefile.objs | 2 +- > hw/tpm/tpm_passthrough.c | 13 +---------- > hw/tpm/tpm_util.c | 50 ++++++++++++++++++++++++++++++++++++++++ > hw/tpm/tpm_xenstubdoms.c | 36 +++++++++++++++++++++++------ > include/sysemu/tpm_backend_int.h | 1 + > 5 files changed, 82 insertions(+), 20 deletions(-) > create mode 100644 hw/tpm/tpm_util.c > > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs > index 190e776..cba961c 100644 > --- a/hw/tpm/Makefile.objs > +++ b/hw/tpm/Makefile.objs > @@ -1,3 +1,3 @@ > -common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o > +common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o tpm_util.o > common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o > common-obj-$(CONFIG_TPM_XENSTUBDOMS) += tpm_xenstubdoms.o xen_vtpm_frontend.o > diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c > index 2a45071..ff08e15 100644 > --- a/hw/tpm/tpm_passthrough.c > +++ b/hw/tpm/tpm_passthrough.c > @@ -112,17 +112,6 @@ static void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len) > } > } > > -static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len) > -{ > - struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in; > - > - if (in_len >= sizeof(*hdr)) { > - return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest); > - } > - > - return false; > -} > - > static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, > const uint8_t *in, uint32_t in_len, > uint8_t *out, uint32_t out_len, > @@ -136,7 +125,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, > tpm_pt->tpm_executing = true; > *selftest_done = false; > > - is_selftest = tpm_passthrough_is_selftest(in, in_len); > + is_selftest = tpm_util_is_selftest(in, in_len); > > ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len); > if (ret != in_len) { > diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c > new file mode 100644 > index 0000000..8566781 > --- /dev/null > +++ b/hw/tpm/tpm_util.c > @@ -0,0 +1,50 @@ > +/* > + * TPM util functions > + * > + * * Copyright (c) 2015 Intel Corporation > + * Authors: > + * Quan Xu > + * > + * Copyright (c) 2010 - 2013 IBM Corporation > + * Authors: > + * Stefan Berger > + * > + * Copyright (C) 2011 IAIK, Graz University of Technology > + * Author: Andreas Niederl I don't think that for this particular function this Copyright applies. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > + */ > + > +#include > + > +#include "qemu-common.h" > +#include "qapi/error.h" > +#include "qemu/sockets.h" > +#include "sysemu/tpm_backend.h" > +#include "tpm_int.h" > +#include "hw/hw.h" > +#include "hw/i386/pc.h" > +#include "sysemu/tpm_backend_int.h" > +#include "tpm_tis.h" Can you reduce the includes to its minimum? > + > +bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len) > +{ > + struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in; > + > + if (in_len >= sizeof(*hdr)) { > + return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest); > + } > + > + return false; > +} > diff --git a/hw/tpm/tpm_xenstubdoms.c b/hw/tpm/tpm_xenstubdoms.c > index 3d046fc..992f2ae 100644 > --- a/hw/tpm/tpm_xenstubdoms.c > +++ b/hw/tpm/tpm_xenstubdoms.c > @@ -65,11 +65,17 @@ typedef struct TPMXenstubdomsState TPMXenstubdomsState; > /* Functions */ > static void tpm_xenstubdoms_cancel_cmd(TPMBackend *tb); > > -static int tpm_xenstubdoms_unix_transfer(const TPMLocality *locty_data) > +static int tpm_xenstubdoms_unix_transfer(const TPMLocality *locty_data, > + bool *selftest_done) > { > size_t rlen; > struct XenDevice *xendev; > int ret; > + bool is_selftest; > + const struct tpm_resp_hdr *hdr; > + > + is_selftest = tpm_util_is_selftest(locty_data->w_buffer.buffer, > + locty_data->w_buffer.size); > > xendev = xen_find_xendev("vtpm", xen_domid, xenstore_dev); > if (xendev == NULL) { > @@ -81,12 +87,26 @@ static int tpm_xenstubdoms_unix_transfer(const TPMLocality *locty_data) > locty_data->r_buffer.size, locty_data->w_offset); > if (ret < 0) { > xen_be_printf(xendev, 0, "Can not send vtpm command.\n"); > - return -1; > + goto err_exit; > } > > - vtpm_recv(xendev, locty_data->r_buffer.buffer, locty_data->r_buffer.size, > - &rlen); > - return 0; > + ret = vtpm_recv(xendev, locty_data->r_buffer.buffer, > + locty_data->r_buffer.size, &rlen); > + if (ret < 0) { > + xen_be_printf(xendev, 0, "vtpm reception command error.\n"); > + goto err_exit; > + } > + > + if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) { > + hdr = (struct tpm_resp_hdr *)locty_data->r_buffer.buffer; > + *selftest_done = (be32_to_cpu(hdr->errcode) == 0); > + } > + > +err_exit: > + if (ret < 0) { > + xen_be_printf(xendev, 0, "vtpm command error.\n"); > + } > + return ret; > } > > static void tpm_xenstubdoms_worker_thread(gpointer data, > @@ -94,13 +114,15 @@ static void tpm_xenstubdoms_worker_thread(gpointer data, > { > TPMXenstubdomsThreadParams *thr_parms = user_data; > TPMBackendCmd cmd = (TPMBackendCmd)data; > + bool selftest_done = false; > > switch (cmd) { > case TPM_BACKEND_CMD_PROCESS_CMD: > - tpm_xenstubdoms_unix_transfer(thr_parms->tpm_state->locty_data); > + tpm_xenstubdoms_unix_transfer(thr_parms->tpm_state->locty_data, > + &selftest_done); > thr_parms->recv_data_callback(thr_parms->tpm_state, > thr_parms->tpm_state->locty_number, > - false); > + selftest_done); > break; > case TPM_BACKEND_CMD_INIT: > case TPM_BACKEND_CMD_END: > diff --git a/include/sysemu/tpm_backend_int.h b/include/sysemu/tpm_backend_int.h > index 05d94d0..e18acab 100644 > --- a/include/sysemu/tpm_backend_int.h > +++ b/include/sysemu/tpm_backend_int.h > @@ -34,6 +34,7 @@ void tpm_backend_thread_create(TPMBackendThread *tbt, > void tpm_backend_thread_end(TPMBackendThread *tbt); > void tpm_backend_thread_tpm_reset(TPMBackendThread *tbt, > GFunc func, gpointer user_data); > +bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len); > > typedef enum TPMBackendCmd { > TPM_BACKEND_CMD_INIT = 1, This patch per se looks good, but some of your modifications will not compile until this patch is applied, due to the missing selftest_done parameter. So you should merge this patch into the 3rd patch in this series. Stefan