* Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
@ 2015-01-19 17:15 ` Stefano Stabellini
0 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2015-01-19 17:15 UTC (permalink / raw)
To: Quan Xu; +Cc: stefano.stabellini, qemu-devel, xen-devel
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@intel.com>
> ---
> 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@intel.com>
> + *
> + * 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.
> 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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
2015-01-19 17:15 ` Stefano Stabellini
(?)
@ 2015-01-20 3:21 ` Xu, Quan
2015-01-20 10:49 ` Stefano Stabellini
2015-01-20 10:49 ` [Qemu-devel] " Stefano Stabellini
-1 siblings, 2 replies; 15+ messages in thread
From: Xu, Quan @ 2015-01-20 3:21 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: qemu-devel, xen-devel
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Tuesday, January 20, 2015 1:15 AM
> To: Xu, Quan
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> stefano.stabellini@eu.citrix.com
> 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@intel.com>
> > ---
> > 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@intel.com>
> > + *
> > + * 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.
-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
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
2015-01-20 3:21 ` [Qemu-devel] " Xu, Quan
@ 2015-01-20 10:49 ` Stefano Stabellini
2015-01-20 10:49 ` [Qemu-devel] " Stefano Stabellini
1 sibling, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2015-01-20 10:49 UTC (permalink / raw)
To: Xu, Quan; +Cc: xen-devel, qemu-devel, Stefano Stabellini
On Tue, 20 Jan 2015, Xu, Quan wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: Tuesday, January 20, 2015 1:15 AM
> > To: Xu, Quan
> > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > stefano.stabellini@eu.citrix.com
> > 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@intel.com>
> > > ---
> > > 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@intel.com>
> > > + *
> > > + * 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.
> -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
> > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
2015-01-20 3:21 ` [Qemu-devel] " Xu, Quan
2015-01-20 10:49 ` Stefano Stabellini
@ 2015-01-20 10:49 ` Stefano Stabellini
2015-01-20 14:11 ` Xu, Quan
2015-01-20 14:11 ` [Qemu-devel] " Xu, Quan
1 sibling, 2 replies; 15+ messages in thread
From: Stefano Stabellini @ 2015-01-20 10:49 UTC (permalink / raw)
To: Xu, Quan; +Cc: xen-devel, qemu-devel, Stefano Stabellini
On Tue, 20 Jan 2015, Xu, Quan wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: Tuesday, January 20, 2015 1:15 AM
> > To: Xu, Quan
> > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > stefano.stabellini@eu.citrix.com
> > 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@intel.com>
> > > ---
> > > 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@intel.com>
> > > + *
> > > + * 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.
> -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
> > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
2015-01-20 10:49 ` [Qemu-devel] " Stefano Stabellini
@ 2015-01-20 14:11 ` Xu, Quan
2015-01-20 14:11 ` [Qemu-devel] " Xu, Quan
1 sibling, 0 replies; 15+ messages in thread
From: Xu, Quan @ 2015-01-20 14:11 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: qemu-devel, xen-devel
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Tuesday, January 20, 2015 6:50 PM
> To: Xu, Quan
> Cc: Stefano Stabellini; qemu-devel@nongnu.org; xen-devel@lists.xen.org
> 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@eu.citrix.com]
> > > Sent: Tuesday, January 20, 2015 1:15 AM
> > > To: Xu, Quan
> > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > > stefano.stabellini@eu.citrix.com
> > > 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@intel.com>
> > > > ---
> > > > 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@intel.com>
> > > > + *
> > > > + * 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
> > > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
2015-01-20 10:49 ` [Qemu-devel] " Stefano Stabellini
2015-01-20 14:11 ` Xu, Quan
@ 2015-01-20 14:11 ` Xu, Quan
1 sibling, 0 replies; 15+ messages in thread
From: Xu, Quan @ 2015-01-20 14:11 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: qemu-devel, xen-devel
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Tuesday, January 20, 2015 6:50 PM
> To: Xu, Quan
> Cc: Stefano Stabellini; qemu-devel@nongnu.org; xen-devel@lists.xen.org
> 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@eu.citrix.com]
> > > Sent: Tuesday, January 20, 2015 1:15 AM
> > > To: Xu, Quan
> > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > > stefano.stabellini@eu.citrix.com
> > > 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@intel.com>
> > > > ---
> > > > 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@intel.com>
> > > > + *
> > > > + * 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
> > > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
2015-01-19 17:15 ` Stefano Stabellini
(?)
(?)
@ 2015-01-20 3:21 ` Xu, Quan
-1 siblings, 0 replies; 15+ messages in thread
From: Xu, Quan @ 2015-01-20 3:21 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: qemu-devel, xen-devel
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Tuesday, January 20, 2015 1:15 AM
> To: Xu, Quan
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> stefano.stabellini@eu.citrix.com
> 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@intel.com>
> > ---
> > 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@intel.com>
> > + *
> > + * 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.
-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
> >
^ permalink raw reply [flat|nested] 15+ messages in thread