All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH vtpm v2 09/12] add iomem support to libxl
@ 2012-10-05 18:02 Matthew Fioravante
  2012-10-05 18:02 ` [PATCH vtpm v2 10/12] make devid a type so it is initialized properly Matthew Fioravante
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-05 18:02 UTC (permalink / raw)
  To: xen-devel, ian.campbell; +Cc: Matthew Fioravante

This patch adds a new option for xen config files for
directly mapping hardware io memory into a vm.

Signed-off-by: Matthew Fioravante <matthew.fioravante@jhuapl.edu>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 013270d..428da21 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -496,6 +496,17 @@ is given in hexadecimal and may either a span e.g. C<2f8-2ff>
 It is recommended to use this option only for trusted VMs under
 administrator control.
 
+=item B<iomem=[ "IOMEM_START,NUM_PAGES", "IOMEM_START,NUM_PAGES", ... ]>
+
+Allow guest to access specific hardware I/O memory pages. B<IOMEM_START>
+is a physical page number. B<NUM_PAGES> is the number
+of pages beginning with B<START_PAGE> to allow access. Both values
+must be given in hexadecimal.
+
+It is recommended to use this option only for trusted VMs under
+administrator control.
+
+
 =item B<irqs=[ NUMBER, NUMBER, ... ]>
 
 Allow a guest to access specific physical IRQs.
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index ef17f05..96f9018 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -942,7 +942,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
 
         ret = xc_domain_ioport_permission(CTX->xch, domid,
                                           io->first, io->number, 1);
-        if ( ret<0 ){
+        if ( ret < 0 ){
             LOGE(ERROR,
                  "failed give dom%d access to ioports %"PRIx32"-%"PRIx32,
                  domid, io->first, io->first + io->number - 1);
@@ -956,13 +956,31 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         LOG(DEBUG, "dom%d irq %"PRIx32, domid, irq);
 
         ret = xc_domain_irq_permission(CTX->xch, domid, irq, 1);
-        if ( ret<0 ){
+        if ( ret < 0 ){
             LOGE(ERROR,
                  "failed give dom%d access to irq %"PRId32, domid, irq);
             ret = ERROR_FAIL;
         }
     }
 
+    for (i = 0; i < d_config->b_info.num_iomem; i++) {
+        libxl_iomem_range *io = &d_config->b_info.iomem[i];
+
+        LOG(DEBUG, "dom%d iomem %"PRIx64"-%"PRIx64,
+            domid, io->start, io->start + io->number - 1);
+
+        ret = xc_domain_iomem_permission(CTX->xch, domid,
+                                          io->start, io->number, 1);
+        if ( ret < 0 ) {
+            LOGE(ERROR,
+                 "failed give dom%d access to iomem range %"PRIx64"-%"PRIx64,
+                 domid, io->start, io->start + io->number - 1);
+            ret = ERROR_FAIL;
+        }
+    }
+
+
+
     for (i = 0; i < d_config->num_nics; i++) {
         /* We have to init the nic here, because we still haven't
          * called libxl_device_nic_add at this point, but qemu needs
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6d5c578..cf83c60 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -140,6 +140,11 @@ libxl_ioport_range = Struct("ioport_range", [
     ("number", uint32),
     ])
 
+libxl_iomem_range = Struct("iomem_range", [
+    ("start", uint64),
+    ("number", uint64),
+    ])
+
 libxl_vga_interface_info = Struct("vga_interface_info", [
     ("kind",    libxl_vga_interface_type),
     ])
@@ -284,6 +289,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
     ("ioports",          Array(libxl_ioport_range, "num_ioports")),
     ("irqs",             Array(uint32, "num_irqs")),
+    ("iomem",            Array(libxl_iomem_range, "num_iomem")),
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1627cac..f13d983 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -574,8 +574,8 @@ static void parse_config_data(const char *config_source,
     long l;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
-    XLU_ConfigList *ioports, *irqs;
-    int num_ioports, num_irqs;
+    XLU_ConfigList *ioports, *irqs, *iomem;
+    int num_ioports, num_irqs, num_iomem;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
     int pci_permissive = 0;
@@ -1005,6 +1005,33 @@ static void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_list(config, "iomem", &iomem, &num_iomem, 0)) {
+        b_info->num_iomem = num_iomem;
+        b_info->iomem = calloc(num_iomem, sizeof(*b_info->iomem));
+        if (b_info->iomem == NULL) {
+            fprintf(stderr, "unable to allocate memory for iomem\n");
+            exit(-1);
+        }
+        for (i = 0; i < num_iomem; i++) {
+            buf = xlu_cfg_get_listitem (iomem, i);
+            if (!buf) {
+                fprintf(stderr,
+                        "xl: Unable to get element %d in iomem list\n", i);
+                exit(1);
+            }
+            if(sscanf(buf, "%" SCNx64",%" SCNx64, 
+                     &b_info->iomem[i].start, 
+                     &b_info->iomem[i].number) 
+                  != 2) {
+               fprintf(stderr,
+                       "xl: Invalid argument parsing iomem: %s\n", buf);
+               exit(1);
+            }
+        }
+    }
+
+
+
     if (!xlu_cfg_get_list (config, "disk", &vbds, 0, 0)) {
         d_config->num_disks = 0;
         d_config->disks = NULL;
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH vtpm v2 10/12] make devid a type so it is initialized properly
  2012-10-05 18:02 [PATCH vtpm v2 09/12] add iomem support to libxl Matthew Fioravante
@ 2012-10-05 18:02 ` Matthew Fioravante
  2012-10-05 18:02 ` [PATCH vtpm v2 11/12] add vtpm support to libxl Matthew Fioravante
  2012-10-05 18:02 ` [PATCH vtpm v2 12/12] Matthew Fioravante now maintains VTPM Matthew Fioravante
  2 siblings, 0 replies; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-05 18:02 UTC (permalink / raw)
  To: xen-devel, ian.campbell; +Cc: Matthew Fioravante

Previously device ids in libxl were treated as integers meaning they
were being initialized to 0, which is a valid device id. This patch
makes devid its own type in libxl and initializes it to -1, an invalid
value.

This fixes a bug where if you try to do a xl DEV-attach multiple
time it will continuously try to reattach device 0 instead of
generated a new device id.

Signed-off-by: Matthew Fioravante <matthew.fioravante@jhuapl.edu>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 2915f71..84b4fd7 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -60,7 +60,7 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
                                         passby=idl.PASS_BY_REFERENCE))
     elif ty.typename in ["libxl_uuid", "libxl_mac", "libxl_hwcap"]:
         s += "rand_bytes((uint8_t *)%s, sizeof(*%s));\n" % (v,v)
-    elif ty.typename in ["libxl_domid"] or isinstance(ty, idl.Number):
+    elif ty.typename in ["libxl_domid", "libxl_devid"] or isinstance(ty, idl.Number):
         s += "%s = rand() %% (sizeof(%s)*8);\n" % \
              (ty.pass_arg(v, parent is None),
               ty.pass_arg(v, parent is None))
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 599c7f1..7a7c419 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -307,6 +307,7 @@ void libxl_cpuid_dispose(libxl_cpuid_policy_list *cpuid_list);
 #define LIBXL_PCI_FUNC_ALL (~0U)
 
 typedef uint32_t libxl_domid;
