[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
> -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@xxxxxxxxxxxxx] > Sent: Tuesday, January 20, 2015 6:50 PM > To: Xu, Quan > Cc: Stefano Stabellini; qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx > Subject: RE: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend > driver > > On Tue, 20 Jan 2015, Xu, Quan wrote: > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:stefano.stabellini@xxxxxxxxxxxxx] > > > Sent: Tuesday, January 20, 2015 1:15 AM > > > To: Xu, Quan > > > Cc: qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; > > > stefano.stabellini@xxxxxxxxxxxxx > > > Subject: Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM > > > frontend driver > > > > > > On Tue, 30 Dec 2014, Quan Xu wrote: > > > > This drvier transfers any request/repond between TPM xenstubdoms > > > > driver and Xen vTPM stubdom, and facilitates communications > > > > between Xen vTPM stubdom domain and vTPM xenstubdoms driver. It is > > > > a glue for the TPM xenstubdoms driver and Xen stubdom vTPM domain > > > > that provides the actual TPM functionality. > > > > > > > > (Xen) Xen backend driver should run before running this frontend, > > > > and initialize XenStore as the following for communication. > > > > > > > > [XenStore] > > > > .. > > > > FE.DOMAIN.ID > > > > device = "" > > > > vtpm = "" > > > > 0 = "" > > > > backend = > > > "/local/domain/{BE.DOMAIN.ID}/backend/vtpm/{FE.DOMAIN.ID}/0" > > > > backend-id = "BE.DOMAIN.ID" > > > > state = "1" > > > > handle = "0" > > > > .. > > > > > > > > (QEMU) xen_vtpmdev_ops is initialized with the following process: > > > > xen_hvm_init() > > > > [...] > > > > -->xen_fe_register("vtpm", ...) > > > > -->xenstore_fe_scan() > > > > -->xen_fe_get_xendev() > > > > --> XenDevOps.alloc() > > > > -->xen_fe_check() > > > > --> XenDevOps.init() > > > > --> XenDevOps.initialise() > > > > --> XenDevOps.connected() > > > > -->xs_watch() > > > > [...] > > > > > > > > --Changes in v3: > > > > -Move xen_stubdom_vtpm.c to xen_vtpm_frontend.c -Read Xen vTPM > > > > status via XenStore > > > > > > > > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx> > > > > --- > > > > hw/tpm/Makefile.objs | 1 + > > > > hw/tpm/xen_vtpm_frontend.c | 264 > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > hw/xen/xen_backend.c | 34 ++++++ > > > > include/hw/xen/xen_backend.h | 9 +- > > > > include/hw/xen/xen_common.h | 6 + > > > > xen-hvm.c | 16 +++ > > > > 6 files changed, 328 insertions(+), 2 deletions(-) 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..00cc888 > > > > --- /dev/null > > > > +++ b/hw/tpm/xen_vtpm_frontend.c > > > > @@ -0,0 +1,264 @@ > > > > +/* > > > > + * Connect to Xen vTPM stubdom domain > > > > + * > > > > + * Copyright (c) 2014 Intel Corporation > > > > + * Authors: > > > > + * Quan Xu <quan.xu@xxxxxxxxx> > > > > + * > > > > + * 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 > > > > +<http://www.gnu.org/licenses/> */ > > > > + > > > > +#include <stdio.h> > > > > +#include <stdlib.h> > > > > +#include <stdarg.h> > > > > +#include <string.h> > > > > +#include <unistd.h> > > > > +#include <signal.h> > > > > +#include <inttypes.h> > > > > +#include <time.h> > > > > +#include <fcntl.h> > > > > +#include <errno.h> > > > > +#include <sys/ioctl.h> > > > > +#include <sys/types.h> > > > > +#include <sys/stat.h> > > > > +#include <sys/mman.h> > > > > +#include <sys/uio.h> > > > > + > > > > +#include "hw/hw.h" > > > > +#include "block/aio.h" > > > > +#include "hw/xen/xen_backend.h" > > > > + > > > > +enum tpmif_state { > > > > + TPMIF_STATE_IDLE, /* no contents / vTPM idle / cancel > > > complete */ > > > > + TPMIF_STATE_SUBMIT, /* request ready / vTPM working */ > > > > + TPMIF_STATE_FINISH, /* response ready / vTPM idle */ > > > > + TPMIF_STATE_CANCEL, /* cancel requested / vTPM working */ > > > > +}; > > > > + > > > > +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 { > > > > + uint32_t length; /* request/response length in bytes */ > > > > + > > > > + uint8_t state; /* enum tpmif_state */ > > > > + uint8_t locality; /* for the current request */ > > > > + uint8_t pad; /* should be zero */ > > > > + > > > > + uint8_t nr_extra_pages; /* extra pages for long packets; > > > > + may be zero > > > */ > > > > + uint32_t extra_pages[0]; /* grant IDs; length is actually > > > > +nr_extra_pages */ }; > > > > + > > > > +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, 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; > > > > + > > > > + if (shr->state == TPMIF_STATE_IDLE) { > > > > + return -ECANCELED; > > > > + } > > > > + > > > > + while (vtpm_status(vtpmdev) != VTPM_STATUS_IDLE) { > > > > + vtpm_aio_wait(vtpm_aio_ctx); > > > > + } > > > > + > > > > + offset = sizeof(*shr) + 4*shr->nr_extra_pages; > > > > + memcpy(buf, offset + (uint8_t *)shr, shr->length); > > > > + *count = shr->length; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int vtpm_send(struct XenDevice *xendev, uint8_t* buf, 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 = sizeof(*shr) + 4*shr->nr_extra_pages; > > > > + > > > > + while (vtpm_status(vtpmdev) != VTPM_STATUS_IDLE) { > > > > + vtpm_aio_wait(vtpm_aio_ctx); > > > > + } > > > > + > > > > + memcpy(offset + (uint8_t *)shr, buf, count); > > > > + shr->length = count; > > > > + barrier(); > > > > + shr->state = TPMIF_STATE_SUBMIT; > > > > + xen_wmb(); > > > > + xen_be_send_notify(&vtpmdev->xendev); > > > > + > > > > + while (vtpm_status(vtpmdev) != VTPM_STATUS_IDLE) { > > > > + vtpm_aio_wait(vtpm_aio_ctx); > > > > + } > > > > + > > > > + return count; > > > > +} > > > > + > > > > +static int vtpm_initialise(struct XenDevice *xendev) { > > > > + struct xen_vtpm_dev *vtpmdev = container_of(xendev, struct > > > xen_vtpm_dev, > > > > + xendev); > > > > + xs_transaction_t xbt = XBT_NULL; > > > > + unsigned int ring_ref; > > > > + > > > > + vtpmdev->xendev.fe = xenstore_read_be_str(&vtpmdev->xendev, > > > "frontend"); > > > > + if (vtpmdev->xendev.fe == NULL) { > > > > + return -1; > > > > + } > > > > + > > > > + /* Get backend domid */ > > > > + if (xenstore_read_fe_int(&vtpmdev->xendev, "backend-id", > > > > + &vtpmdev->bedomid)) { > > > > + return -1; > > > > + } > > > > + > > > > + /*alloc share page*/ > > > > + vtpmdev->shr = xc_gntshr_share_pages(vtpmdev->xen_xcs, > > > vtpmdev->bedomid, 1, > > > > + &ring_ref, > > > PROT_READ|PROT_WRITE); > > > > + vtpmdev->ring_ref = ring_ref; > > > > + if (vtpmdev->shr == NULL) { > > > > + return -1; > > > > + } > > > > + > > > > + /*Create event channel */ > > > > + if (xen_be_alloc_unbound(&vtpmdev->xendev, 0, > > > > + vtpmdev->bedomid)) > > > { > > > > + xc_gntshr_munmap(vtpmdev->xen_xcs, vtpmdev->shr, 1); > > > > + return -1; > > > > + } > > > > + xc_evtchn_unmask(vtpmdev->xendev.evtchndev, > > > > + vtpmdev->xendev.local_port); > > > > + > > > > +again: > > > > + xbt = xs_transaction_start(xenstore); > > > > + if (xbt == XBT_NULL) { > > > > + goto abort_transaction; > > > > + } > > > > + > > > > + if (xenstore_write_int(vtpmdev->xendev.fe, "ring-ref", > > > > + vtpmdev->ring_ref)) { > > > > + goto abort_transaction; > > > > + } > > > > + > > > > + if (xenstore_write_int(vtpmdev->xendev.fe, "event-channel", > > > > + vtpmdev->xendev.local_port)) { > > > > + goto abort_transaction; > > > > + } > > > > + > > > > + /* Publish protocol v2 feature */ > > > > + if (xenstore_write_int(vtpmdev->xendev.fe, "feature-protocol-v2", > 1)) { > > > > + goto abort_transaction; > > > > + } > > > > + > > > > + if (!xs_transaction_end(xenstore, xbt, 0)) { > > > > + if (errno == EAGAIN) { > > > > + goto again; > > > > + } > > > > + } > > > > + /* Tell vtpm backend that we are ready */ > > > > + xenbus_switch_state(&vtpmdev->xendev, > > > > + XenbusStateInitialised); > > > > + > > > > + return 0; > > > > + > > > > +abort_transaction: > > > > + xc_gntshr_munmap(vtpmdev->xen_xcs, vtpmdev->shr, 1); > > > > + xs_transaction_end(xenstore, xbt, 1); > > > > + return -1; > > > > +} > > > > + > > > > +static int vtpm_free(struct XenDevice *xendev) { > > > > + struct xen_vtpm_dev *vtpmdev = container_of(xendev, struct > > > xen_vtpm_dev, > > > > + xendev); > > > > + aio_poll(vtpm_aio_ctx, false); > > > > + qemu_bh_delete(vtpmdev->sr_bh); > > > > + if (vtpmdev->shr) { > > > > + xc_gntshr_munmap(vtpmdev->xen_xcs, vtpmdev->shr, 1); > > > > + } > > > > + xc_interface_close(vtpmdev->xen_xcs); > > > > + return 0; > > > > +} > > > > + > > > > +static void vtpm_alloc(struct XenDevice *xendev) { > > > > + struct xen_vtpm_dev *vtpmdev = container_of(xendev, struct > > > xen_vtpm_dev, > > > > + xendev); > > > > + > > > > + vtpm_aio_ctx = aio_context_new(NULL); > > > > + if (vtpm_aio_ctx == NULL) { > > > > + return; > > > > + } > > > > + vtpmdev->sr_bh = aio_bh_new(vtpm_aio_ctx, sr_bh_handler, > > > vtpmdev); > > > > + qemu_bh_schedule(vtpmdev->sr_bh); > > > > + vtpmdev->xen_xcs = xen_xc_gntshr_open(0, 0); } > > > > + > > > > +static void vtpm_event(struct XenDevice *xendev) { > > > > + struct xen_vtpm_dev *vtpmdev = container_of(xendev, struct > > > xen_vtpm_dev, > > > > + xendev); > > > > + > > > > + qemu_bh_schedule(vtpmdev->sr_bh); } > > > > + > > > > +struct XenDevOps xen_vtpmdev_ops = { > > > > + .size = sizeof(struct xen_vtpm_dev), > > > > + .flags = DEVOPS_FLAG_IGNORE_STATE | > > > > + DEVOPS_FLAG_FE, > > > > + .event = vtpm_event, > > > > + .free = vtpm_free, > > > > + .alloc = vtpm_alloc, > > > > + .initialise = vtpm_initialise, > > > > + .backend_changed = vtpm_backend_changed, }; > > > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index > > > > ad6e324..377a9c8 100644 > > > > --- a/hw/xen/xen_backend.c > > > > +++ b/hw/xen/xen_backend.c > > > > @@ -741,6 +741,20 @@ int xen_be_register(const char *type, struct > > > XenDevOps *ops) > > > > return xenstore_scan(type, xen_domid, ops); } > > > > > > > > +int xen_be_alloc_unbound(struct XenDevice *xendev, int dom, int > > > > +remote_dom) { > > > > + xendev->local_port = > > > xc_evtchn_bind_unbound_port(xendev->evtchndev, > > > > + > remote_dom); > > > > + if (xendev->local_port == -1) { > > > > + xen_be_printf(xendev, 0, "xc_evtchn_alloc_unbound failed\n"); > > > > + return -1; > > > > + } > > > > + xen_be_printf(xendev, 2, "bind evtchn port %d\n", > xendev->local_port); > > > > + qemu_set_fd_handler(xc_evtchn_fd(xendev->evtchndev), > > > > + xen_be_evtchn_event, NULL, xendev); > > > > + return 0; > > > > +} > > > > > > Why are you calling this xen_be_alloc_unbound? Shouldn't it be > > > called xen_fe_alloc_unbound and be in xen_frontend.c, given that is > > > called only by a frontend? > > > > > > > > > > int xen_be_bind_evtchn(struct XenDevice *xendev) { > > > > if (xendev->local_port != -1) { @@ -813,6 +827,26 @@ void > > > > xen_be_printf(struct XenDevice *xendev, int > > > msg_level, const char *fmt, ... > > > > qemu_log_flush(); > > > > } > > > > > > > > +void vtpm_backend_changed(struct XenDevice *xendev, const char > > > > +*node) { > > > > + int be_state; > > > > + > > > > + if (strcmp(node, "state") == 0) { > > > > + xenstore_read_be_int(xendev, node, &be_state); > > > > + switch (be_state) { > > > > + case XenbusStateConnected: > > > > + /*TODO*/ > > > > + break; > > > > + case XenbusStateClosing: > > > > + case XenbusStateClosed: > > > > + xenbus_switch_state(xendev, XenbusStateClosing); > > > > + break; > > > > + default: > > > > + break; > > > > + } > > > > + } > > > > +} > > > > > > This should be written as a generic fe function in xen_frontend.c > > > > > > > > > > void xen_qtail_insert_xendev(struct XenDevice *xendev) { > > > > QTAILQ_INSERT_TAIL(&xendevs, xendev, next); diff --git > > > > a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h > > > > index > > > > 06e202f..d07ae36 100644 > > > > --- a/include/hw/xen/xen_backend.h > > > > +++ b/include/hw/xen/xen_backend.h > > > > @@ -102,8 +102,12 @@ char *xenstore_fe_read_be_str(const char > > > > *type, int dom, int dev); int xenbus_switch_state(struct > > > > XenDevice *xendev, enum xenbus_state xbus); struct XenDevice > > > > *xen_fe_get_xendev(const > > > char *type, int dom, int dev, > > > > struct XenDevOps *ops); -void > > > > xenstore_fe_update(char *watch, char *type, int dom, > > > > - struct XenDevOps *ops); > > > > +void xenstore_fe_update(char *watch, char *type, int dom, struct > > > > +XenDevOps *ops); > > > > + > > > > +/*Xen vtpm*/ > > > > +int vtpm_send(struct XenDevice *xendev, uint8_t* buf, size_t > > > > +count); int vtpm_recv(struct XenDevice *xendev, uint8_t* buf, > > > > +size_t *count); void vtpm_backend_changed(struct XenDevice > > > > +*xendev, const char *node); > > > > > > > > /* actual backend drivers */ > > > > extern struct XenDevOps xen_console_ops; /* xen_console.c > */ > > > > @@ -111,6 +115,7 @@ extern struct XenDevOps xen_kbdmouse_ops; > /* > > > xen_framebuffer.c */ > > > > extern struct XenDevOps xen_framebuffer_ops; /* xen_framebuffer.c > */ > > > > extern struct XenDevOps xen_blkdev_ops; /* xen_disk.c > */ > > > > extern struct XenDevOps xen_netdev_ops; /* xen_nic.c > */ > > > > +extern struct XenDevOps xen_vtpmdev_ops; /* > > > xen_vtpm_frontend.c*/ > > > > > > > > void xen_init_display(int domid); > > > > > > > > diff --git a/include/hw/xen/xen_common.h > > > > b/include/hw/xen/xen_common.h index 07731b9..2e9bb62 100644 > > > > --- a/include/hw/xen/xen_common.h > > > > +++ b/include/hw/xen/xen_common.h > > > > @@ -130,6 +130,12 @@ static inline XenXC > > > > xen_xc_interface_open(void > > > *logger, void *dombuild_logger, > > > > return xc_interface_open(logger, dombuild_logger, > > > > open_flags); } > > > > > > > > +static inline xc_gntshr *xen_xc_gntshr_open(void *logger, > > > > + unsigned int open_flags) > { > > > > + return xc_gntshr_open(logger, open_flags); } > > > > + > > > > /* FIXME There is now way to have the xen fd */ static inline > > > > int xc_fd(xc_interface *xen_xc) { diff --git a/xen-hvm.c > > > > b/xen-hvm.c index 05e522c..5b93cd4 100644 > > > > --- a/xen-hvm.c > > > > +++ b/xen-hvm.c > > > > @@ -986,6 +986,12 @@ int xen_hvm_init(ram_addr_t > > > > *below_4g_mem_size, > > > ram_addr_t *above_4g_mem_size, > > > > int i, rc; > > > > unsigned long ioreq_pfn; > > > > unsigned long bufioreq_evtchn; > > > > + > > > > +#ifdef CONFIG_TPM_XENSTUBDOMS > > > > + char path[XEN_BUFSIZE]; > > > > + unsigned int stubdom_vtpm = 0; #endif > > > > + > > > > XenIOState *state; > > > > > > > > state = g_malloc0(sizeof (XenIOState)); @@ -1071,6 +1077,16 > > > > @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t > > > *above_4g_mem_size, > > > > fprintf(stderr, "%s: xen backend core setup failed\n", > > > __FUNCTION__); > > > > return -1; > > > > } > > > > + > > > > +#ifdef CONFIG_TPM_XENSTUBDOMS > > > > + snprintf(path, sizeof(path), > > > "/local/domain/%d/platform/acpi_stubdom_vtpm", > > > > + xen_domid); > > > > + xs_read(state->xenstore, 0, path, &stubdom_vtpm); > > > > + if (stubdom_vtpm) { > > > > + xen_fe_register("vtpm", &xen_vtpmdev_ops); > > > > + } > > > > +#endif > > > > > > I think you should always call xen_fe_register(vtpm ...) and move > > > this chunk in the xen_vtpmdev_ops .init function. > > > > It should get xenstore status before to register vtpm. > > xen_fe_register() > > --> xenstore_fe_scan() > > --> xen_fe_get_xendev() ... > > --> xen_fe_check() > > --> ops .init() > > > > It will do more much more redundant work, if move this chunk in this > > chunk in. maybe QEMU will crash if Xen doesn't register vtpm. I think I > > this > chunk could simplify further. > > This is just setup code, ran only once at startup time. We can afford to be a > bit > inefficient. > > Consider that is most cases, if acpi_stubdom_vtpm is missing, than the entire > vtpm directory on xenstore is going to be missing, therefore xenstore_fe_scan > won't call xen_fe_get_xendev for vtpm. > Okay, I will move this chunk in the xen_vtpmdev_ops .init function. -Quan > > > > -Quan > > > > > > > > > > > > xen_be_register("console", &xen_console_ops); > > > > xen_be_register("vkbd", &xen_kbdmouse_ops); > > > > xen_be_register("qdisk", &xen_blkdev_ops); > > > > -- > > > > 1.8.3.2 > > > > > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |