From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34757) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YiOYa-0001WI-CT for qemu-devel@nongnu.org; Wed, 15 Apr 2015 10:44:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YiOYX-0005on-DY for qemu-devel@nongnu.org; Wed, 15 Apr 2015 10:44:48 -0400 Received: from e17.ny.us.ibm.com ([129.33.205.207]:52655) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YiOYX-0005oj-84 for qemu-devel@nongnu.org; Wed, 15 Apr 2015 10:44:45 -0400 Received: from /spool/local by e17.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 15 Apr 2015 10:44:44 -0400 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 828FC38C8063 for ; Wed, 15 Apr 2015 10:44:38 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t3FEiZZk21299210 for ; Wed, 15 Apr 2015 14:44:37 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 t3FEiWSC009763 for ; Wed, 15 Apr 2015 10:44:33 -0400 Message-ID: <552E7950.7030806@linux.vnet.ibm.com> Date: Wed, 15 Apr 2015 10:44:32 -0400 From: Stefan Berger MIME-Version: 1.0 References: <1428649159-30879-1-git-send-email-quan.xu@intel.com> <1428649159-30879-4-git-send-email-quan.xu@intel.com> In-Reply-To: <1428649159-30879-4-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 3/6] Qemu-Xen-vTPM: Xen frontend driver infrastructure 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: > This patch adds infrastructure for xen front drivers living in qemu, > so drivers don't need to implement common stuff on their own. It's > mostly xenbus management stuff: some functions to access XenStore, > setting up XenStore watches, callbacks on device discovery and state > changes, and handle event channel between the virtual machines. > > Call xen_fe_register() function to register XenDevOps, and make sure, > [...] > 3 = "" > [...] > device = "" (frontend device, the backend is running in QEMU/.etc) > vkbd = "" > [...] > vif = "" > [...] > > .. > > (QEMU) xen_vtpmdev_ops is initialized with the following process: > xen_hvm_init() > [...] > -->xen_fe_register("vtpm", ...) > -->xenstore_fe_scan() > -->xen_fe_try_init(ops) > --> XenDevOps.init() > -->xen_fe_get_xendev() > --> XenDevOps.alloc() > -->xen_fe_check() > -->xen_fe_try_initialise() > --> XenDevOps.initialise() > -->xen_fe_try_connected() > --> XenDevOps.connected() > -->xs_watch() > [...] > > Signed-off-by: Quan Xu > > --Changes in v5: > -Comments enhancement. > -Pass the size of the buff for vtpm_recv()|vtpm_send(). > --- > hw/tpm/Makefile.objs | 1 + > hw/tpm/xen_vtpm_frontend.c | 321 +++++++++++++++++++++++++++++++++++++++++++ > hw/xen/xen_frontend.c | 20 +++ > include/hw/xen/xen_backend.h | 7 + > include/hw/xen/xen_common.h | 6 + > xen-hvm.c | 5 + > 6 files changed, 360 insertions(+) > create mode 100644 hw/tpm/xen_vtpm_frontend.c > > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs > index 99f5983..57919fa 100644 > --- a/hw/tpm/Makefile.objs > +++ b/hw/tpm/Makefile.objs > @@ -1,2 +1,3 @@ > common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o > common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o > +common-obj-$(CONFIG_TPM_XENSTUBDOMS) += xen_vtpm_frontend.o > diff --git a/hw/tpm/xen_vtpm_frontend.c b/hw/tpm/xen_vtpm_frontend.c > new file mode 100644 > index 0000000..3ca9501 > --- /dev/null > +++ b/hw/tpm/xen_vtpm_frontend.c > @@ -0,0 +1,321 @@ > +/* > + * Connect to Xen vTPM stubdom domain > + * > + * Copyright (c) 2015 Intel Corporation > + * Authors: > + * Quan Xu > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "hw/hw.h" > +#include "block/aio.h" > +#include "hw/xen/xen_backend.h" > + > +#ifndef XS_STUBDOM_VTPM_ENABLE > +#define XS_STUBDOM_VTPM_ENABLE "1" > +#endif > + > +#ifndef PAGE_SIZE > +#define PAGE_SIZE 4096 > +#endif > + > +enum tpmif_state { > + /* No contents, vTPM idle, cancel complete */ > + TPMIF_STATE_IDLE, > + /* Request ready or vTPM working */ > + TPMIF_STATE_SUBMIT, > + /* Response ready or vTPM idle */ > + TPMIF_STATE_FINISH, > + /* Cancel requested or vTPM working */ > + TPMIF_STATE_CANCEL, > +}; > + > +static AioContext *vtpm_aio_ctx; > + > +enum status_bits { > + VTPM_STATUS_RUNNING = 0x1, > + VTPM_STATUS_IDLE = 0x2, > + VTPM_STATUS_RESULT = 0x4, > + VTPM_STATUS_CANCELED = 0x8, > +}; > + > +struct tpmif_shared_page { > + /* Request and response length in bytes */ > + uint32_t length; > + /* Enum tpmif_state */ > + uint8_t state; > + /* For the current request */ > + uint8_t locality; > + /* Should be zero */ > + uint8_t pad; > + /* Extra pages for long packets; may be zero */ > + uint8_t nr_extra_pages; > + /* > + * Grant IDs, the length is actually nr_extra_pages. > + * beyond the extra_pages entries is the actual requese > + * and response. > + */ requese->request > + uint32_t extra_pages[0]; > +}; > + > +struct xen_vtpm_dev { > + struct XenDevice xendev; /* must be first */ > + struct tpmif_shared_page *shr; > + xc_gntshr *xen_xcs; > + int ring_ref; > + int bedomid; > + QEMUBH *sr_bh; > +}; > + > +static uint8_t vtpm_status(struct xen_vtpm_dev *vtpmdev) > +{ > + switch (vtpmdev->shr->state) { > + case TPMIF_STATE_IDLE: > + case TPMIF_STATE_FINISH: > + return VTPM_STATUS_IDLE; > + case TPMIF_STATE_SUBMIT: > + case TPMIF_STATE_CANCEL: > + return VTPM_STATUS_RUNNING; > + default: > + return 0; > + } > +} > + > +static bool vtpm_aio_wait(AioContext *ctx) > +{ > + return aio_poll(ctx, true); > +} > + > +static void sr_bh_handler(void *opaque) > +{ > +} > + > +int vtpm_recv(struct XenDevice *xendev, uint8_t* buf, uint32_t buf_size, > + size_t *count) > +{ > + struct xen_vtpm_dev *vtpmdev = container_of(xendev, struct xen_vtpm_dev, > + xendev); > + struct tpmif_shared_page *shr = vtpmdev->shr; > + unsigned int offset; > + size_t length = shr->length; > + > + if (shr->state == TPMIF_STATE_IDLE) { > + return -ECANCELED; > + } > + > + offset = sizeof(*shr) + sizeof(shr->extra_pages[0])*shr->nr_extra_pages; offset now points to where the TPM response starts, right? > + if (offset > buf_size) { You are comparing offset as if it was the size of the TPM response, but that's not what it is as far as I understand this. I would have thought that shr->length indicates the size of the TPM response that starts at offset. So then you should only have to ensure that shr->length <= buf_size and never copy more than buf_size bytes to buf. Similar comments to vtpm_send. > + return -EIO; > + } > + > + if (offset + length > buf_size) { > + length = buf_size - offset; > + } > + > + if (length > *count) { > + length = *count; > + } > + > + memcpy(buf, offset + (uint8_t *)shr, shr->length); use length rather than shr->length otherwise length goes unused. Rest looks good. Stefan