+typedef int libxl_devid;
 
 /*
  * Formatting Enumerations.
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cf83c60..de111a6 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -8,6 +8,7 @@ namespace("libxl_")
 libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
 
 libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False)
+libxl_devid = Builtin("devid", json_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1")
 libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
 libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
 libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
@@ -343,7 +344,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
 libxl_device_vfb = Struct("device_vfb", [
     ("backend_domid", libxl_domid),
-    ("devid",         integer),
+    ("devid",         libxl_devid),
     ("vnc",           libxl_vnc_info),
     ("sdl",           libxl_sdl_info),
     # set keyboard layout, default is en-us keyboard
@@ -352,7 +353,7 @@ libxl_device_vfb = Struct("device_vfb", [
 
 libxl_device_vkb = Struct("device_vkb", [
     ("backend_domid", libxl_domid),
-    ("devid", integer),
+    ("devid", libxl_devid),
     ])
 
 libxl_device_disk = Struct("device_disk", [
@@ -369,7 +370,7 @@ libxl_device_disk = Struct("device_disk", [
 
 libxl_device_nic = Struct("device_nic", [
     ("backend_domid", libxl_domid),
-    ("devid", integer),
+    ("devid", libxl_devid),
     ("mtu", integer),
     ("model", string),
     ("mac", libxl_mac),
@@ -399,7 +400,7 @@ libxl_diskinfo = Struct("diskinfo", [
     ("backend_id", uint32),
     ("frontend", string),
     ("frontend_id", uint32),
-    ("devid", integer),
+    ("devid", libxl_devid),
     ("state", integer),
     ("evtch", integer),
     ("rref", integer),
@@ -410,7 +411,7 @@ libxl_nicinfo = Struct("nicinfo", [
     ("backend_id", uint32),
     ("frontend", string),
     ("frontend_id", uint32),
-    ("devid", integer),
+    ("devid", libxl_devid),
     ("state", integer),
     ("evtch", integer),
     ("rref_tx", integer),
diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
index 42f374e..97d088d 100644
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -10,6 +10,7 @@ builtins = {
     "int":                  ("int",                    "%(c)s = Int_val(%(o)s)",            "Val_int(%(c)s)"  ),
     "char *":               ("string",                 "%(c)s = dup_String_val(gc, %(o)s)", "caml_copy_string(%(c)s)"),
     "libxl_domid":          ("domid",                  "%(c)s = Int_val(%(o)s)",            "Val_int(%(c)s)"  ),
+    "libxl_devid":          ("devid",                  "%(c)s = Int_val(%(o)s)",            "Val_int(%(c)s)"  ),
     "libxl_defbool":        ("bool option",            "%(c)s = Defbool_val(%(o)s)",        "Val_defbool(%(c)s)" ),
     "libxl_uuid":           ("int array",              "Uuid_val(gc, lg, &%(c)s, %(o)s)",   "Val_uuid(&%(c)s)"),
     "libxl_key_value_list": ("(string * string) list", None,                                None),
@@ -41,8 +42,8 @@ def stub_fn_name(ty, name):
     return "stub_xl_%s_%s" % (ty.rawname,name)
     
 def ocaml_type_of(ty):
-    if ty.rawname == "domid":
-        return "domid"
+    if ty.rawname in ["domid","devid"]:
+        return ty.rawname
     elif isinstance(ty,idl.UInt):
         if ty.width in [8, 16]:
             # handle as ints
diff --git a/tools/ocaml/libs/xl/xenlight.ml.in b/tools/ocaml/libs/xl/xenlight.ml.in
index c47623c..dcc1a38 100644
--- a/tools/ocaml/libs/xl/xenlight.ml.in
+++ b/tools/ocaml/libs/xl/xenlight.ml.in
@@ -16,6 +16,7 @@
 exception Error of string
 
 type domid = int
+type devid = int
 
 (* @@LIBXL_TYPES@@ *)
 
diff --git a/tools/ocaml/libs/xl/xenlight.mli.in b/tools/ocaml/libs/xl/xenlight.mli.in
index 4717bac..3fd0165 100644
--- a/tools/ocaml/libs/xl/xenlight.mli.in
+++ b/tools/ocaml/libs/xl/xenlight.mli.in
@@ -16,6 +16,7 @@
 exception Error of string
 
 type domid = int
+type devid = int
 
 (* @@LIBXL_TYPES@@ *)
 
diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c
index 0551c76..32f982a 100644
--- a/tools/python/xen/lowlevel/xl/xl.c
+++ b/tools/python/xen/lowlevel/xl/xl.c
@@ -281,6 +281,11 @@ int attrib__libxl_domid_set(PyObject *v, libxl_domid *domid) {
     return 0;
 }
 
+int attrib__libxl_devid_set(PyObject *v, libxl_devid *devid) {
+   *devid = PyInt_AsLong(v);
+   return 0;
+}
+
 int attrib__struct_in_addr_set(PyObject *v, struct in_addr *pptr)
 {
     PyErr_SetString(PyExc_NotImplementedError, "Setting in_addr");
@@ -342,6 +347,10 @@ PyObject *attrib__libxl_domid_get(libxl_domid *domid) {
     return PyInt_FromLong(*domid);
 }
 
+PyObject *attrib__libxl_devid_get(libxl_devid *devid) {
+    return PyInt_FromLong(*devid);
+}
+
 PyObject *attrib__struct_in_addr_get(struct in_addr *pptr)
 {
     PyErr_SetString(PyExc_NotImplementedError, "Getting in_addr");
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-05 18:02 [PATCH vtpm v2 09/12] add iomem support to libxl Matthew Fioravante
  2012-10-05 18:02 ` [PATCH vtpm v2 10/12] make devid a type so it is initialized properly Matthew Fioravante
@ 2012-10-05 18:02 ` Matthew Fioravante
  2012-10-09 10:32   ` Ian Campbell
  2012-10-05 18:02 ` [PATCH vtpm v2 12/12] Matthew Fioravante now maintains VTPM Matthew Fioravante
  2 siblings, 1 reply; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-05 18:02 UTC (permalink / raw)
  To: xen-devel, ian.campbell; +Cc: Matthew Fioravante

This patch adds vtpm support to libxl. It adds vtpm parsing to config
files and 3 new xl commands:
vtpm-attach
vtpm-detach
vtpm-list

Signed-off-by: Matthew Fioravante <matthew.fioravante@jhuapl.edu>

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 428da21..9f6ee5a 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -298,6 +298,35 @@ Specifies the networking provision (both emulated network adapters,
 and Xen virtual interfaces) to provided to the guest.  See
 F<docs/misc/xl-network-configuration.markdown>.
 
+=item B<vtpm=[ "VTPM_SPEC_STRING", "VTPM_SPEC_STRING", ...]>
+
+Specifies the virtual trusted platform module to be
+provided to the guest. Please see F<docs/misc/vtpm.txt>
+for more details.
+
+Each B<VTPM_SPEC_STRING> is a comma-separated list of C<KEY=VALUE>
+settings, from the following list:
+
+=over 4
+
+=item C<backend=DOMAIN>
+
+Specify the backend domain name of id. This value is required!
+If this domain is a guest, the backend should be set to the 
+vtpm domain name. If this domain is a vtpm, the 
+backend should be set to the vtpm manager domain name. 
+
+=item C<uuid=UUID>
+
+Specify the uuid of this vtpm device. The uuid is used to uniquely
+identify the vtpm device. You can create one using the uuidgen
+program on unix systems. If left unspecified, a new uuid
+will be randomly generated every time the domain boots.
+If this is a vtpm domain, you should specify a value. The
+value is optional if this is a guest domain.
+
+=back
+
 =item B<vfb=[ "VFB_SPEC_STRING", "VFB_SPEC_STRING", ...]>
 
 Specifies the paravirtual framebuffer devices which should be supplied
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 25ce777..be9ad4c 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1101,6 +1101,31 @@ List virtual network interfaces for a domain.
 
 =back
 
+=head2 VTPM DEVICES
+
+=over 4
+
+=item B<vtpm-attach> I<domain-id> I<vtpm-device>
+
+Creates a new vtpm device in the domain specified by I<domain-id>.
+I<vtpm-device> describes the device to attach, using the same format as the
+B<vtpm> string in the domain config file. See L<xl.cfg> for
+more information.
+
+=item B<vtpm-detach> I<domain-id> I<devid|uuid>
+
+Removes the vtpm device from the domain specified by I<domain-id>.
+I<devid> is the numeric device id given to the virtual trusted
+platform module device. You will need to run B<xl vtpm-list> to determine that number.
+Alternatively the I<uuid> of the vtpm can be used to
+select the virtual device to detach.
+
+=item B<vtpm-list> I<domain-id>
+
+List virtual trusted platform modules for a domain.
+
+=back
+
 =head1 PCI PASS-THROUGH
 
 =over 4
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1606eb1..17094ca 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1726,6 +1726,246 @@ out:
 }
 
 /******************************************************************************/
