Mailing List Archive

[PATCH vtpm v2 11/12] add vtpm support to libxl
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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH vtpm v2 11/12] add vtpm support to libxl [ In reply to ]
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
>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH vtpm v2 11/12] add vtpm support to libxl [ In reply to ]
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
>>
>
Re: [PATCH vtpm v2 11/12] add vtpm support to libxl [ In reply to ]
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
>>>
>
Re: [PATCH vtpm v2 11/12] add vtpm support to libxl [ In reply to ]
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.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH vtpm v2 11/12] add vtpm support to libxl [ In reply to ]
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.
Re: [PATCH vtpm v2 11/12] add vtpm support to libxl [ In reply to ]
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.


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