From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49502) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yhmwu-0002Nt-Cc for qemu-devel@nongnu.org; Mon, 13 Apr 2015 18:35:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yhmwq-0001Nj-AS for qemu-devel@nongnu.org; Mon, 13 Apr 2015 18:35:24 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:49438) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yhmwq-0001NM-3Z for qemu-devel@nongnu.org; Mon, 13 Apr 2015 18:35:20 -0400 Received: from /spool/local by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 Apr 2015 16:35:16 -0600 Received: from b03cxnp07028.gho.boulder.ibm.com (b03cxnp07028.gho.boulder.ibm.com [9.17.130.15]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id A0A101FF001F for ; Mon, 13 Apr 2015 16:26:23 -0600 (MDT) Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by b03cxnp07028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t3DMXKYM34734174 for ; Mon, 13 Apr 2015 15:33:20 -0700 Received: from d03av05.boulder.ibm.com (localhost [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t3DMZB5s021567 for ; Mon, 13 Apr 2015 16:35:12 -0600 Message-ID: <552C449E.6040308@linux.vnet.ibm.com> Date: Mon, 13 Apr 2015 18:35:10 -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> <552ADA7B.2050402@linux.vnet.ibm.com> In-Reply-To: <552ADA7B.2050402@linux.vnet.ibm.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/12/2015 04:50 PM, Stefan Berger wrote: > 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 >> >> +#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); >> Put this into tpm_util.h. >> 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. Actually, please put the part that splits out the tpm_util_is_selftest function into tpm_util.c,.h and the changes to the Makefile into a patch before your current patch 3. Stefan