+int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
+{
+   if(libxl_uuid_is_nil(&vtpm->uuid)) {
+      libxl_uuid_generate(&vtpm->uuid);
+   }
+   return 0;
+}
+
+static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
+                                   libxl_device_vtpm *vtpm,
+                                   libxl__device *device)
+{
+   device->backend_devid   = vtpm->devid;
+   device->backend_domid   = vtpm->backend_domid;
+   device->backend_kind    = LIBXL__DEVICE_KIND_VTPM;
+   device->devid           = vtpm->devid;
+   device->domid           = domid;
+   device->kind            = LIBXL__DEVICE_KIND_VTPM;
+
+   return 0;
+}
+
+void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
+                           libxl_device_vtpm *vtpm,
+                           libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    flexarray_t *front;
+    flexarray_t *back;
+    libxl__device *device;
+    char *dompath, **l;
+    unsigned int nb, rc;
+
+    rc = libxl__device_vtpm_setdefault(gc, vtpm);
+    if (rc) goto out;
+
+    front = flexarray_make(16, 1);
+    if (!front) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+    back = flexarray_make(16, 1);
+    if (!back) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    if(vtpm->devid == -1) {
+        if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
+            rc = ERROR_FAIL;
+            goto out_free;
+        }
+        l = libxl__xs_directory(gc, XBT_NULL, libxl__sprintf(gc, "%s/device/vtpm", dompath), &nb);
+        if(l == NULL || nb == 0) {
+            vtpm->devid = 0;
+        } else {
+            vtpm->devid = strtoul(l[nb - 1], NULL, 10) + 1;
+        }
+    }
+
+    GCNEW(device);
+    rc = libxl__device_from_vtpm(gc, domid, vtpm, device);
+    if ( rc != 0 ) goto out_free;
+
+    flexarray_append(back, "frontend-id");
+    flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+    flexarray_append(back, "online");
+    flexarray_append(back, "1");
+    flexarray_append(back, "state");
+    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+
+    flexarray_append(back, "uuid");
+    flexarray_append(back, libxl__sprintf(gc, LIBXL_UUID_FMT, LIBXL_UUID_BYTES(vtpm->uuid)));
+    flexarray_append(back, "instance"); /* MAYBE CAN GET RID OF THIS */
+    flexarray_append(back, "0");
+    flexarray_append(back, "pref_instance"); /* MAYBE CAN GET RID OF THIS */
+    flexarray_append(back, "0");
+    flexarray_append(back, "resume");
+    flexarray_append(back, "False");
+    flexarray_append(back, "ready"); /* MAYBE CAN GET RID OF THIS */
+    flexarray_append(back, "1");
+
+    flexarray_append(front, "backend-id");
+    flexarray_append(front, libxl__sprintf(gc, "%d", vtpm->backend_domid));
+    flexarray_append(front, "state");
+    flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+    flexarray_append(front, "handle");
+    flexarray_append(front, libxl__sprintf(gc, "%d", vtpm->devid));
+
+    libxl__device_generic_add(gc, XBT_NULL, device,
+                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+
+    aodev->dev = device;
+    aodev->action = DEVICE_CONNECT;
+    libxl__wait_device_connection(egc, aodev);
+
+    rc = 0;
+out_free:
+    flexarray_free(back);
+    flexarray_free(front);
+out:
+    aodev->rc = rc;
+    if(rc) aodev->callback(egc, aodev);
+    return;
+}
+
+static void libxl__device_vtpm_from_xs_fe(libxl__gc *gc,
+                                          const char* fe_path,
+                                          libxl_device_vtpm *vtpm)
+{
+   char* tmp;
+
+   memset(vtpm, 0, sizeof(*vtpm));
+   tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/handle", fe_path));
+   if (tmp) {
+      vtpm->devid = atoi(tmp);
+   }
+   tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend-id", fe_path));
+   if(tmp) {
+      vtpm->backend_domid = atoi(tmp);
+   }
+   tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", fe_path));
+   if(tmp) {
+      libxl_uuid_from_string(&(vtpm->uuid), tmp);
+   }
+}
+
+libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *num)
+{
+    GC_INIT(ctx);
+
+    libxl_device_vtpm* vtpms = NULL;
+    char* fe_path = NULL;
+    char** dir = NULL;
+    unsigned int ndirs = 0;
+
+    *num = 0;
+
+    fe_path = libxl__sprintf(gc, "%s/device/vtpm", libxl__xs_get_dompath(gc, domid));
+    dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
+    if(dir) {
+       vtpms = malloc(sizeof(*vtpms) * ndirs);
+       libxl_device_vtpm* vtpm;
+       libxl_device_vtpm* end = vtpms + ndirs;
+       for(vtpm = vtpms; vtpm < end; ++vtpm, ++dir) {
+          const char* path = libxl__sprintf(gc, "%s/%s", fe_path, *dir);
+          libxl__device_vtpm_from_xs_fe(gc, path, vtpm);
+       }
+    }
+    *num = ndirs;
+
+    GC_FREE;
+    return vtpms;
+}
+
+int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
+                               libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
+{
+    GC_INIT(ctx);
+    char *dompath, *vtpmpath;
+    char *val;
+    int rc = 0;
+
+    libxl_vtpminfo_init(vtpminfo);
+    dompath = libxl__xs_get_dompath(gc, domid);
+    vtpminfo->devid = vtpm->devid;
+
+    vtpmpath = libxl__sprintf(gc, "%s/device/vtpm/%d", dompath, vtpminfo->devid);
+    vtpminfo->backend = xs_read(ctx->xsh, XBT_NULL,
+                                libxl__sprintf(gc, "%s/backend", vtpmpath), NULL);
+    if (!vtpminfo->backend) {
+        goto err;
+    }
+    if(!libxl__xs_read(gc, XBT_NULL, vtpminfo->backend)) {
+       goto err;
+    }
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend-id", vtpmpath));
+    vtpminfo->backend_id = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/state", vtpmpath));
+    vtpminfo->state = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/event-channel", vtpmpath));
+    vtpminfo->evtch = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/ring-ref", vtpmpath));
+    vtpminfo->rref = val ? strtoul(val, NULL, 10) : -1;
+    vtpminfo->frontend = xs_read(ctx->xsh, XBT_NULL,
+                                 libxl__sprintf(gc, "%s/frontend", vtpminfo->backend), NULL);
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/frontend-id", vtpminfo->backend));
+    vtpminfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", vtpminfo->backend));
+    if(val == NULL) {
+       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid does not exist!\n", vtpminfo->backend);
+       goto err;
+    }
+    if(libxl_uuid_from_string(&(vtpminfo->uuid), val)) {
+       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid is a malformed uuid?? (%s) Probably a bug!\n", vtpminfo->backend, val);
+       goto err;
+    }
+
+    goto exit;
+err:
+    rc = ERROR_FAIL;
+exit:
+    GC_FREE;
+    return rc;
+}
+
+int libxl_devid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
+                              int devid, libxl_device_vtpm *vtpm)
+{
+    libxl_device_vtpm *vtpms;
+    int nb, i;
+    int rc;
+
+    vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
+    if (!vtpms)
+        return ERROR_FAIL;
+
+    memset(vtpm, 0, sizeof (libxl_device_vtpm));
+    rc = 1;
+    for (i = 0; i < nb; ++i) {
+        if(devid == vtpms[i].devid) {
+            vtpm->backend_domid = vtpms[i].backend_domid;
+            vtpm->devid = vtpms[i].devid;
+            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
+            rc = 0;
+            break;
+        }
+    }
+
+    for (i=0; i<nb; i++)
+        libxl_device_vtpm_dispose(&vtpms[i]);
+    free(vtpms);
+    return rc;
+}
+
+
+/******************************************************************************/
 
 int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
 {
@@ -3123,6 +3363,8 @@ out:
  * libxl_device_disk_destroy
  * libxl_device_nic_remove
  * libxl_device_nic_destroy
+ * libxl_device_vtpm_remove
+ * libxl_device_vtpm_destroy
  * libxl_device_vkb_remove
  * libxl_device_vkb_destroy
  * libxl_device_vfb_remove
@@ -3174,6 +3416,10 @@ DEFINE_DEVICE_REMOVE(vkb, destroy, 1)
 DEFINE_DEVICE_REMOVE(vfb, remove, 0)
 DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
 
+/* vtpm */
+DEFINE_DEVICE_REMOVE(vtpm, remove, 0)
+DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
+
 #undef DEFINE_DEVICE_REMOVE
 
 /******************************************************************************/
@@ -3182,6 +3428,7 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
 /* The following functions are defined:
  * libxl_device_disk_add
  * libxl_device_nic_add
+ * libxl_device_vtpm_add
  */
 
 #define DEFINE_DEVICE_ADD(type)                                         \
@@ -3208,6 +3455,9 @@ DEFINE_DEVICE_ADD(disk)
 /* nic */
 DEFINE_DEVICE_ADD(nic)
 
+/* vtpm */
+DEFINE_DEVICE_ADD(vtpm)
+
 #undef DEFINE_DEVICE_ADD
 
 /******************************************************************************/
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 7a7c419..3cb9ff8 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -478,13 +478,14 @@ typedef struct {
     libxl_domain_create_info c_info;
     libxl_domain_build_info b_info;
 
-    int num_disks, num_nics, num_pcidevs, num_vfbs, num_vkbs;
+    int num_disks, num_nics, num_pcidevs, num_vfbs, num_vkbs, num_vtpms;
 
     libxl_device_disk *disks;
     libxl_device_nic *nics;
     libxl_device_pci *pcidevs;
     libxl_device_vfb *vfbs;
     libxl_device_vkb *vkbs;
+    libxl_device_vtpm *vtpms;
 
     libxl_action_on_shutdown on_poweroff;
     libxl_action_on_shutdown on_reboot;
@@ -745,6 +746,23 @@ libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
+/* Virtual TPMs */
+int libxl_device_vtpm_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vtpm *vtpm,
+                          const libxl_asyncop_how *ao_how)
+                          LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vtpm_remove(libxl_ctx *ctx, uint32_t domid,
+                            libxl_device_vtpm *vtpm,
+                            const libxl_asyncop_how *ao_how)
+                            LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vtpm_destroy(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_vtpm *vtpm,
+                              const libxl_asyncop_how *ao_how)
+                              LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *num);
+int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
+                               libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo);
+
 /* Keyboard */
 int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
                          const libxl_asyncop_how *ao_how)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 96f9018..6529051 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -55,6 +55,10 @@ void libxl_domain_config_dispose(libxl_domain_config *d_config)
         libxl_device_vkb_dispose(&d_config->vkbs[i]);
     free(d_config->vkbs);
 
+    for (i=0; i<d_config->num_vtpms; i++)
+       libxl_device_vtpm_dispose(&d_config->vtpms[i]);
+    free(d_config->vtpms);
+
     libxl_domain_create_info_dispose(&d_config->c_info);
     libxl_domain_build_info_dispose(&d_config->b_info);
 }
@@ -601,6 +605,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
                                 int ret);
 
+static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev,
+                                   int ret);
 static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
                                  int ret);
 
@@ -1084,13 +1090,13 @@ static void domcreate_devmodel_started(libxl__egc *egc,
     if (d_config->num_nics > 0) {
         /* Attach nics */
         libxl__multidev_begin(ao, &dcs->multidev);
-        dcs->multidev.callback = domcreate_attach_pci;
+        dcs->multidev.callback = domcreate_attach_vtpms;
         libxl__add_nics(egc, ao, domid, d_config, &dcs->multidev);
         libxl__multidev_prepared(egc, &dcs->multidev, 0);
         return;
     }
 
-    domcreate_attach_pci(egc, &dcs->multidev, 0);
+    domcreate_attach_vtpms(egc, &dcs->multidev, 0);
     return;
 
 error_out:
@@ -1098,6 +1104,36 @@ error_out:
     domcreate_complete(egc, dcs, ret);
 }
 
