[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Qemu-devel] [PATCH v5 3/6] Qemu-Xen-vTPM: Xen frontend driver infrastructure



On 04/10/2015 02:59 AM, Quan Xu wrote:
This patch adds infrastructure for xen front drivers living in qemu,
so drivers don't need to implement common stuff on their own.  It's
mostly xenbus management stuff: some functions to access XenStore,
setting up XenStore watches, callbacks on device discovery and state
changes, and handle event channel between the virtual machines.

Call xen_fe_register() function to register XenDevOps, and make sure,
          [...]
     3 = ""
      [...]
      device = "" (frontend device, the backend is running in QEMU/.etc)
       vkbd = ""
        [...]
       vif = ""
        [...]

  ..

(QEMU) xen_vtpmdev_ops is initialized with the following process:
   xen_hvm_init()
     [...]
     -->xen_fe_register("vtpm", ...)
       -->xenstore_fe_scan()
         -->xen_fe_try_init(ops)
           --> XenDevOps.init()
         -->xen_fe_get_xendev()
           --> XenDevOps.alloc()
         -->xen_fe_check()
           -->xen_fe_try_initialise()
             --> XenDevOps.initialise()
           -->xen_fe_try_connected()
             --> XenDevOps.connected()
         -->xs_watch()
     [...]

Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>

--Changes in v5:
  -Comments enhancement.
  -Pass the size of the buff for vtpm_recv()|vtpm_send().
---
  hw/tpm/Makefile.objs         |   1 +
  hw/tpm/xen_vtpm_frontend.c   | 321 +++++++++++++++++++++++++++++++++++++++++++
  hw/xen/xen_frontend.c        |  20 +++
  include/hw/xen/xen_backend.h |   7 +
  include/hw/xen/xen_common.h  |   6 +
  xen-hvm.c                    |   5 +
  6 files changed, 360 insertions(+)
  create mode 100644 hw/tpm/xen_vtpm_frontend.c

diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index 99f5983..57919fa 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,2 +1,3 @@
  common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
  common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
+common-obj-$(CONFIG_TPM_XENSTUBDOMS) += xen_vtpm_frontend.o
diff --git a/hw/tpm/xen_vtpm_frontend.c b/hw/tpm/xen_vtpm_frontend.c
new file mode 100644
index 0000000..3ca9501
--- /dev/null
+++ b/hw/tpm/xen_vtpm_frontend.c
@@ -0,0 +1,321 @@
+/*
+ * Connect to Xen vTPM stubdom domain
+ *
+ *  Copyright (c) 2015 Intel Corporation
+ *  Authors:
+ *    Quan Xu <quan.xu@xxxxxxxxx>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+#include <signal.h>
+#include <inttypes.h>
+#include <time.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <sys/uio.h>
+
+#include "hw/hw.h"
+#include "block/aio.h"
+#include "hw/xen/xen_backend.h"
+
+#ifndef XS_STUBDOM_VTPM_ENABLE
+#define XS_STUBDOM_VTPM_ENABLE    "1"
+#endif
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE      4096
+#endif
+
+enum tpmif_state {
+    /* No contents, vTPM idle, cancel complete */
+    TPMIF_STATE_IDLE,
+    /* Request ready or vTPM working */
+    TPMIF_STATE_SUBMIT,
+    /* Response ready or vTPM idle */
+    TPMIF_STATE_FINISH,
+    /* Cancel requested or vTPM working */
+    TPMIF_STATE_CANCEL,
+};
+
+static AioContext *vtpm_aio_ctx;
+
+enum status_bits {
+    VTPM_STATUS_RUNNING  = 0x1,
+    VTPM_STATUS_IDLE     = 0x2,
+    VTPM_STATUS_RESULT   = 0x4,
+    VTPM_STATUS_CANCELED = 0x8,
+};
+
+struct tpmif_shared_page {
+    /* Request and response length in bytes */
+    uint32_t length;
+    /* Enum tpmif_state */
+    uint8_t  state;
+    /* For the current request */
+    uint8_t  locality;
+    /* Should be zero */
+    uint8_t  pad;
+    /* Extra pages for long packets; may be zero */
+    uint8_t  nr_extra_pages;
+    /*
+     * Grant IDs, the length is actually nr_extra_pages.
+     * beyond the extra_pages entries is the actual requese
+     * and response.
+     */

requese->request


+    uint32_t extra_pages[0];
+};
+
+struct xen_vtpm_dev {
+    struct XenDevice xendev;  /* must be first */
+    struct           tpmif_shared_page *shr;
+    xc_gntshr        *xen_xcs;
+    int              ring_ref;
+    int              bedomid;
+    QEMUBH           *sr_bh;
+};
+
+static uint8_t vtpm_status(struct xen_vtpm_dev *vtpmdev)
+{
+    switch (vtpmdev->shr->state) {
+    case TPMIF_STATE_IDLE:
+    case TPMIF_STATE_FINISH:
+        return VTPM_STATUS_IDLE;
+    case TPMIF_STATE_SUBMIT:
+    case TPMIF_STATE_CANCEL:
+        return VTPM_STATUS_RUNNING;
+    default:
+        return 0;
+    }
+}
+
+static bool vtpm_aio_wait(AioContext *ctx)
+{
+    return aio_poll(ctx, true);
+}
+
+static void sr_bh_handler(void *opaque)
+{
+}
+
+int vtpm_recv(struct XenDevice *xendev, uint8_t* buf, uint32_t buf_size,
+              size_t *count)
+{
+    struct xen_vtpm_dev *vtpmdev = container_of(xendev, struct xen_vtpm_dev,
+                                                xendev);
+    struct tpmif_shared_page *shr = vtpmdev->shr;
+    unsigned int offset;
+    size_t length = shr->length;
+
+    if (shr->state == TPMIF_STATE_IDLE) {
+        return -ECANCELED;
+    }
+
+    offset = sizeof(*shr) + sizeof(shr->extra_pages[0])*shr->nr_extra_pages;

offset now points to where the TPM response starts, right?

+    if (offset > buf_size) {

You are comparing offset as if it was the size of the TPM response, but that's not what it is as far as I understand this.

I would have thought that shr->length indicates the size of the TPM response that starts at offset. So then you should only have to ensure that shr->length <= buf_size and never copy more than buf_size bytes to buf.

Similar comments to vtpm_send.

+        return -EIO;
+    }
+
+    if (offset + length > buf_size) {
+        length = buf_size - offset;
+    }
+
+    if (length > *count) {
+        length = *count;
+    }
+
+    memcpy(buf, offset + (uint8_t *)shr, shr->length);

use length rather than shr->length otherwise length goes unused.


Rest looks good.

   Stefan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.