+static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev, int ret) {
+   libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+   STATE_AO_GC(dcs->ao);
+   int domid = dcs->guest_domid;
+
+   libxl_domain_config* const d_config = dcs->guest_config;
+
+   if(ret) {
+      LOG(ERROR, "unable to add nic devices");
+      goto error_out;
+   }
+
+    /* Plug vtpm devices */
+    if (d_config->num_vtpms > 0) {
+        /* Attach vtpms */
+        libxl__multidev_begin(ao, &dcs->multidev);
+        dcs->multidev.callback = domcreate_attach_pci;
+        libxl__add_vtpms(egc, ao, domid, d_config, &dcs->multidev);
+        libxl__multidev_prepared(egc, &dcs->multidev, 0);
+        return;
+    }
+
+   domcreate_attach_pci(egc, multidev, 0);
+   return;
+
+error_out:
+   assert(ret);
+   domcreate_complete(egc, dcs, ret);
+}
+
 static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
                                  int ret)
 {
@@ -1111,7 +1147,7 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
     libxl_domain_config *const d_config = dcs->guest_config;
 
     if (ret) {
-        LOG(ERROR, "unable to add nic devices");
+        LOG(ERROR, "unable to add vtpm devices");
         goto error_out;
     }
 
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c3283f1..51dd06e 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -497,6 +497,7 @@ void libxl__multidev_prepared(libxl__egc *egc,
  * The following functions are defined:
  * libxl__add_disks
  * libxl__add_nics
+ * libxl__add_vtpms
  */
 
 #define DEFINE_DEVICES_ADD(type)                                        \
@@ -515,6 +516,7 @@ void libxl__multidev_prepared(libxl__egc *egc,
 
 DEFINE_DEVICES_ADD(disk)
 DEFINE_DEVICES_ADD(nic)
+DEFINE_DEVICES_ADD(vtpm)
 
 #undef DEFINE_DEVICES_ADD
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b6f54ba..e9a7cbb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -954,6 +954,7 @@ _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
                                          uint32_t domid);
+_hidden int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm);
 _hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
 _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
 _hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
@@ -1975,6 +1976,10 @@ _hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
                                    libxl_device_nic *nic,
                                    libxl__ao_device *aodev);
 
+_hidden void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
+                                   libxl_device_vtpm *vtpm,
+                                   libxl__ao_device *aodev);
+
 /* Internal function to connect a vkb device */
 _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
                                   libxl_device_vkb *vkb);
@@ -2407,6 +2412,10 @@ _hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                              libxl_domain_config *d_config,
                              libxl__multidev *multidev);
 
+_hidden void libxl__add_vtpms(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+                             libxl_domain_config *d_config,
+                             libxl__multidev *multidev);
+
 /*----- device model creation -----*/
 
 /* First layer; wraps libxl__spawn_spawn. */
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index de111a6..7eac4a8 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -395,6 +395,12 @@ libxl_device_pci = Struct("device_pci", [
     ("permissive", bool),
     ])
 
+libxl_device_vtpm = Struct("device_vtpm", [
+    ("backend_domid",    libxl_domid),
+    ("devid",            libxl_devid),
+    ("uuid",             libxl_uuid),
+])
+
 libxl_diskinfo = Struct("diskinfo", [
     ("backend", string),
     ("backend_id", uint32),
@@ -418,6 +424,18 @@ libxl_nicinfo = Struct("nicinfo", [
     ("rref_rx", integer),
     ], dir=DIR_OUT)
 
+libxl_vtpminfo = Struct("vtpminfo", [
+    ("backend", string),
+    ("backend_id", uint32),
+    ("frontend", string),
+    ("frontend_id", uint32),
+    ("devid", libxl_devid),
+    ("state", integer),
+    ("evtch", integer),
+    ("rref", integer),
+    ("uuid", libxl_uuid),
+    ], dir=DIR_OUT)
+
 libxl_vcpuinfo = Struct("vcpuinfo", [
     ("vcpuid", uint32),
     ("cpu", uint32),
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index 5ac8c9c..c40120e 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -19,6 +19,7 @@ libxl__device_kind = Enumeration("device_kind", [
     (5, "VFB"),
     (6, "VKBD"),
     (7, "CONSOLE"),
+    (8, "VTPM"),
     ])
 
 libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 55cd299..73a158a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -463,6 +463,35 @@ int libxl_pipe(libxl_ctx *ctx, int pipes[2])
     return 0;
 }
 
+int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
+                            libxl_uuid* uuid, libxl_device_vtpm *vtpm)
+{
+    libxl_device_vtpm *vtpms;
+    int nb, i;
+    int rc;
+
+    vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
+    if (!vtpms)
+        return ERROR_FAIL;
+
+    memset(vtpm, 0, sizeof (libxl_device_vtpm));
+    rc = 1;
+    for (i = 0; i < nb; ++i) {
+        if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
+            vtpm->backend_domid = vtpms[i].backend_domid;
+            vtpm->devid = vtpms[i].devid;
+            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
+            rc = 0;
+            break;
+        }
+    }
+
+    for (i=0; i<nb; i++)
+        libxl_device_vtpm_dispose(&vtpms[i]);
+    free(vtpms);
+    return rc;
+}
+
 int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
                             const char *mac, libxl_device_nic *nic)
 {
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 83aaac7..40f3f30 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -64,6 +64,11 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid, int devid,
 int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, const char *vdev,
                                libxl_device_disk *disk);
 
+int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
+                               libxl_uuid *uuid, libxl_device_vtpm *vtpm);
+int libxl_devid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
+                               int devid, libxl_device_vtpm *vtpm);
+
 int libxl_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *bitmap, int n_bits);
     /* Allocated bimap is from malloc, libxl_bitmap_dispose() to be
      * called by the application when done. */
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 0b2f848..be6f38b 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -79,6 +79,9 @@ int main_networkdetach(int argc, char **argv);
 int main_blockattach(int argc, char **argv);
 int main_blocklist(int argc, char **argv);
 int main_blockdetach(int argc, char **argv);
+int main_vtpmattach(int argc, char **argv);
+int main_vtpmlist(int argc, char **argv);
+int main_vtpmdetach(int argc, char **argv);
 int main_uptime(int argc, char **argv);
 int main_tmem_list(int argc, char **argv);
 int main_tmem_freeze(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f13d983..4c32d8f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -573,7 +573,7 @@ static void parse_config_data(const char *config_source,
     const char *buf;
     long l;
     XLU_Config *config;
-    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
+    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
     XLU_ConfigList *ioports, *irqs, *iomem;
     int num_ioports, num_irqs, num_iomem;
     int pci_power_mgmt = 0;
@@ -1048,6 +1048,55 @@ static void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_list(config, "vtpm", &vtpms, 0, 0)) {
+        d_config->num_vtpms = 0;
+        d_config->vtpms = NULL;
+        while ((buf = xlu_cfg_get_listitem (vtpms, d_config->num_vtpms)) != NULL) {
+            libxl_device_vtpm *vtpm;
+            char * buf2 = strdup(buf);
+            char *p, *p2;
+            bool got_backend = false;
+
+            d_config->vtpms = (libxl_device_vtpm *) realloc(d_config->vtpms, sizeof(libxl_device_vtpm) * (d_config->num_vtpms+1));
+            vtpm = d_config->vtpms + d_config->num_vtpms;
+            libxl_device_vtpm_init(vtpm);
+            vtpm->devid = d_config->num_vtpms;
+
+            p = strtok(buf2, ",");
+            if(p) {
+               do {
+                  while(*p == ' ')
+                     ++p;
+                  if ((p2 = strchr(p, '=')) == NULL)
+                     break;
+                  *p2 = '\0';
+                  if (!strcmp(p, "backend")) {
+                     if(domain_qualifier_to_domid(p2 + 1, &(vtpm->backend_domid), 0))
+                     {
+                        fprintf(stderr, "Specified vtpm backend domain `%s' does not exist!\n", p2 + 1);
+                        exit(1);
+                     }
+                     got_backend = true;
+                  } else if(!strcmp(p, "uuid")) {
+                     if( libxl_uuid_from_string(&vtpm->uuid, p2 + 1) ) {
+                        fprintf(stderr, "Failed to parse vtpm UUID: %s\n", p2 + 1);
+                        exit(1);
+                    }
+                  } else {
+                     fprintf(stderr, "Unknown string `%s' in vtpm spec\n", p);
+                     exit(1);
+                  }
+               } while ((p = strtok(NULL, ",")) != NULL);
+            }
+            if(!got_backend) {
+               fprintf(stderr, "vtpm spec missing required backend field!\n");
+               exit(1);
+            }
+            free(buf2);
+            d_config->num_vtpms++;
+        }
+    }
+
     if (!xlu_cfg_get_list (config, "vif", &nics, 0, 0)) {
         d_config->num_nics = 0;
         d_config->nics = NULL;
@@ -1073,7 +1122,7 @@ static void parse_config_data(const char *config_source,
 
             p = strtok(buf2, ",");
             if (!p)
-                goto skip;
+                goto skip_nic;
             do {
                 while (*p == ' ')
                     p++;
@@ -1137,7 +1186,7 @@ static void parse_config_data(const char *config_source,
                     fprintf(stderr, "the accel parameter for vifs is currently not supported\n");
                 }
             } while ((p = strtok(NULL, ",")) != NULL);
-skip:
+skip_nic:
             free(buf2);
             d_config->num_nics++;
         }
@@ -5573,6 +5622,131 @@ int main_blockdetach(int argc, char **argv)
     return rc;
 }
 
+int main_vtpmattach(int argc, char **argv)
+{
+    int opt;
+    libxl_device_vtpm vtpm;
+    char *oparg;
+    unsigned int val;
+    uint32_t domid;
+
+    if ((opt = def_getopt(argc, argv, "", "vtpm-attach", 1)) != -1)
+        return opt;
+
+    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+        return 1;
+    }
+    ++optind;
+
+    libxl_device_vtpm_init(&vtpm);
+    for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
+        if (MATCH_OPTION("uuid", *argv, oparg)) {
+            if(libxl_uuid_from_string(&(vtpm.uuid), oparg)) {
+                fprintf(stderr, "Invalid uuid specified (%s)\n", oparg);
+                return 1;
+            }
+        } else if (MATCH_OPTION("backend", *argv, oparg)) {
+            if(domain_qualifier_to_domid(oparg, &val, 0)) {
+                fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
+                val = 0;
+            }
+            vtpm.backend_domid = val;
+        } else {
+            fprintf(stderr, "unrecognized argument `%s'\n", *argv);
+            return 1;
+        }
+    }
+
+    if(dryrun_only) {
+       char* json = libxl_device_vtpm_to_json(ctx, &vtpm);
+       printf("vtpm: %s\n", json);
+       free(json);
+       libxl_device_vtpm_dispose(&vtpm);
+       if (ferror(stdout) || fflush(stdout)) { perror("stdout"); exit(-1); }
+       return 0;
+    }
+
+    if (libxl_device_vtpm_add(ctx, domid, &vtpm, 0)) {
+        fprintf(stderr, "libxl_device_vtpm_add failed.\n");
+        return 1;
+    }
+    libxl_device_vtpm_dispose(&vtpm);
+    return 0;
+}
+
+int main_vtpmlist(int argc, char **argv)
+{
+    int opt;
+    libxl_device_vtpm *vtpms;
+    libxl_vtpminfo vtpminfo;
+    int nb, i;
+
+    if ((opt = def_getopt(argc, argv, "", "vtpm-list", 1)) != -1)
+        return opt;
+
+    /*      Idx  BE   UUID   Hdl  Sta  evch rref  BE-path */
+    printf("%-3s %-2s %-36s %-6s %-5s %-6s %-5s %-10s\n",
+           "Idx", "BE", "Uuid", "handle", "state", "evt-ch", "ring-ref", "BE-path");
+    for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
+        uint32_t domid;
+        if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
+            fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
+            continue;
+        }
+        if (!(vtpms = libxl_device_vtpm_list(ctx, domid, &nb))) {
+            continue;
+        }
+        for (i = 0; i < nb; ++i) {
+           if(!libxl_device_vtpm_getinfo(ctx, domid, &vtpms[i], &vtpminfo)) {
+              /*      Idx  BE     UUID             Hdl Sta evch rref BE-path*/
+              printf("%-3d %-2d " LIBXL_UUID_FMT " %6d %5d %6d %8d %-30s\n",
+                    vtpminfo.devid, vtpminfo.backend_id,
+                    LIBXL_UUID_BYTES(vtpminfo.uuid),
+                    vtpminfo.devid, vtpminfo.state, vtpminfo.evtch,
+                    vtpminfo.rref, vtpminfo.backend);
+
+              libxl_vtpminfo_dispose(&vtpminfo);
+           }
+           libxl_device_vtpm_dispose(&vtpms[i]);
+        }
+        free(vtpms);
+    }
+    return 0;
+}
+
+int main_vtpmdetach(int argc, char **argv)
+{
+    uint32_t domid;
+    int opt, rc=0;
+    libxl_device_vtpm vtpm;
+    libxl_uuid uuid;
+
+    if ((opt = def_getopt(argc, argv, "", "vtpm-detach", 2)) != -1)
+        return opt;
+
+    domid = find_domain(argv[optind]);
+
+    if ( libxl_uuid_from_string(&uuid, argv[optind+1])) {
+        if (libxl_devid_to_device_vtpm(ctx, domid, atoi(argv[optind+1]), &vtpm)) {
+            fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
+            return 1;
+        }
+    } else {
+        if (libxl_uuid_to_device_vtpm(ctx, domid, &uuid, &vtpm)) {
+            fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
+            return 1;
+        }
+    }
+    rc = libxl_device_vtpm_remove(ctx, domid, &vtpm, 0);
+    if (rc) {
+        fprintf(stderr, "libxl_device_vtpm_remove failed.\n");
+    }
+    libxl_device_vtpm_dispose(&vtpm);
+    return rc;
+}
+
+
 static char *uptime_to_string(unsigned long uptime, int short_mode)
 {
     int sec, min, hour, day;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 85ea768..7c018eb 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -338,6 +338,21 @@ struct cmd_spec cmd_table[] = {
       "Destroy a domain's virtual block device",
       "<Domain> <DevId>",
     },
+    { "vtpm-attach",
+      &main_vtpmattach, 0, 1,
+      "Create a new virtual TPM device",
+      "<Domain> [uuid=<uuid>] [backend=<BackDomain>]",
+    },
+    { "vtpm-list",
+      &main_vtpmlist, 0, 0,
+      "List virtual TPM devices for a domain",
+      "<Domain(s)>",
+    },
+    { "vtpm-detach",
+      &main_vtpmdetach, 0, 1,
+      "Destroy a domain's virtual TPM device",
+      "<Domain> <DevId|uuid>",
+    },
     { "uptime",
       &main_uptime, 0, 0,
       "Print uptime for all/some domains",
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH vtpm v2 12/12] Matthew Fioravante now maintains VTPM
  2012-10-05 18:02 [PATCH vtpm v2 09/12] add iomem support to libxl Matthew Fioravante
  2012-10-05 18:02 ` [PATCH vtpm v2 10/12] make devid a type so it is initialized properly Matthew Fioravante
  2012-10-05 18:02 ` [PATCH vtpm v2 11/12] add vtpm support to libxl Matthew Fioravante
@ 2012-10-05 18:02 ` Matthew Fioravante
  2 siblings, 0 replies; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-05 18:02 UTC (permalink / raw)
  To: xen-devel, ian.campbell; +Cc: Matthew Fioravante

See MAINTAINERS file

Signed-off-by: Matthew Fioravante <matthew.fioravante@jhuapl.edu>

diff --git a/MAINTAINERS b/MAINTAINERS
index 094fe9e..0bde721 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -242,6 +242,21 @@ S:	Supported
 T:	hg http://xenbits.xen.org/linux-2.6.18-xen.hg
 F:	drivers/xen/usb*/
 
+VTPM
+M:	Matthew Fioravante <matthew.fioravante@jhuapl.edu>
+S:	Supported
+F:	tools/vtpm/
+F:	tools/vtpm_manager/
+F:	extras/minios-os/tpmfront.c
+F:	extras/minios-os/tpmback.c
+F:	extras/minios-os/tpm-tis.c
+F:	extras/minios-os/include/tpmfront.h
+F:	extras/minios-os/include/tpmback.h
+F:	extras/minios-os/include/tpm-tis.h
+F:	stubdom/vtpm/
+F:	stubdom/vtpmmgr/
+F:	docs/misc/vtpm.txt
+
 X86 ARCHITECTURE
 M:	Keir Fraser <keir@xen.org>
 M:	Jan Beulich <jbeulich@suse.com>
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-05 18:02 ` [PATCH vtpm v2 11/12] add vtpm support to libxl Matthew Fioravante
@ 2012-10-09 10:32   ` Ian Campbell
  2012-10-09 13:36     ` Matthew Fioravante
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-10-09 10:32 UTC (permalink / raw)
  To: Matthew Fioravante; +Cc: xen-devel@lists.xen.org

IIRC you are reworking the vtpm stuff to only support the stub domaun
model and not the process model -- does this mean this patch will change
or is this already only doing stub stuff?

> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 1606eb1..17094ca 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1726,6 +1726,246 @@ out:
>  }
> 
>  /******************************************************************************/
> +int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
> +{
> +   if(libxl_uuid_is_nil(&vtpm->uuid)) {
> +      libxl_uuid_generate(&vtpm->uuid);
> +   }
> +   return 0;
> +}
> +
> +static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
> +                                   libxl_device_vtpm *vtpm,
> +                                   libxl__device *device)
> +{
> +   device->backend_devid   = vtpm->devid;
> +   device->backend_domid   = vtpm->backend_domid;
> +   device->backend_kind    = LIBXL__DEVICE_KIND_VTPM;
> +   device->devid           = vtpm->devid;
> +   device->domid           = domid;
> +   device->kind            = LIBXL__DEVICE_KIND_VTPM;
> +
> +   return 0;
> +}
> +
> +void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
> +                           libxl_device_vtpm *vtpm,
> +                           libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    flexarray_t *front;
> +    flexarray_t *back;
> +    libxl__device *device;
> +    char *dompath, **l;
> +    unsigned int nb, rc;
> +
> +    rc = libxl__device_vtpm_setdefault(gc, vtpm);
> +    if (rc) goto out;
> +
> +    front = flexarray_make(16, 1);

Sorry but in the meantime the flexarray interface has changed, it now
accepts a gc -- see commit 25991:5c6b72b62bd7.


> +    if (!front) {
> +        rc = ERROR_NOMEM;
> +        goto out;
> +    }
> +    back = flexarray_make(16, 1);
> +    if (!back) {
> +        rc = ERROR_NOMEM;
> +        goto out;
> +    }
> +
> +    if(vtpm->devid == -1) {
> +        if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
> +            rc = ERROR_FAIL;
> +            goto out_free;
> +        }
> +        l = libxl__xs_directory(gc, XBT_NULL, libxl__sprintf(gc, "%s/device/vtpm", dompath), &nb);

You have some very long lines in this patch. Can you try and keep it to
75-80 characters please.

There are various helper macros like GCSPRINTF which can help to reduce
the length of lines. Also you might find the LOG* macros useful instead
of the more verbose LIBXL__LOG*.

[...]

> +    flexarray_append(back, "instance"); /* MAYBE CAN GET RID OF THIS */
> +    flexarray_append(back, "0");
> +    flexarray_append(back, "pref_instance"); /* MAYBE CAN GET RID OF THIS */
> +    flexarray_append(back, "0");
> +    flexarray_append(back, "resume");
> +    flexarray_append(back, "False");
> +    flexarray_append(back, "ready"); /* MAYBE CAN GET RID OF THIS */
> +    flexarray_append(back, "1");

Can we decide now or is this a future work thing?

Not a lot of existing code uses it but we have flexarray_append_pair
which can clarify the pairing of keys and values if you would like to
use it.

> +static void libxl__device_vtpm_from_xs_fe(libxl__gc *gc,
> +                                          const char* fe_path,
> +                                          libxl_device_vtpm *vtpm)
> 

Other devices have from_xs_be not fe. This is because we "trust" the be
to be less malicious.

> +{
> [...]
> +   tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", fe_path));
> +   if(tmp) {
> +      libxl_uuid_from_string(&(vtpm->uuid), tmp);
> +   }
> +}
> [...]
> +int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
> +                               libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
> +{
> [...]
> +    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", vtpminfo->backend));
> +    if(val == NULL) {
> +       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid does not exist!\n", vtpminfo->backend);
> +       goto err;
> +    }
> +    if(libxl_uuid_from_string(&(vtpminfo->uuid), val)) {
> +       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid is a malformed uuid?? (%s) Probably a bug!\n", vtpminfo->backend, val);
> +       goto err;

This is fatal here but not in from_xs_fe?

> +static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev, int ret) {

brace on next line please.

> +   libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
> +   STATE_AO_GC(dcs->ao);
> +   int domid = dcs->guest_domid;
> +
> +   libxl_domain_config* const d_config = dcs->guest_config;
> +
> +   if(ret) {
> +      LOG(ERROR, "unable to add nic devices");
> +      goto error_out;
> +   }

Four space indents above please.

> +    /* Plug vtpm devices */
> +    if (d_config->num_vtpms > 0) {
> +        /* Attach vtpms */
> +        libxl__multidev_begin(ao, &dcs->multidev);
> +        dcs->multidev.callback = domcreate_attach_pci;
> +        libxl__add_vtpms(egc, ao, domid, d_config, &dcs->multidev);
> +        libxl__multidev_prepared(egc, &dcs->multidev, 0);
> +        return;
> +    }

This indent is ok.

> +
> +   domcreate_attach_pci(egc, multidev, 0);
> +   return;
> +
> +error_out:
> +   assert(ret);
> +   domcreate_complete(egc, dcs, ret);

But here we've gone back to 3 spaces again.

> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 55cd299..73a158a 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -463,6 +463,35 @@ int libxl_pipe(libxl_ctx *ctx, int pipes[2])
>      return 0;
>  }
> 
> +int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
> +                            libxl_uuid* uuid, libxl_device_vtpm *vtpm)
> +{
> +    libxl_device_vtpm *vtpms;
> +    int nb, i;
> +    int rc;
> +
> +    vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
> +    if (!vtpms)
> +        return ERROR_FAIL;
> +
> +    memset(vtpm, 0, sizeof (libxl_device_vtpm));
> +    rc = 1;
> +    for (i = 0; i < nb; ++i) {
> +        if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
> +            vtpm->backend_domid = vtpms[i].backend_domid;
> +            vtpm->devid = vtpms[i].devid;
> +            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
> +            rc = 0;
> +            break;
> +        }
> +    }
> +
> +    for (i=0; i<nb; i++)
> +        libxl_device_vtpm_dispose(&vtpms[i]);
> +    free(vtpms);

I think I saw this a few times (probably copied from elsewhere) but the
modern alternative is to define libxl_THING_list_free and use that to
free the result of libxl_THING_list.

We just didn't go back and change all the existing instances when we did
this.


> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 85ea768..7c018eb 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -338,6 +338,21 @@ struct cmd_spec cmd_table[] = {
>        "Destroy a domain's virtual block device",
>        "<Domain> <DevId>",
>      },
> +    { "vtpm-attach",
> +      &main_vtpmattach, 0, 1,
> +      "Create a new virtual TPM device",
> +      "<Domain> [uuid=<uuid>] [backend=<BackDomain>]",
> +    },
> +    { "vtpm-list",
> +      &main_vtpmlist, 0, 0,

I think you want the first 0 to be 1 since you do support dry run in
this command

> +      "List virtual TPM devices for a domain",
> +      "<Domain(s)>",
> +    },
> +    { "vtpm-detach",
> +      &main_vtpmdetach, 0, 1,
> +      "Destroy a domain's virtual TPM device",
> +      "<Domain> <DevId|uuid>",
> +    },
>      { "uptime",
>        &main_uptime, 0, 0,
>        "Print uptime for all/some domains",
> --
> 1.7.4.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-09 10:32   ` Ian Campbell
@ 2012-10-09 13:36     ` Matthew Fioravante
  2012-10-09 13:42       ` Matthew Fioravante
  2012-10-09 13:42       ` Ian Campbell
  0 siblings, 2 replies; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-09 13:36 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xen.org


[-- Attachment #1.1: Type: text/plain, Size: 8600 bytes --]

On 10/09/2012 06:32 AM, Ian Campbell wrote:
> IIRC you are reworking the vtpm stuff to only support the stub domaun
> model and not the process model -- does this mean this patch will change
> or is this already only doing stub stuff?
Since I'm not removing the process model, its possible that this. the
tpm mini-os drivers, and the linux vtpm drivers might change slightly.
Would you prefer I held off on these last 2 patches until the changes
are finalized?
>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 1606eb1..17094ca 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1726,6 +1726,246 @@ out:
>>  }
>>
>>  /******************************************************************************/
>> +int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
>> +{
>> +   if(libxl_uuid_is_nil(&vtpm->uuid)) {
>> +      libxl_uuid_generate(&vtpm->uuid);
>> +   }
>> +   return 0;
>> +}
>> +
>> +static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
>> +                                   libxl_device_vtpm *vtpm,
>> +                                   libxl__device *device)
>> +{
>> +   device->backend_devid   = vtpm->devid;
>> +   device->backend_domid   = vtpm->backend_domid;
>> +   device->backend_kind    = LIBXL__DEVICE_KIND_VTPM;
>> +   device->devid           = vtpm->devid;
>> +   device->domid           = domid;
>> +   device->kind            = LIBXL__DEVICE_KIND_VTPM;
>> +
>> +   return 0;
>> +}
>> +
>> +void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
>> +                           libxl_device_vtpm *vtpm,
>> +                           libxl__ao_device *aodev)
>> +{
>> +    STATE_AO_GC(aodev->ao);
>> +    flexarray_t *front;
>> +    flexarray_t *back;
>> +    libxl__device *device;
>> +    char *dompath, **l;
>> +    unsigned int nb, rc;
>> +
>> +    rc = libxl__device_vtpm_setdefault(gc, vtpm);
>> +    if (rc) goto out;
>> +
>> +    front = flexarray_make(16, 1);
> Sorry but in the meantime the flexarray interface has changed, it now
> accepts a gc -- see commit 25991:5c6b72b62bd7.
Will fix
>
>
>> +    if (!front) {
>> +        rc = ERROR_NOMEM;
>> +        goto out;
>> +    }
>> +    back = flexarray_make(16, 1);
>> +    if (!back) {
>> +        rc = ERROR_NOMEM;
>> +        goto out;
>> +    }
>> +
>> +    if(vtpm->devid == -1) {
>> +        if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
>> +            rc = ERROR_FAIL;
>> +            goto out_free;
>> +        }
>> +        l = libxl__xs_directory(gc, XBT_NULL, libxl__sprintf(gc, "%s/device/vtpm", dompath), &nb);
> You have some very long lines in this patch. Can you try and keep it to
> 75-80 characters please.
>
> There are various helper macros like GCSPRINTF which can help to reduce
> the length of lines. Also you might find the LOG* macros useful instead
> of the more verbose LIBXL__LOG*.
noted
>
> [...]
>
>> +    flexarray_append(back, "instance"); /* MAYBE CAN GET RID OF THIS */
>> +    flexarray_append(back, "0");
>> +    flexarray_append(back, "pref_instance"); /* MAYBE CAN GET RID OF THIS */
>> +    flexarray_append(back, "0");
>> +    flexarray_append(back, "resume");
>> +    flexarray_append(back, "False");
>> +    flexarray_append(back, "ready"); /* MAYBE CAN GET RID OF THIS */
>> +    flexarray_append(back, "1");
> Can we decide now or is this a future work thing?
These will probably be removed from the driver now that the process
model is gone.
>
> Not a lot of existing code uses it but we have flexarray_append_pair
> which can clarify the pairing of keys and values if you would like to
> use it.
Probably makes code a little more readable, ill look into it.
>
>> +static void libxl__device_vtpm_from_xs_fe(libxl__gc *gc,
>> +                                          const char* fe_path,
>> +                                          libxl_device_vtpm *vtpm)
>>
> Other devices have from_xs_be not fe. This is because we "trust" the be
> to be less malicious.
will rework
>
>> +{
>> [...]
>> +   tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", fe_path));
>> +   if(tmp) {
>> +      libxl_uuid_from_string(&(vtpm->uuid), tmp);
>> +   }
>> +}
>> [...]
>> +int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
>> +                               libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
>> +{
>> [...]
>> +    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", vtpminfo->backend));
>> +    if(val == NULL) {
>> +       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid does not exist!\n", vtpminfo->backend);
>> +       goto err;
>> +    }
>> +    if(libxl_uuid_from_string(&(vtpminfo->uuid), val)) {
>> +       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid is a malformed uuid?? (%s) Probably a bug!\n", vtpminfo->backend, val);
>> +       goto err;
> This is fatal here but not in from_xs_fe?
>
>> +static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev, int ret) {
> brace on next line please.
>
>> +   libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
>> +   STATE_AO_GC(dcs->ao);
>> +   int domid = dcs->guest_domid;
>> +
>> +   libxl_domain_config* const d_config = dcs->guest_config;
>> +
>> +   if(ret) {
>> +      LOG(ERROR, "unable to add nic devices");
>> +      goto error_out;
>> +   }
> Four space indents above please.
>
>> +    /* Plug vtpm devices */
>> +    if (d_config->num_vtpms > 0) {
>> +        /* Attach vtpms */
>> +        libxl__multidev_begin(ao, &dcs->multidev);
>> +        dcs->multidev.callback = domcreate_attach_pci;
>> +        libxl__add_vtpms(egc, ao, domid, d_config, &dcs->multidev);
>> +        libxl__multidev_prepared(egc, &dcs->multidev, 0);
>> +        return;
>> +    }
> This indent is ok.
>
>> +
>> +   domcreate_attach_pci(egc, multidev, 0);
>> +   return;
>> +
>> +error_out:
>> +   assert(ret);
>> +   domcreate_complete(egc, dcs, ret);
> But here we've gone back to 3 spaces again.
>
>> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
>> index 55cd299..73a158a 100644
>> --- a/tools/libxl/libxl_utils.c
>> +++ b/tools/libxl/libxl_utils.c
>> @@ -463,6 +463,35 @@ int libxl_pipe(libxl_ctx *ctx, int pipes[2])
>>      return 0;
>>  }
>>
>> +int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
>> +                            libxl_uuid* uuid, libxl_device_vtpm *vtpm)
>> +{
>> +    libxl_device_vtpm *vtpms;
>> +    int nb, i;
>> +    int rc;
>> +
>> +    vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
>> +    if (!vtpms)
>> +        return ERROR_FAIL;
>> +
>> +    memset(vtpm, 0, sizeof (libxl_device_vtpm));
>> +    rc = 1;
>> +    for (i = 0; i < nb; ++i) {
>> +        if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
>> +            vtpm->backend_domid = vtpms[i].backend_domid;
>> +            vtpm->devid = vtpms[i].devid;
>> +            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
>> +            rc = 0;
>> +            break;
>> +        }
>> +    }
>> +
>> +    for (i=0; i<nb; i++)
>> +        libxl_device_vtpm_dispose(&vtpms[i]);
>> +    free(vtpms);
> I think I saw this a few times (probably copied from elsewhere) but the
> modern alternative is to define libxl_THING_list_free and use that to
> free the result of libxl_THING_list.
>
> We just didn't go back and change all the existing instances when we did
> this.
>
>
>> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
>> index 85ea768..7c018eb 100644
>> --- a/tools/libxl/xl_cmdtable.c
>> +++ b/tools/libxl/xl_cmdtable.c
>> @@ -338,6 +338,21 @@ struct cmd_spec cmd_table[] = {
>>        "Destroy a domain's virtual block device",
>>        "<Domain> <DevId>",
>>      },
>> +    { "vtpm-attach",
>> +      &main_vtpmattach, 0, 1,
>> +      "Create a new virtual TPM device",
>> +      "<Domain> [uuid=<uuid>] [backend=<BackDomain>]",
>> +    },
>> +    { "vtpm-list",
>> +      &main_vtpmlist, 0, 0,
> I think you want the first 0 to be 1 since you do support dry run in
> this command
agreed, must have been a typo
>
>> +      "List virtual TPM devices for a domain",
>> +      "<Domain(s)>",
>> +    },
>> +    { "vtpm-detach",
>> +      &main_vtpmdetach, 0, 1,
>> +      "Destroy a domain's virtual TPM device",
>> +      "<Domain> <DevId|uuid>",
>> +    },
>>      { "uptime",
>>        &main_uptime, 0, 0,
>>        "Print uptime for all/some domains",
>> --
>> 1.7.4.4
>>
>



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-09 13:36     ` Matthew Fioravante
@ 2012-10-09 13:42       ` Matthew Fioravante
  2012-10-09 13:42       ` Ian Campbell
  1 sibling, 0 replies; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-09 13:42 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xen.org


[-- Attachment #1.1: Type: text/plain, Size: 8929 bytes --]

On 10/09/2012 09:36 AM, Matthew Fioravante wrote:
> On 10/09/2012 06:32 AM, Ian Campbell wrote:
>> IIRC you are reworking the vtpm stuff to only support the stub domaun
>> model and not the process model -- does this mean this patch will change
>> or is this already only doing stub stuff?
> Since I'm not removing the process model, its possible that this. the
> tpm mini-os drivers, and the linux vtpm drivers might change slightly.
> Would you prefer I held off on these last 2 patches until the changes
> are finalized?
Sorry typo *since I am removing the process model*
>>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>>> index 1606eb1..17094ca 100644
>>> --- a/tools/libxl/libxl.c
>>> +++ b/tools/libxl/libxl.c
>>> @@ -1726,6 +1726,246 @@ out:
>>>  }
>>>
>>>  /******************************************************************************/
>>> +int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
>>> +{
>>> +   if(libxl_uuid_is_nil(&vtpm->uuid)) {
>>> +      libxl_uuid_generate(&vtpm->uuid);
>>> +   }
>>> +   return 0;
>>> +}
>>> +
>>> +static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
>>> +                                   libxl_device_vtpm *vtpm,
>>> +                                   libxl__device *device)
>>> +{
>>> +   device->backend_devid   = vtpm->devid;
>>> +   device->backend_domid   = vtpm->backend_domid;
>>> +   device->backend_kind    = LIBXL__DEVICE_KIND_VTPM;
>>> +   device->devid           = vtpm->devid;
>>> +   device->domid           = domid;
>>> +   device->kind            = LIBXL__DEVICE_KIND_VTPM;
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
>>> +                           libxl_device_vtpm *vtpm,
>>> +                           libxl__ao_device *aodev)
>>> +{
>>> +    STATE_AO_GC(aodev->ao);
>>> +    flexarray_t *front;
>>> +    flexarray_t *back;
>>> +    libxl__device *device;
>>> +    char *dompath, **l;
>>> +    unsigned int nb, rc;
>>> +
>>> +    rc = libxl__device_vtpm_setdefault(gc, vtpm);
>>> +    if (rc) goto out;
>>> +
>>> +    front = flexarray_make(16, 1);
>> Sorry but in the meantime the flexarray interface has changed, it now
>> accepts a gc -- see commit 25991:5c6b72b62bd7.
> Will fix
>>
>>> +    if (!front) {
>>> +        rc = ERROR_NOMEM;
>>> +        goto out;
>>> +    }
>>> +    back = flexarray_make(16, 1);
>>> +    if (!back) {
>>> +        rc = ERROR_NOMEM;
>>> +        goto out;
>>> +    }
>>> +
>>> +    if(vtpm->devid == -1) {
>>> +        if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
>>> +            rc = ERROR_FAIL;
>>> +            goto out_free;
>>> +        }
>>> +        l = libxl__xs_directory(gc, XBT_NULL, libxl__sprintf(gc, "%s/device/vtpm", dompath), &nb);
>> You have some very long lines in this patch. Can you try and keep it to
>> 75-80 characters please.
>>
>> There are various helper macros like GCSPRINTF which can help to reduce
>> the length of lines. Also you might find the LOG* macros useful instead
>> of the more verbose LIBXL__LOG*.
> noted
>> [...]
>>
>>> +    flexarray_append(back, "instance"); /* MAYBE CAN GET RID OF THIS */
>>> +    flexarray_append(back, "0");
>>> +    flexarray_append(back, "pref_instance"); /* MAYBE CAN GET RID OF THIS */
>>> +    flexarray_append(back, "0");
>>> +    flexarray_append(back, "resume");
>>> +    flexarray_append(back, "False");
>>> +    flexarray_append(back, "ready"); /* MAYBE CAN GET RID OF THIS */
>>> +    flexarray_append(back, "1");
>> Can we decide now or is this a future work thing?
> These will probably be removed from the driver now that the process
> model is gone.
>> Not a lot of existing code uses it but we have flexarray_append_pair
>> which can clarify the pairing of keys and values if you would like to
>> use it.
> Probably makes code a little more readable, ill look into it.
>>> +static void libxl__device_vtpm_from_xs_fe(libxl__gc *gc,
>>> +                                          const char* fe_path,
>>> +                                          libxl_device_vtpm *vtpm)
>>>
>> Other devices have from_xs_be not fe. This is because we "trust" the be
>> to be less malicious.
> will rework
>>> +{
>>> [...]
>>> +   tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", fe_path));
>>> +   if(tmp) {
>>> +      libxl_uuid_from_string(&(vtpm->uuid), tmp);
>>> +   }
>>> +}
>>> [...]
>>> +int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
>>> +                               libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
>>> +{
>>> [...]
>>> +    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/uuid", vtpminfo->backend));
>>> +    if(val == NULL) {
>>> +       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid does not exist!\n", vtpminfo->backend);
>>> +       goto err;
>>> +    }
>>> +    if(libxl_uuid_from_string(&(vtpminfo->uuid), val)) {
>>> +       LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s/uuid is a malformed uuid?? (%s) Probably a bug!\n", vtpminfo->backend, val);
>>> +       goto err;
>> This is fatal here but not in from_xs_fe?
>>
>>> +static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev, int ret) {
>> brace on next line please.
>>
>>> +   libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
>>> +   STATE_AO_GC(dcs->ao);
>>> +   int domid = dcs->guest_domid;
>>> +
>>> +   libxl_domain_config* const d_config = dcs->guest_config;
>>> +
>>> +   if(ret) {
>>> +      LOG(ERROR, "unable to add nic devices");
>>> +      goto error_out;
>>> +   }
>> Four space indents above please.
>>
>>> +    /* Plug vtpm devices */
>>> +    if (d_config->num_vtpms > 0) {
>>> +        /* Attach vtpms */
>>> +        libxl__multidev_begin(ao, &dcs->multidev);
>>> +        dcs->multidev.callback = domcreate_attach_pci;
>>> +        libxl__add_vtpms(egc, ao, domid, d_config, &dcs->multidev);
>>> +        libxl__multidev_prepared(egc, &dcs->multidev, 0);
>>> +        return;
>>> +    }
>> This indent is ok.
>>
>>> +
>>> +   domcreate_attach_pci(egc, multidev, 0);
>>> +   return;
>>> +
>>> +error_out:
>>> +   assert(ret);
>>> +   domcreate_complete(egc, dcs, ret);
>> But here we've gone back to 3 spaces again.
>>
>>> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
>>> index 55cd299..73a158a 100644
>>> --- a/tools/libxl/libxl_utils.c
>>> +++ b/tools/libxl/libxl_utils.c
>>> @@ -463,6 +463,35 @@ int libxl_pipe(libxl_ctx *ctx, int pipes[2])
>>>      return 0;
>>>  }
>>>
>>> +int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
>>> +                            libxl_uuid* uuid, libxl_device_vtpm *vtpm)
>>> +{
>>> +    libxl_device_vtpm *vtpms;
>>> +    int nb, i;
>>> +    int rc;
>>> +
>>> +    vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
>>> +    if (!vtpms)
>>> +        return ERROR_FAIL;
>>> +
>>> +    memset(vtpm, 0, sizeof (libxl_device_vtpm));
>>> +    rc = 1;
>>> +    for (i = 0; i < nb; ++i) {
>>> +        if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
>>> +            vtpm->backend_domid = vtpms[i].backend_domid;
>>> +            vtpm->devid = vtpms[i].devid;
>>> +            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
>>> +            rc = 0;
>>> +            break;
>>> +        }
>>> +    }
>>> +
>>> +    for (i=0; i<nb; i++)
>>> +        libxl_device_vtpm_dispose(&vtpms[i]);
>>> +    free(vtpms);
>> I think I saw this a few times (probably copied from elsewhere) but the
>> modern alternative is to define libxl_THING_list_free and use that to
>> free the result of libxl_THING_list.
>>
>> We just didn't go back and change all the existing instances when we did
>> this.
>>
>>
>>> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
>>> index 85ea768..7c018eb 100644
>>> --- a/tools/libxl/xl_cmdtable.c
>>> +++ b/tools/libxl/xl_cmdtable.c
>>> @@ -338,6 +338,21 @@ struct cmd_spec cmd_table[] = {
>>>        "Destroy a domain's virtual block device",
>>>        "<Domain> <DevId>",
>>>      },
>>> +    { "vtpm-attach",
>>> +      &main_vtpmattach, 0, 1,
>>> +      "Create a new virtual TPM device",
>>> +      "<Domain> [uuid=<uuid>] [backend=<BackDomain>]",
>>> +    },
>>> +    { "vtpm-list",
>>> +      &main_vtpmlist, 0, 0,
>> I think you want the first 0 to be 1 since you do support dry run in
>> this command
> agreed, must have been a typo
>>> +      "List virtual TPM devices for a domain",
>>> +      "<Domain(s)>",
>>> +    },
>>> +    { "vtpm-detach",
>>> +      &main_vtpmdetach, 0, 1,
>>> +      "Destroy a domain's virtual TPM device",
>>> +      "<Domain> <DevId|uuid>",
>>> +    },
>>>      { "uptime",
>>>        &main_uptime, 0, 0,
>>>        "Print uptime for all/some domains",
>>> --
>>> 1.7.4.4
>>>
>



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-09 13:36     ` Matthew Fioravante
  2012-10-09 13:42       ` Matthew Fioravante
@ 2012-10-09 13:42       ` Ian Campbell
  2012-10-09 13:47         ` Matthew Fioravante
  1 sibling, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-10-09 13:42 UTC (permalink / raw)
  To: Matthew Fioravante; +Cc: xen-devel@lists.xen.org

On Tue, 2012-10-09 at 14:36 +0100, Matthew Fioravante wrote:
> On 10/09/2012 06:32 AM, Ian Campbell wrote:
> > IIRC you are reworking the vtpm stuff to only support the stub domaun
> > model and not the process model -- does this mean this patch will change
> > or is this already only doing stub stuff?
> Since I'm not removing the process model, its possible that this. the
> tpm mini-os drivers, and the linux vtpm drivers might change slightly.
> Would you prefer I held off on these last 2 patches until the changes
> are finalized?

Might be easiest if you just resent the whole lot when you are ready?

I think I've applied everything which might be plausibly independent to
this -- if there's remaining stuff you think could go in now and not be
affected by this please point me at it.

Ian.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-09 13:42       ` Ian Campbell
@ 2012-10-09 13:47         ` Matthew Fioravante
  2012-10-09 13:50           ` Ian Campbell
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Fioravante @ 2012-10-09 13:47 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xen.org


[-- Attachment #1.1: Type: text/plain, Size: 1453 bytes --]

On 10/09/2012 09:42 AM, Ian Campbell wrote:
> On Tue, 2012-10-09 at 14:36 +0100, Matthew Fioravante wrote:
>> On 10/09/2012 06:32 AM, Ian Campbell wrote:
>>> IIRC you are reworking the vtpm stuff to only support the stub domaun
>>> model and not the process model -- does this mean this patch will change
>>> or is this already only doing stub stuff?
>> Since I'm not removing the process model, its possible that this. the
>> tpm mini-os drivers, and the linux vtpm drivers might change slightly.
>> Would you prefer I held off on these last 2 patches until the changes
>> are finalized?
> Might be easiest if you just resent the whole lot when you are ready?
>
> I think I've applied everything which might be plausibly independent to
> this -- if there's remaining stuff you think could go in now and not be
> affected by this please point me at it.
>
> Ian.
>
>
I think we can hold off on patches 8 (tpm drivers) and 11 (libxl vtpm)
for now. I'll resend them with the next batch. All of the independent
mini-so enhancements and the xl devid/iomem patches can be commited.
Those will not change and might be useful for someone doing something else.

The next and final set of patches will include patches 8 and 11 from
here, removal of the vtpm process model, vtpm-stubdom, vtpmmgrdom, and a
few new libraries for stubdom that they require. I'll need a little time
to rework and test them, but they will be coming soon.


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vtpm v2 11/12] add vtpm support to libxl
  2012-10-09 13:47         ` Matthew Fioravante
@ 2012-10-09 13:50           ` Ian Campbell
  0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2012-10-09 13:50 UTC (permalink / raw)
  To: Matthew Fioravante; +Cc: xen-devel@lists.xen.org

On Tue, 2012-10-09 at 14:47 +0100, Matthew Fioravante wrote:
> On 10/09/2012 09:42 AM, Ian Campbell wrote:
> > On Tue, 2012-10-09 at 14:36 +0100, Matthew Fioravante wrote:
> >> On 10/09/2012 06:32 AM, Ian Campbell wrote:
> >>> IIRC you are reworking the vtpm stuff to only support the stub domaun
> >>> model and not the process model -- does this mean this patch will change
> >>> or is this already only doing stub stuff?
> >> Since I'm not removing the process model, its possible that this. the
> >> tpm mini-os drivers, and the linux vtpm drivers might change slightly.
> >> Would you prefer I held off on these last 2 patches until the changes
> >> are finalized?
> > Might be easiest if you just resent the whole lot when you are ready?
> >
> > I think I've applied everything which might be plausibly independent to
> > this -- if there's remaining stuff you think could go in now and not be
> > affected by this please point me at it.
> >
> > Ian.
> >
> >
> I think we can hold off on patches 8 (tpm drivers) and 11 (libxl vtpm)
> for now. I'll resend them with the next batch. All of the independent
> mini-so enhancements and the xl devid/iomem patches can be commited.
> Those will not change and might be useful for someone doing something else.

I think these are all in now, please let me know if I've missed one.

> The next and final set of patches will include patches 8 and 11 from
> here, removal of the vtpm process model, vtpm-stubdom, vtpmmgrdom, and a
> few new libraries for stubdom that they require. I'll need a little time
> to rework and test them, but they will be coming soon.

Great, thanks.

Ian.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-10-09 13:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-05 18:02 [PATCH vtpm v2 09/12] add iomem support to libxl Matthew Fioravante
2012-10-05 18:02 ` [PATCH vtpm v2 10/12] make devid a type so it is initialized properly Matthew Fioravante
2012-10-05 18:02 ` [PATCH vtpm v2 11/12] add vtpm support to libxl Matthew Fioravante
2012-10-09 10:32   ` Ian Campbell
2012-10-09 13:36     ` Matthew Fioravante
2012-10-09 13:42       ` Matthew Fioravante
2012-10-09 13:42       ` Ian Campbell
2012-10-09 13:47         ` Matthew Fioravante
2012-10-09 13:50           ` Ian Campbell
2012-10-05 18:02 ` [PATCH vtpm v2 12/12] Matthew Fioravante now maintains VTPM Matthew Fioravante

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.