[libvirt] [PATCH v3 00/11] Add support for guests with TPM passthrough device

Hello! The following set of patches adds support to libvirt for adding a TPM passthrough device to a QEMU guest. Support for this was recently accepted into QEMU. This set of patches borrows a lot from the recently added support for rng's. Regards, Stefan --- v2->v3: - followed tree to 03122d7b - some bits fixed v1->v2: - Followed Daniel Berrange's comments (except for the one on de-consolidating the JSON monitor code)

Probe for QEMU's QMP TPM support by querying the lists of supported TPM models (query-tpm-models) and backend types (query-tpm-types). The setting of the capability flags following the strings returned from the commands above is only provided in the patch where domain_conf.c gets TPM support due to dependencies on functions only introduced there. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_monitor.c | 44 +++++++++++++++++++++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 90 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 8 +++ 6 files changed, 154 insertions(+) Index: libvirt/src/qemu/qemu_capabilities.c =================================================================== --- libvirt.orig/src/qemu/qemu_capabilities.c +++ libvirt/src/qemu/qemu_capabilities.c @@ -212,6 +212,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAS "rng-egd", "virtio-ccw", "dtb", + "tpm-passthrough", + + "tpm-tis", /* 135 */ ); struct _virQEMUCaps { Index: libvirt/src/qemu/qemu_capabilities.h =================================================================== --- libvirt.orig/src/qemu/qemu_capabilities.h +++ libvirt/src/qemu/qemu_capabilities.h @@ -173,6 +173,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_OBJECT_RNG_EGD = 131, /* EGD protocol daemon for rng */ QEMU_CAPS_VIRTIO_CCW = 132, /* -device virtio-*-ccw */ QEMU_CAPS_DTB = 133, /* -dtb file */ + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 134, /* -tpmdev passthrough */ + QEMU_CAPS_DEVICE_TPM_TIS = 135, /* -device tpm_tis */ QEMU_CAPS_LAST, /* this must always be the last item */ }; @@ -254,4 +256,5 @@ int virQEMUCapsParseDeviceStr(virQEMUCap VIR_ENUM_DECL(virQEMUCaps); bool virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps); + #endif /* __QEMU_CAPABILITIES_H__*/ Index: libvirt/src/qemu/qemu_monitor.c =================================================================== --- libvirt.orig/src/qemu/qemu_monitor.c +++ libvirt/src/qemu/qemu_monitor.c @@ -3522,3 +3522,47 @@ int qemuMonitorNBDServerStop(qemuMonitor return qemuMonitorJSONNBDServerStop(mon); } + + +int qemuMonitorGetTPMModels(qemuMonitorPtr mon, + char ***tpmmodels) +{ + VIR_DEBUG("mon=%p tpmmodels=%p", + mon, tpmmodels); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetTPMModels(mon, tpmmodels); +} + + +int qemuMonitorGetTPMTypes(qemuMonitorPtr mon, + char ***tpmtypes) +{ + VIR_DEBUG("mon=%p tpmtypes=%p", + mon, tpmtypes); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetTPMTypes(mon, tpmtypes); +} Index: libvirt/src/qemu/qemu_monitor.h =================================================================== --- libvirt.orig/src/qemu/qemu_monitor.h +++ libvirt/src/qemu/qemu_monitor.h @@ -683,6 +683,12 @@ int qemuMonitorNBDServerAdd(qemuMonitorP const char *deviceID, bool writable); int qemuMonitorNBDServerStop(qemuMonitorPtr); +int qemuMonitorGetTPMModels(qemuMonitorPtr mon, + char ***tpmmodels); + +int qemuMonitorGetTPMTypes(qemuMonitorPtr mon, + char ***tpmtypes); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that Index: libvirt/src/qemu/qemu_monitor_json.c =================================================================== --- libvirt.orig/src/qemu/qemu_monitor_json.c +++ libvirt/src/qemu/qemu_monitor_json.c @@ -41,6 +41,7 @@ #include "datatypes.h" #include "virerror.h" #include "virjson.h" +#include "virstring.h" #ifdef WITH_DTRACE_PROBES # include "libvirt_qemu_probes.h" @@ -4693,3 +4694,92 @@ qemuMonitorJSONNBDServerStop(qemuMonitor virJSONValueFree(reply); return ret; } + + +static int +qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd, + char ***array) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **list = NULL; + int n = 0; + size_t i; + + *array = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand(qmpCmd, NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply was missing return data"), + qmpCmd); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was not an array"), + qmpCmd); + goto cleanup; + } + + /* null-terminated list */ + if (VIR_ALLOC_N(list, n + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueGetString(child))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s array element does not contain data"), + qmpCmd); + goto cleanup; + } + + if (!(list[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *array = list; + +cleanup: + if (ret < 0) + virStringFreeList(list); + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +int qemuMonitorJSONGetTPMModels(qemuMonitorPtr mon, + char ***tpmmodels) +{ + return qemuMonitorJSONGetStringArray(mon, "query-tpm-models", tpmmodels); +} + + +int qemuMonitorJSONGetTPMTypes(qemuMonitorPtr mon, + char ***tpmtypes) +{ + return qemuMonitorJSONGetStringArray(mon, "query-tpm-types", tpmtypes); +} Index: libvirt/src/qemu/qemu_monitor_json.h =================================================================== --- libvirt.orig/src/qemu/qemu_monitor_json.h +++ libvirt/src/qemu/qemu_monitor_json.h @@ -341,4 +341,12 @@ int qemuMonitorJSONNBDServerAdd(qemuMoni const char *deviceID, bool writable); int qemuMonitorJSONNBDServerStop(qemuMonitorPtr mon); +int qemuMonitorJSONGetTPMModels(qemuMonitorPtr mon, + char ***tpmmodels) + ATTRIBUTE_NONNULL(2); + +int qemuMonitorJSONGetTPMTypes(qemuMonitorPtr mon, + char ***tpmtypes) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */

On 03/21/2013 11:42 AM, Stefan Berger wrote: [snip]
+ + +static int +qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd, + char ***array) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **list = NULL; + int n = 0; + size_t i; + + *array = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand(qmpCmd, NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply was missing return data"), + qmpCmd); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was not an array"), + qmpCmd); + goto cleanup; + } + + /* null-terminated list */ + if (VIR_ALLOC_N(list, n + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueGetString(child))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s array element does not contain data"), + qmpCmd); + goto cleanup; + } + + if (!(list[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *array = list; + +cleanup: + if (ret < 0) + virStringFreeList(list); + virJSONValueFree(cmd); + virJSONValueFree(reply);
Does data need to be freed? -- Regards, Corey Bryant
+ return ret; +} + +int qemuMonitorJSONGetTPMModels(qemuMonitorPtr mon, + char ***tpmmodels) +{ + return qemuMonitorJSONGetStringArray(mon, "query-tpm-models", tpmmodels); +} + + +int qemuMonitorJSONGetTPMTypes(qemuMonitorPtr mon, + char ***tpmtypes) +{ + return qemuMonitorJSONGetStringArray(mon, "query-tpm-types", tpmtypes); +} Index: libvirt/src/qemu/qemu_monitor_json.h =================================================================== --- libvirt.orig/src/qemu/qemu_monitor_json.h +++ libvirt/src/qemu/qemu_monitor_json.h @@ -341,4 +341,12 @@ int qemuMonitorJSONNBDServerAdd(qemuMoni const char *deviceID, bool writable); int qemuMonitorJSONNBDServerStop(qemuMonitorPtr mon); +int qemuMonitorJSONGetTPMModels(qemuMonitorPtr mon, + char ***tpmmodels) + ATTRIBUTE_NONNULL(2); + +int qemuMonitorJSONGetTPMTypes(qemuMonitorPtr mon, + char ***tpmtypes) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */

On 04/01/2013 03:45 PM, Corey Bryant wrote: >> + if (ret < 0) >> + goto cleanup; >> + >> + ret = -1; >> + >> + if (!(data = virJSONValueObjectGet(reply, "return"))) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("%s reply was missing return data"), >> + qmpCmd); >> + goto cleanup; >> + } >> + >> + if ((n = virJSONValueArraySize(data)) < 0) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("%s reply data was not an array"), >> + qmpCmd); >> + goto cleanup; >> + } >> + >> + /* null-terminated list */ >> + if (VIR_ALLOC_N(list, n + 1) < 0) { >> + virReportOOMError(); >> + goto cleanup; >> + } >> + >> + for (i = 0 ; i < n ; i++) { >> + virJSONValuePtr child = virJSONValueArrayGet(data, i); >> + const char *tmp; >> + >> + if (!(tmp = virJSONValueGetString(child))) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("%s array element does not contain data"), >> + qmpCmd); >> + goto cleanup; >> + } >> + >> + if (!(list[i] = strdup(tmp))) { >> + virReportOOMError(); >> + goto cleanup; >> + } >> + } >> + >> + ret = n; >> + *array = list; >> + >> +cleanup: >> + if (ret < 0) >> + virStringFreeList(list); >> + virJSONValueFree(cmd); >> + virJSONValueFree(reply); > + > > Does data need to be freed? > It doesn't seem to be the case for the 'return' object. No other code frees it, either. Stefan

On 04/01/2013 01:52 PM, Stefan Berger wrote: > On 04/01/2013 03:45 PM, Corey Bryant wrote: >>> + if (ret < 0) >>> + goto cleanup; >>> + >>> + ret = -1; >>> + >>> + if (!(data = virJSONValueObjectGet(reply, "return"))) { >>> + virReportError(VIR_ERR_INTERNAL_ERROR, >>> + _("%s reply was missing return data"), >>> + qmpCmd); >>> + goto cleanup; >>> + } >>> + >>> +cleanup: >>> + if (ret < 0) >>> + virStringFreeList(list); >>> + virJSONValueFree(cmd); >>> + virJSONValueFree(reply); >> + >> >> Does data need to be freed? >> > > It doesn't seem to be the case for the 'return' object. No other code > frees it, either. data is a pointer to contents already within reply; freeing reply is recursive, and explicitly freeing data in addition to reply would be a double-free bug. This aspect of the code is correct as written. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/util/virstring.c | 14 ++++++++++++++ src/util/virstring.h | 2 ++ 3 files changed, 17 insertions(+) Index: libvirt/src/util/virstring.c =================================================================== --- libvirt.orig/src/util/virstring.c +++ libvirt/src/util/virstring.c @@ -166,3 +166,17 @@ void virStringFreeList(char **strings) } VIR_FREE(strings); } + + +bool +virStringArrayHasString(char **strings, const char *needle) +{ + size_t i = 0; + + while (strings[i]) { + if (STREQ(strings[i++], needle)) + return true; + } + + return false; +} Index: libvirt/src/util/virstring.h =================================================================== --- libvirt.orig/src/util/virstring.h +++ libvirt/src/util/virstring.h @@ -35,4 +35,6 @@ char *virStringJoin(const char **strings void virStringFreeList(char **strings); +bool virStringArrayHasString(char **strings, const char *needle); + #endif /* __VIR_STRING_H__ */ Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -1714,6 +1714,7 @@ virStorageFileResize; # util/virstring.h +virStringArrayHasString; virStringFreeList; virStringJoin; virStringSplit;

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 57 ++++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 43 +++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) Index: libvirt/docs/formatdomain.html.in =================================================================== --- libvirt.orig/docs/formatdomain.html.in +++ libvirt/docs/formatdomain.html.in @@ -4376,6 +4376,63 @@ qemu-kvm -net nic,model=? /dev/null </dl> + <h4><a name="elementsTpm">TPM device</a></h4> + + <p> + The TPM device enables a QEMU guest to have access to TPM + functionality. + </p> + <p> + The TPM passthrough device type provides access to the host's TPM + for one QEMU guest. No other software may be is using the TPM device, + typically /dev/tpm0, at the time the QEMU guest is started. + <span class="since">'passthrough' since 1.0.4</span> + </p> + + <p> + Example: usage of the TPM passthrough device + </p> +<pre> + ... + <device> + <tpm model='tpm-tis'> + <backend type='passthrough'> + <backend path='/dev/tpm0'/> + </backend> + </tpm> + </device> +</pre> + <dl> + <dt><code>model</code></dt> + <dd> + <p> + The <code>model</code> attribute specifies what device + model QEMU provides to the guest. If no model name is provided, + <code>tpm-tis</code> will automatically be chosen. + </p> + </dd> + <dt><code>backend</code></dt> + <dd> + <p> + The <code>backend</code> element specifies the type of + TPM device. The following types are supported: + </p> + <ul> + <li>'passthrough' — use the hosts's TPM device. + </ul> + </dd> + <dt><code>backend type='passthrough'</code></dt> + <dd> + <p> + This backend type requires exclusive access to a TPM device on + the host. + An example for such a device is /dev/tpm0. The filename is + specified as path attribute of the <code>source</code> element. + If no file name is specified then /dev/tpm0 is automatically used. + </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> Index: libvirt/docs/schemas/domaincommon.rng =================================================================== --- libvirt.orig/docs/schemas/domaincommon.rng +++ libvirt/docs/schemas/domaincommon.rng @@ -2824,6 +2824,48 @@ <text/> </element> </define> + + <define name="tpm"> + <element name="tpm"> + <optional> + <attribute name="model"> + <choice> + <value>tpm-tis</value> + </choice> + </attribute> + </optional> + <ref name="tpm-backend"/> + <optional> + <ref name="alias"/> + </optional> + </element> + </define> + + <define name="tpm-backend"> + <element name="backend"> + <choice> + <group> + <attribute name="type"> + <value>passthrough</value> + </attribute> + <ref name="tpm-passthrough-device"/> + </group> + </choice> + </element> + </define> + + <define name="tpm-passthrough-device"> + <optional> + <element name="device"> + <optional> + <attribute name="path"> + <ref name="filePath"/> + </attribute> + </optional> + </element> + </optional> + </define> + <define name="input"> <element name="input"> <attribute name="type"> @@ -3134,6 +3176,7 @@ <ref name="redirdev"/> <ref name="redirfilter"/> <ref name="rng"/> + <ref name="tpm"/> </choice> </zeroOrMore> <optional>

On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- docs/formatdomain.html.in | 57 ++++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 43 +++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+)
Index: libvirt/docs/formatdomain.html.in =================================================================== --- libvirt.orig/docs/formatdomain.html.in +++ libvirt/docs/formatdomain.html.in @@ -4376,6 +4376,63 @@ qemu-kvm -net nic,model=? /dev/null
</dl>
+ <h4><a name="elementsTpm">TPM device</a></h4> + + <p> + The TPM device enables a QEMU guest to have access to TPM + functionality. + </p> + <p> + The TPM passthrough device type provides access to the host's TPM + for one QEMU guest. No other software may be is using the TPM device, + typically /dev/tpm0, at the time the QEMU guest is started. + <span class="since">'passthrough' since 1.0.4</span> + </p> + + <p> + Example: usage of the TPM passthrough device + </p> +<pre> + ... + <device> + <tpm model='tpm-tis'> + <backend type='passthrough'> + <backend path='/dev/tpm0'/> + </backend> + </tpm> + </device> +</pre> + <dl> + <dt><code>model</code></dt> + <dd> + <p> + The <code>model</code> attribute specifies what device + model QEMU provides to the guest. If no model name is provided, + <code>tpm-tis</code> will automatically be chosen. + </p> + </dd> + <dt><code>backend</code></dt> + <dd> + <p> + The <code>backend</code> element specifies the type of + TPM device. The following types are supported: + </p> + <ul> + <li>'passthrough' — use the hosts's TPM device.
s/hosts's/host's/
+ </ul> + </dd> + <dt><code>backend type='passthrough'</code></dt> + <dd> + <p> + This backend type requires exclusive access to a TPM device on + the host. + An example for such a device is /dev/tpm0. The filename is + specified as path attribute of the <code>source</code> element. + If no file name is specified then /dev/tpm0 is automatically used. + </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3>
<p> Index: libvirt/docs/schemas/domaincommon.rng =================================================================== --- libvirt.orig/docs/schemas/domaincommon.rng +++ libvirt/docs/schemas/domaincommon.rng @@ -2824,6 +2824,48 @@ <text/> </element> </define> + + <define name="tpm"> + <element name="tpm"> + <optional> + <attribute name="model"> + <choice> + <value>tpm-tis</value> + </choice> + </attribute> + </optional> + <ref name="tpm-backend"/> + <optional> + <ref name="alias"/> + </optional> + </element> + </define> + + <define name="tpm-backend"> + <element name="backend"> + <choice> + <group> + <attribute name="type"> + <value>passthrough</value> + </attribute> + <ref name="tpm-passthrough-device"/> + </group> + </choice> + </element> + </define> + + <define name="tpm-passthrough-device"> + <optional> + <element name="device"> + <optional> + <attribute name="path"> + <ref name="filePath"/> + </attribute>
Do you need to define cancel-path too?
+ </optional> + </element> + </optional> + </define> + <define name="input"> <element name="input"> <attribute name="type"> @@ -3134,6 +3176,7 @@ <ref name="redirdev"/> <ref name="redirfilter"/> <ref name="rng"/> + <ref name="tpm"/> </choice> </zeroOrMore> <optional>
-- Regards, Corey Bryant

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- po/POTFILES.in | 1 src/Makefile.am | 1 src/libvirt_private.syms | 4 + src/util/virtpm.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virtpm.h | 27 ++++++++++ 5 files changed, 157 insertions(+) Index: libvirt/src/Makefile.am =================================================================== --- libvirt.orig/src/Makefile.am +++ libvirt/src/Makefile.am @@ -122,6 +122,7 @@ UTIL_SOURCES = \ util/virthreadwin32.h \ util/virthreadpool.c util/virthreadpool.h \ util/virtime.h util/virtime.c \ + util/virtpm.h util/virtpm.c \ util/virtypedparam.c util/virtypedparam.h \ util/virusb.c util/virusb.h \ util/viruri.h util/viruri.c \ Index: libvirt/src/util/virtpm.c =================================================================== --- /dev/null +++ libvirt/src/util/virtpm.c @@ -0,0 +1,124 @@ +/* + * virtpm.c: TPM support + * + * Copyright (C) 2013 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ + +#include <config.h> + +#include <stdio.h> +#include <dirent.h> +#include <unistd.h> +#include <sys/stat.h> + +#include "virobject.h" +#include "viralloc.h" +#include "virutil.h" +#include "virerror.h" +#include "virbuffer.h" +#include "virtpm.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/* + * Check whether the given base path, e.g., /sys/class/misc/tpm0/device, + * is the sysfs entry of a TPM. A TPM sysfs entry should be uniquely + * recognizable by the file entries 'pcrs' and 'cancel'. + * Upon success 'true' is returned and the basebath buffer has '/cancel' + * appended. + */ +static bool +virTPMCheckSysfsCancel(char *basepath, size_t bufsz) +{ + char *path = NULL; + struct stat statbuf; + + if (virAsprintf(&path, "%s/pcrs", basepath) < 0) { + virReportOOMError(); + goto error; + } + if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) + goto error; + + VIR_FREE(path); + + if (virAsprintf(&path, "%s/cancel", basepath) < 0) { + virReportOOMError(); + goto error; + } + + if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) + goto error; + + if (!virStrncpy(basepath, path, strlen(path) + 1, bufsz)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Basepath buffer is too small")); + goto error; + } + + VIR_FREE(path); + + return true; + +error: + VIR_FREE(path); + return false; +} + + +char * +virTPMFindCancelPath(void) +{ + unsigned int idx; + int len; + DIR *pnp_dir; + char path[100], *p; + struct dirent entry, *result; + bool found = false; + + snprintf(path, sizeof(path), "/sys/class/misc"); + pnp_dir = opendir(path); + if (pnp_dir != NULL) { + while (readdir_r(pnp_dir, &entry, &result) == 0 && + result != NULL) { + if (sscanf(entry.d_name, "tpm%u%n", &idx, &len) < 1 || + len <= strlen("tpm") || + len != strlen(entry.d_name)) { + continue; + } + snprintf(path, sizeof(path), "/sys/class/misc/%s/device", + entry.d_name); + if (!virTPMCheckSysfsCancel(path, sizeof(path))) { + continue; + } + + found = true; + break; + } + closedir(pnp_dir); + } + + if (found) { + if (!(p = strdup(path))) + virReportOOMError(); + return p; + } + + return NULL; +} Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -1772,6 +1772,10 @@ virTimeStringThen; virTimeStringThenRaw; +# util/virtpm.h +virTPMFindCancelPath; + + # util/virtypedparam.h virTypedParameterArrayValidate; virTypedParameterAssign; Index: libvirt/src/util/virtpm.h =================================================================== --- /dev/null +++ libvirt/src/util/virtpm.h @@ -0,0 +1,27 @@ +/* + * virtpm.h: TPM support + * + * Copyright (C) 2013 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ +#ifndef __VIR_TPM_H__ +# define __VIR_TPM_H__ + +char *virTPMFindCancelPath(void); + +#endif /* __VIR_TPM_H__ */ Index: libvirt/po/POTFILES.in =================================================================== --- libvirt.orig/po/POTFILES.in +++ libvirt/po/POTFILES.in @@ -181,6 +181,7 @@ src/util/virsysinfo.c src/util/virerror.c src/util/virerror.h src/util/virtime.c +src/util/virtpm.c src/util/virtypedparam.c src/util/viruri.c src/util/virusb.c

On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- po/POTFILES.in | 1 src/Makefile.am | 1 src/libvirt_private.syms | 4 + src/util/virtpm.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virtpm.h | 27 ++++++++++ 5 files changed, 157 insertions(+)
Index: libvirt/src/Makefile.am =================================================================== --- libvirt.orig/src/Makefile.am +++ libvirt/src/Makefile.am @@ -122,6 +122,7 @@ UTIL_SOURCES = \ util/virthreadwin32.h \ util/virthreadpool.c util/virthreadpool.h \ util/virtime.h util/virtime.c \ + util/virtpm.h util/virtpm.c \ util/virtypedparam.c util/virtypedparam.h \ util/virusb.c util/virusb.h \ util/viruri.h util/viruri.c \ Index: libvirt/src/util/virtpm.c =================================================================== --- /dev/null +++ libvirt/src/util/virtpm.c @@ -0,0 +1,124 @@ +/* + * virtpm.c: TPM support + * + * Copyright (C) 2013 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + *<http://www.gnu.org/licenses/>. + * + * Author: Stefan Berger<stefanb@linux.vnet.ibm.com> + */ + +#include <config.h> + +#include <stdio.h> +#include <dirent.h> +#include <unistd.h> +#include <sys/stat.h> + +#include "virobject.h" +#include "viralloc.h" +#include "virutil.h" +#include "virerror.h" +#include "virbuffer.h" +#include "virtpm.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/* + * Check whether the given base path, e.g., /sys/class/misc/tpm0/device, + * is the sysfs entry of a TPM. A TPM sysfs entry should be uniquely + * recognizable by the file entries 'pcrs' and 'cancel'. + * Upon success 'true' is returned and the basebath buffer has '/cancel'
s/basebath/basepath
+ * appended. + */ +static bool +virTPMCheckSysfsCancel(char *basepath, size_t bufsz) +{ + char *path = NULL; + struct stat statbuf; + + if (virAsprintf(&path, "%s/pcrs", basepath) < 0) { + virReportOOMError(); + goto error; + } + if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) + goto error; + + VIR_FREE(path); + + if (virAsprintf(&path, "%s/cancel", basepath) < 0) { + virReportOOMError(); + goto error; + } + + if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode)) + goto error; + + if (!virStrncpy(basepath, path, strlen(path) + 1, bufsz)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Basepath buffer is too small")); + goto error; + } + + VIR_FREE(path); + + return true; + +error: + VIR_FREE(path); + return false; +} + + +char * +virTPMFindCancelPath(void) +{ + unsigned int idx; + int len; + DIR *pnp_dir; + char path[100], *p;
Is there any reason not to use PATH_MAX instead of 100 here?
+ struct dirent entry, *result; + bool found = false; + + snprintf(path, sizeof(path), "/sys/class/misc"); + pnp_dir = opendir(path); + if (pnp_dir != NULL) { + while (readdir_r(pnp_dir, &entry, &result) == 0 && + result != NULL) { + if (sscanf(entry.d_name, "tpm%u%n", &idx, &len) < 1 || + len <= strlen("tpm") || + len != strlen(entry.d_name)) { + continue; + } + snprintf(path, sizeof(path), "/sys/class/misc/%s/device", + entry.d_name); + if (!virTPMCheckSysfsCancel(path, sizeof(path))) { + continue; + } + + found = true; + break; + } + closedir(pnp_dir); + } + + if (found) { + if (!(p = strdup(path))) + virReportOOMError(); + return p; + } + + return NULL; +} Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -1772,6 +1772,10 @@ virTimeStringThen; virTimeStringThenRaw;
+# util/virtpm.h +virTPMFindCancelPath; + + # util/virtypedparam.h virTypedParameterArrayValidate; virTypedParameterAssign; Index: libvirt/src/util/virtpm.h =================================================================== --- /dev/null +++ libvirt/src/util/virtpm.h @@ -0,0 +1,27 @@ +/* + * virtpm.h: TPM support + * + * Copyright (C) 2013 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + *<http://www.gnu.org/licenses/>. + * + * Author: Stefan Berger<stefanb@linux.vnet.ibm.com> + */ +#ifndef __VIR_TPM_H__ +# define __VIR_TPM_H__ + +char *virTPMFindCancelPath(void); + +#endif /* __VIR_TPM_H__ */ Index: libvirt/po/POTFILES.in =================================================================== --- libvirt.orig/po/POTFILES.in +++ libvirt/po/POTFILES.in @@ -181,6 +181,7 @@ src/util/virsysinfo.c src/util/virerror.c src/util/virerror.h src/util/virtime.c +src/util/virtpm.c src/util/virtypedparam.c src/util/viruri.c src/util/virusb.c
-- Regards, Corey Bryant

On 04/01/2013 04:07 PM, Corey Bryant wrote:
On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
+ +char * +virTPMFindCancelPath(void) +{ + unsigned int idx; + int len; + DIR *pnp_dir; + char path[100], *p;
Is there any reason not to use PATH_MAX instead of 100 here?
You get a compiler error then due to the function using too much space on the stack. Stefan

On Tue, Apr 02, 2013 at 06:23:55AM -0400, Stefan Berger wrote:
On 04/01/2013 04:07 PM, Corey Bryant wrote:
On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
+ +char * +virTPMFindCancelPath(void) +{ + unsigned int idx; + int len; + DIR *pnp_dir; + char path[100], *p;
Is there any reason not to use PATH_MAX instead of 100 here?
You get a compiler error then due to the function using too much space on the stack.
The answer to that is *NOT* to use a shorter 100 byte buffer, but to use virAsprintf() instead of snprintf(). Never use fixed length buffers for filepaths. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Parse the domain XML with TPM passthrough support. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 33 ++++++++ src/libvirt_private.syms | 5 + 3 files changed, 217 insertions(+) Index: libvirt/src/conf/domain_conf.c =================================================================== --- libvirt.orig/src/conf/domain_conf.c +++ libvirt/src/conf/domain_conf.c @@ -51,6 +51,7 @@ #include "netdev_bandwidth_conf.h" #include "netdev_vlan_conf.h" #include "device_conf.h" +#include "virtpm.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -714,6 +715,13 @@ VIR_ENUM_IMPL(virDomainRNGBackend, "random", "egd"); +VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST, + "tpm-tis") + +VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST, + "passthrough") + + #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE @@ -1569,6 +1577,23 @@ void virDomainHostdevDefClear(virDomainH } } +void virDomainTPMDefFree(virDomainTPMDefPtr def) +{ + if (!def) + return; + + switch (def->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + VIR_FREE(def->data.passthrough.source.data.file.path); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + virDomainDeviceInfoClear(&def->info); + VIR_FREE(def); +} + void virDomainHostdevDefFree(virDomainHostdevDefPtr def) { if (!def) @@ -1830,6 +1855,8 @@ void virDomainDefFree(virDomainDefPtr de virDomainRNGDefFree(def->rng); + virDomainTPMDefFree(def->tpm); + VIR_FREE(def->os.type); VIR_FREE(def->os.machine); VIR_FREE(def->os.init); @@ -6473,6 +6500,103 @@ error: goto cleanup; } +/* Parse the XML definition for a TPM device + * + * The XML looks like this: + * + * <tpm model='tpm-tis'> + * <backend type='passthrough'> + * <device path='/dev/tpm0'/> + * </backend> + * </tpm> + * + */ +static virDomainTPMDefPtr +virDomainTPMDefParseXML(const xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + char *type = NULL; + char *path = NULL; + char *model = NULL; + char *backend = NULL; + virDomainTPMDefPtr def; + xmlNodePtr save = ctxt->node; + xmlNodePtr *backends = NULL; + int nbackends; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + model = virXMLPropString(node, "model"); + if (model != NULL && + (int)(def->model = virDomainTPMModelTypeFromString(model)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Unknown TPM frontend model '%s'"), model); + goto error; + } else { + def->model = VIR_DOMAIN_TPM_MODEL_TIS; + } + + ctxt->node = node; + + if ((nbackends = virXPathNodeSet("./backend", ctxt, &backends)) < 0) + goto error; + + if (nbackends > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one TPM backend is supported")); + goto error; + } + + if (!(backend = virXMLPropString(backends[0], "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing TPM device backend type")); + goto error; + } + + if ((int)(def->type = virDomainTPMBackendTypeFromString(backend)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unknown TPM backend type '%s'"), + backend); + goto error; + } + + switch (def->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + path = virXPathString("string(./backend/device/@path)", ctxt); + if (!path && !(path = strdup(VIR_DOMAIN_TPM_DEFAULT_DEVICE))) { + virReportOOMError(); + goto error; + } + def->data.passthrough.source.data.file.path = path; + def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV; + path = NULL; + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + goto error; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + +cleanup: + VIR_FREE(type); + VIR_FREE(path); + VIR_FREE(model); + VIR_FREE(backend); + VIR_FREE(backends); + ctxt->node = save; + return def; + +error: + virDomainTPMDefFree(def); + def = NULL; + goto cleanup; +} + /* Parse the XML definition for an input device */ static virDomainInputDefPtr virDomainInputDefParseXML(const char *ostype, @@ -10835,6 +10959,23 @@ virDomainDefParseXML(virCapsPtr caps, goto error; VIR_FREE(nodes); } + VIR_FREE(nodes); + + /* Parse the TPM devices */ + if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0) + goto error; + + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM device is supported")); + goto error; + } + + if (n > 0) { + if (!(def->tpm = virDomainTPMDefParseXML(nodes[0], ctxt, flags))) + goto error; + } + VIR_FREE(nodes); /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { @@ -13775,6 +13916,39 @@ virDomainSoundCodecDefFormat(virBufferPt } static int +virDomainTPMDefFormat(virBufferPtr buf, + virDomainTPMDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, " <tpm model='%s'>\n", + virDomainTPMModelTypeToString(def->model)); + + virBufferAsprintf(buf, " <backend type='%s'>\n", + virDomainTPMBackendTypeToString(def->type)); + + switch (def->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + virBufferEscapeString(buf, " <device path='%s'/>\n", + def->data.passthrough.source.data.file.path); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + virBufferAddLit(buf, " </backend>\n"); + + if (virDomainDeviceInfoIsSet(&def->info, flags)) { + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + } + + virBufferAddLit(buf, " </tpm>\n"); + + return 0; +} + + +static int virDomainSoundDefFormat(virBufferPtr buf, virDomainSoundDefPtr def, unsigned int flags) @@ -15099,6 +15273,11 @@ virDomainDefFormatInternal(virDomainDefP virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) goto error; + if (def->tpm) { + if (virDomainTPMDefFormat(buf, def->tpm, flags) < 0) + goto error; + } + if (def->ngraphics > 0) { /* If graphics is enabled, add the implicit mouse */ virDomainInputDef autoInput = { Index: libvirt/src/conf/domain_conf.h =================================================================== --- libvirt.orig/src/conf/domain_conf.h +++ libvirt/src/conf/domain_conf.h @@ -1080,6 +1080,33 @@ struct _virDomainHubDef { virDomainDeviceInfo info; }; +enum virDomainTPMModel { + VIR_DOMAIN_TPM_MODEL_TIS, + + VIR_DOMAIN_TPM_MODEL_LAST +}; + +enum virDomainTPMBackendType { + VIR_DOMAIN_TPM_TYPE_PASSTHROUGH, + + VIR_DOMAIN_TPM_TYPE_LAST +}; + +# define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0" + +typedef struct _virDomainTPMDef virDomainTPMDef; +typedef virDomainTPMDef *virDomainTPMDefPtr; +struct _virDomainTPMDef { + enum virDomainTPMBackendType type; + virDomainDeviceInfo info; + enum virDomainTPMModel model; + union { + struct { + virDomainChrSourceDef source; + } passthrough; + } data; +}; + enum virDomainInputType { VIR_DOMAIN_INPUT_TYPE_MOUSE, VIR_DOMAIN_INPUT_TYPE_TABLET, @@ -1866,6 +1893,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainTPMDefPtr tpm; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; @@ -2018,6 +2046,9 @@ void virDomainDeviceInfoClear(virDomainD void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearCCWAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); +void virDomainTPMDefFree(virDomainTPMDefPtr def); +char *virDomainTPMGetPathCopy(virDomainTPMDefPtr def); +char *virDomainTPMGetCancelPath(virDomainTPMDefPtr def); typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev, @@ -2382,6 +2413,8 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceMous VIR_ENUM_DECL(virDomainHyperv) VIR_ENUM_DECL(virDomainRNGModel) VIR_ENUM_DECL(virDomainRNGBackend) +VIR_ENUM_DECL(virDomainTPMModel) +VIR_ENUM_DECL(virDomainTPMBackend) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -320,6 +320,11 @@ virDomainTimerTickpolicyTypeFromString; virDomainTimerTickpolicyTypeToString; virDomainTimerTrackTypeFromString; virDomainTimerTrackTypeToString; +virDomainTPMBackendTypeFromString; +virDomainTPMBackendTypeToString; +virDomainTPMDefFree; +virDomainTPMModelTypeFromString; +virDomainTPMModelTypeToString; virDomainVcpuPinAdd; virDomainVcpuPinDefArrayFree; virDomainVcpuPinDefCopy;

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 56 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) Index: libvirt/src/qemu/qemu_capabilities.c =================================================================== --- libvirt.orig/src/qemu/qemu_capabilities.c +++ libvirt/src/qemu/qemu_capabilities.c @@ -38,6 +38,7 @@ #include "virbitmap.h" #include "virnodesuspend.h" #include "qemu_monitor.h" +#include "virstring.h" #include <fcntl.h> #include <sys/stat.h> @@ -2131,6 +2132,59 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEM static int +virQEMUCapsProbeQMPTPM(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + int nentries, i; + char **entries = NULL; + struct typeToCaps { + int type; + enum virQEMUCapsFlags caps; + }; + const struct typeToCaps tpmTypesToCaps[] = { + { + .type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH, + .caps = QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, + }, + }; + const struct typeToCaps tpmModelsToCaps[] = { + { + .type = VIR_DOMAIN_TPM_MODEL_TIS, + .caps = QEMU_CAPS_DEVICE_TPM_TIS, + }, + }; + + if ((nentries = qemuMonitorGetTPMModels(mon, &entries)) < 0) + return -1; + + if (nentries > 0) { + for (i = 0; i < ARRAY_CARDINALITY(tpmModelsToCaps); i++) { + const char *needle = virDomainTPMModelTypeToString( + tpmModelsToCaps[i].type); + if (virStringArrayHasString(entries, needle)) + virQEMUCapsSet(qemuCaps, tpmModelsToCaps[i].caps); + } + } + virStringFreeList(entries); + + if ((nentries = qemuMonitorGetTPMTypes(mon, &entries)) < 0) + return -1; + + if (nentries > 0) { + for (i = 0; i < ARRAY_CARDINALITY(tpmTypesToCaps); i++) { + const char *needle = virDomainTPMBackendTypeToString( + tpmTypesToCaps[i].type); + if (virStringArrayHasString(entries, needle)) + virQEMUCapsSet(qemuCaps, tpmTypesToCaps[i].caps); + } + } + virStringFreeList(entries); + + return 0; +} + + +static int virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) { @@ -2480,6 +2534,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCa goto cleanup; if (virQEMUCapsProbeQMPKVMState(qemuCaps, mon) < 0) goto cleanup; + if (virQEMUCapsProbeQMPTPM(qemuCaps, mon) < 0) + goto cleanup; ret = 0;

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/security/security_dac.c | 53 ++++++++++++++++++++++ src/security/security_selinux.c | 96 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) Index: libvirt/src/security/security_selinux.c =================================================================== --- libvirt.orig/src/security/security_selinux.c +++ libvirt/src/security/security_selinux.c @@ -45,6 +45,7 @@ #include "virrandom.h" #include "virutil.h" #include "virconf.h" +#include "virtpm.h" #define VIR_FROM_THIS VIR_FROM_SECURITY @@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData { #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux" +static int +virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm); + + /* * Returns 0 on success, 1 if already reserved, or -1 on fatal error */ @@ -1062,6 +1069,84 @@ err: return rc; } + +static int +virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm) +{ + int rc; + virSecurityLabelDefPtr seclabel; + char *cancel_path; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + rc = virSecuritySELinuxSetFilecon( + tpm->data.passthrough.source.data.file.path, + seclabel->imagelabel); + if (rc < 0) + return -1; + + if ((cancel_path = virTPMFindCancelPath()) != NULL) { + rc = virSecuritySELinuxSetFilecon(cancel_path, + seclabel->imagelabel); + VIR_FREE(cancel_path); + if (rc < 0) { + virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def, + tpm); + return -1; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot determine TPM command cancel path")); + return -1; + } + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return 0; +} + + +static int +virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm) +{ + int rc = 0; + virSecurityLabelDefPtr seclabel; + char *cancel_path; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + rc = virSecuritySELinuxRestoreSecurityFileLabel( + mgr, tpm->data.passthrough.source.data.file.path); + + if ((cancel_path = virTPMFindCancelPath()) != NULL) { + if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, + cancel_path) < 0) + rc = -1; + VIR_FREE(cancel_path); + } + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return rc; +} + + static int virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -1730,6 +1815,12 @@ virSecuritySELinuxRestoreSecurityAllLabe if (secdef->norelabel || data->skipAllLabel) return 0; + if (def->tpm) { + if (virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def, + def->tpm) < 0) + rc = -1; + } + for (i = 0 ; i < def->nhostdevs ; i++) { if (virSecuritySELinuxRestoreSecurityHostdevLabel(mgr, def, @@ -2144,6 +2235,11 @@ virSecuritySELinuxSetSecurityAllLabel(vi NULL) < 0) return -1; } + if (def->tpm) { + if (virSecuritySELinuxSetSecurityTPMFileLabel(mgr, def, + def->tpm) < 0) + return -1; + } if (virDomainChrDefForeach(def, true, Index: libvirt/src/security/security_dac.c =================================================================== --- libvirt.orig/src/security/security_dac.c +++ libvirt/src/security/security_dac.c @@ -716,6 +716,46 @@ virSecurityDACRestoreChardevCallback(vir static int +virSecurityDACSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm) +{ + int ret = 0; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + ret = virSecurityDACSetChardevLabel(mgr, def, + &tpm->data.passthrough.source); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + +static int +virSecurityDACRestoreSecurityTPMFileLabel( + virSecurityManagerPtr mgr, + virDomainTPMDefPtr tpm) +{ + int ret = 0; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + ret = virSecurityDACRestoreChardevLabel(mgr, + &tpm->data.passthrough.source); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + +static int virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, int migrated) @@ -752,6 +792,12 @@ virSecurityDACRestoreSecurityAllLabel(vi mgr) < 0) rc = -1; + if (def->tpm) { + if (virSecurityDACRestoreSecurityTPMFileLabel(mgr, + def->tpm) < 0) + rc = -1; + } + if (def->os.kernel && virSecurityDACRestoreSecurityFileLabel(def->os.kernel) < 0) rc = -1; @@ -815,6 +861,13 @@ virSecurityDACSetSecurityAllLabel(virSec mgr) < 0) return -1; + if (def->tpm) { + if (virSecurityDACSetSecurityTPMFileLabel(mgr, + def, + def->tpm) < 0) + return -1; + } + if (virSecurityDACGetImageIds(def, priv, &user, &group)) return -1;

On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- src/security/security_dac.c | 53 ++++++++++++++++++++++ src/security/security_selinux.c | 96 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+)
Index: libvirt/src/security/security_selinux.c =================================================================== --- libvirt.orig/src/security/security_selinux.c +++ libvirt/src/security/security_selinux.c @@ -45,6 +45,7 @@ #include "virrandom.h" #include "virutil.h" #include "virconf.h" +#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData { #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux"
+static int +virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm); + + /* * Returns 0 on success, 1 if already reserved, or -1 on fatal error */ @@ -1062,6 +1069,84 @@ err: return rc; }
+ +static int +virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm) +{ + int rc; + virSecurityLabelDefPtr seclabel; + char *cancel_path; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + rc = virSecuritySELinuxSetFilecon( + tpm->data.passthrough.source.data.file.path, + seclabel->imagelabel); + if (rc < 0) + return -1; + + if ((cancel_path = virTPMFindCancelPath()) != NULL) { + rc = virSecuritySELinuxSetFilecon(cancel_path, + seclabel->imagelabel); + VIR_FREE(cancel_path); + if (rc < 0) { + virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def, + tpm); + return -1; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot determine TPM command cancel path")); + return -1;
This makes me wonder if cancel-path should be specifiable at the libvirt level rather than just using the default sysfs entry. If I've read the code correctly I don't think it can currently be specified. However QEMU is capable of taking a cancel-path string in case it is different from the default sysfs path. -- Regards, Corey Bryant

On 04/01/2013 05:06 PM, Corey Bryant wrote:
On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- src/security/security_dac.c | 53 ++++++++++++++++++++++ src/security/security_selinux.c | 96 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+)
Index: libvirt/src/security/security_selinux.c =================================================================== --- libvirt.orig/src/security/security_selinux.c +++ libvirt/src/security/security_selinux.c @@ -45,6 +45,7 @@ #include "virrandom.h" #include "virutil.h" #include "virconf.h" +#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData { #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux"
+static int +virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm); + + /* * Returns 0 on success, 1 if already reserved, or -1 on fatal error */ @@ -1062,6 +1069,84 @@ err: return rc; }
+ +static int +virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm) +{ + int rc; + virSecurityLabelDefPtr seclabel; + char *cancel_path; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + rc = virSecuritySELinuxSetFilecon( + tpm->data.passthrough.source.data.file.path, + seclabel->imagelabel); + if (rc < 0) + return -1; + + if ((cancel_path = virTPMFindCancelPath()) != NULL) { + rc = virSecuritySELinuxSetFilecon(cancel_path, + seclabel->imagelabel); + VIR_FREE(cancel_path); + if (rc < 0) { + virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def, + tpm); + return -1; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot determine TPM command cancel path")); + return -1;
This makes me wonder if cancel-path should be specifiable at the libvirt level rather than just using the default sysfs entry. If I've read the code correctly I don't think it can currently be specified. However QEMU is capable of taking a cancel-path string in case it is different from the default sysfs path.
I am not sure whether to allow users to specify the path and misconfigure it and to have QEMU write a letter into the wrong file. I wonder whether we could have libvirt determine the path and display it in the XML as read-only, though. Stefan

On 04/02/2013 07:15 AM, Stefan Berger wrote:
On 04/01/2013 05:06 PM, Corey Bryant wrote:
On 03/21/2013 11:42 AM, Stefan Berger wrote:
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- src/security/security_dac.c | 53 ++++++++++++++++++++++ src/security/security_selinux.c | 96 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+)
Index: libvirt/src/security/security_selinux.c =================================================================== --- libvirt.orig/src/security/security_selinux.c +++ libvirt/src/security/security_selinux.c @@ -45,6 +45,7 @@ #include "virrandom.h" #include "virutil.h" #include "virconf.h" +#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData { #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux"
+static int +virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm); + + /* * Returns 0 on success, 1 if already reserved, or -1 on fatal error */ @@ -1062,6 +1069,84 @@ err: return rc; }
+ +static int +virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainTPMDefPtr tpm) +{ + int rc; + virSecurityLabelDefPtr seclabel; + char *cancel_path; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + rc = virSecuritySELinuxSetFilecon( + tpm->data.passthrough.source.data.file.path, + seclabel->imagelabel); + if (rc < 0) + return -1; + + if ((cancel_path = virTPMFindCancelPath()) != NULL) { + rc = virSecuritySELinuxSetFilecon(cancel_path, + seclabel->imagelabel); + VIR_FREE(cancel_path); + if (rc < 0) { + virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def, + tpm); + return -1; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot determine TPM command cancel path")); + return -1;
This makes me wonder if cancel-path should be specifiable at the libvirt level rather than just using the default sysfs entry. If I've read the code correctly I don't think it can currently be specified. However QEMU is capable of taking a cancel-path string in case it is different from the default sysfs path.
I am not sure whether to allow users to specify the path and misconfigure it and to have QEMU write a letter into the wrong file. I wonder whether we could have libvirt determine the path and display it in the XML as read-only, though.
Stefan
After discussing with Stefan some more, I think just using the default path is enough. I don't know why the sysfs path would not be the default anyway. And as far as I know we've decided not to support fd passing for vTPM, at least at this point, so that is not a concern. -- Regards, Corey Bryant

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/conf/domain_audit.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) Index: libvirt/src/conf/domain_audit.c =================================================================== --- libvirt.orig/src/conf/domain_audit.c +++ libvirt/src/conf/domain_audit.c @@ -524,6 +524,58 @@ cleanup: /** + * virDomainAuditTPM: + * @vm: domain making a change in pass-through host device + * @tpm: TPM device being attached or removed + * @reason: one of "start", "attach", or "detach" + * @success: true if the device passthrough operation succeeded + * + * Log an audit message about an attempted device passthrough change. + */ +static void +virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr tpm, + const char *reason, bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + char *path = NULL; + char *device = NULL; + const char *virt; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (!(vmname = virAuditEncode("vm", vm->def->name))) { + VIR_WARN("OOM while encoding audit message"); + return; + } + + if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) { + VIR_WARN("Unexpected virt type %d while encoding audit message", vm->def->virtType); + virt = "?"; + } + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + path = tpm->data.passthrough.source.data.file.path; + if (!(device = virAuditEncode("device", VIR_AUDIT_STR(path)))) { + VIR_WARN("OOM while encoding audit message"); + goto cleanup; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "virt=%s resrc=dev reason=%s %s uuid=%s %s", + virt, reason, vmname, uuidstr, device); + break; + default: + break; + } + +cleanup: + VIR_FREE(vmname); + VIR_FREE(device); +} + + +/** * virDomainAuditCgroup: * @vm: domain making the cgroups ACL change * @cgroup: cgroup that manages the devices @@ -761,6 +813,9 @@ virDomainAuditStart(virDomainObjPtr vm, if (vm->def->rng) virDomainAuditRNG(vm, vm->def->rng, NULL, "start", true); + if (vm->def->tpm) + virDomainAuditTPM(vm, vm->def->tpm, "start", true); + virDomainAuditMemory(vm, 0, vm->def->mem.cur_balloon, "start", true); virDomainAuditVcpu(vm, 0, vm->def->vcpus, "start", true);

Some refactoring for virDomainChrSourceDef type of devices so we can use common code. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_cgroup.c | 53 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-) Index: libvirt/src/qemu/qemu_cgroup.c =================================================================== --- libvirt.orig/src/qemu/qemu_cgroup.c +++ libvirt/src/qemu/qemu_cgroup.c @@ -144,34 +144,62 @@ int qemuTeardownDiskCgroup(virDomainObjP &data); } - static int -qemuSetupChardevCgroup(virDomainDefPtr def, - virDomainChrDefPtr dev, - void *opaque) +qemuSetupChrSourceCgroup(virDomainDefPtr def, + virDomainChrSourceDefPtr dev, + qemuCgroupData *data) { - qemuCgroupData *data = opaque; int rc; - if (dev->source.type != VIR_DOMAIN_CHR_TYPE_DEV) + if (dev->type != VIR_DOMAIN_CHR_TYPE_DEV) return 0; + VIR_DEBUG("Process path '%s' for device", dev->data.file.path); - VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); - rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path, + rc = virCgroupAllowDevicePath(data->cgroup, dev->data.file.path, VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupPath(data->vm, data->cgroup, "allow", - dev->source.data.file.path, "rw", rc); + dev->data.file.path, "rw", rc); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), - dev->source.data.file.path, def->name); + dev->data.file.path, def->name); return -1; } return 0; } +static int +qemuSetupChardevCgroup(virDomainDefPtr def, + virDomainChrDefPtr dev, + void *opaque) +{ + qemuCgroupData *data = opaque; + + return qemuSetupChrSourceCgroup(def, &dev->source, data); +} + + +static int +qemuSetupTPMCgroup(virDomainDefPtr def, + virDomainTPMDefPtr dev, + qemuCgroupData *data) +{ + int rc = 0; + + switch (dev->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + rc = qemuSetupChrSourceCgroup(def, &dev->data.passthrough.source, + data); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return rc; +} + int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED, const char *path, @@ -289,6 +317,11 @@ int qemuSetupCgroup(virQEMUDriverPtr dri &data) < 0) goto cleanup; + if (vm->def->tpm) + qemuSetupTPMCgroup(vm->def, + vm->def->tpm, + &data); + for (i = 0; i < vm->def->nhostdevs; i++) { virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; virUSBDevicePtr usb;

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 217 insertions(+) Index: libvirt/src/qemu/qemu_command.c =================================================================== --- libvirt.orig/src/qemu/qemu_command.c +++ libvirt/src/qemu/qemu_command.c @@ -46,6 +46,7 @@ #include "base64.h" #include "device_conf.h" #include "virstoragefile.h" +#include "virtpm.h" #include <sys/stat.h> #include <fcntl.h> @@ -791,6 +792,10 @@ qemuAssignDeviceAliases(virDomainDefPtr if (virAsprintf(&def->rng->info.alias, "rng%d", 0) < 0) goto no_memory; } + if (def->tpm) { + if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0) + goto no_memory; + } return 0; @@ -4674,6 +4679,92 @@ cleanup: } +static char *qemuBuildTPMBackendStr(const virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + const char *emulator) +{ + const virDomainTPMDefPtr tpm = def->tpm; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *type = virDomainTPMBackendTypeToString(tpm->type); + const char *cancel_path; + + virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias); + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH)) + goto no_support; + + virBufferAddLit(&buf, ",path="); + virBufferEscape(&buf, ',', ",", "%s", + tpm->data.passthrough.source.data.file.path); + + if (!(cancel_path = virTPMFindCancelPath())) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("TPM cancel path could not be determined")); + goto error; + } + + virBufferAddLit(&buf, ",cancel-path="); + virBufferEscape(&buf, ',', ",", "%s", cancel_path); + VIR_FREE(cancel_path); + + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + + no_support: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("The QEMU executable %s does not support TPM " + "backend type %s"), + emulator, type); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + + +static char *qemuBuildTPMDevStr(const virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + const char *emulator) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + const virDomainTPMDefPtr tpm = def->tpm; + const char *model = virDomainTPMModelTypeToString(tpm->model); + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_TIS)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("The QEMU executable %s does not support TPM " + "model %s"), + emulator, model); + goto error; + } + + virBufferAsprintf(&buf, "%s,tpmdev=tpm-%s,id=%s", + model, tpm->info.alias, tpm->info.alias); + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + + static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -6974,6 +7065,22 @@ qemuBuildCommandLine(virConnectPtr conn, } } + if (def->tpm) { + char *optstr; + + if (!(optstr = qemuBuildTPMBackendStr(def, qemuCaps, emulator))) + goto error; + + virCommandAddArgList(cmd, "-tpmdev", optstr, NULL); + VIR_FREE(optstr); + + if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator))) + goto error; + + virCommandAddArgList(cmd, "-device", optstr, NULL); + VIR_FREE(optstr); + } + for (i = 0 ; i < def->ninputs ; i++) { virDomainInputDefPtr input = def->inputs[i]; @@ -8801,6 +8908,112 @@ error: static int +qemuParseCommandLineTPM(virDomainDefPtr dom, + const char *val) +{ + int rc = 0; + virDomainTPMDefPtr tpm; + char **keywords; + char **values; + int nkeywords; + int i; + + if (dom->tpm) + goto error; + + nkeywords = qemuParseKeywords(val, &keywords, &values, 1); + if (nkeywords < 0) + goto error; + + if (VIR_ALLOC(tpm) < 0) + goto no_memory; + + tpm->model = VIR_DOMAIN_TPM_MODEL_TIS; + + for (i = 0; i < nkeywords; i++) { + if (STREQ(keywords[i], "type")) { + if (values[i] && + STREQ(values[i], + virDomainTPMBackendTypeToString(VIR_DOMAIN_TPM_TYPE_PASSTHROUGH))) + tpm->type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH; + } else if (STREQ(keywords[i], + virDomainTPMBackendTypeToString(VIR_DOMAIN_TPM_TYPE_PASSTHROUGH))) { + tpm->type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH; + } else if (STREQ(keywords[i], "path")) { + if (values[i]) { + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + tpm->data.passthrough.source.data.file.path = values[i]; + values[i] = NULL; + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + } else { + goto syntax; + } + } + } + + /* sanity checks */ + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + if (!tpm->data.passthrough.source.data.file.path) { + if (!(tpm->data.passthrough.source.data.file.path = + strdup(VIR_DOMAIN_TPM_DEFAULT_DEVICE))) { + virReportOOMError(); + goto bad_definition; + } + } + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("unknown TPM type")); + goto bad_definition; + } + + /* all ok */ + dom->tpm = tpm; + +cleanup: + for (i = 0 ; i < nkeywords ; i++) { + VIR_FREE(keywords[i]); + VIR_FREE(values[i]); + } + VIR_FREE(keywords); + VIR_FREE(values); + + + return rc; + +syntax: + virDomainTPMDefFree(tpm); + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown TPM syntax '%s'"), val); + rc = -1; + goto cleanup; + +bad_definition: + virDomainTPMDefFree(tpm); + + rc = -1; + goto cleanup; + +no_memory: + virReportOOMError(); + + rc = -1; + goto cleanup; + + +error: + return -1; +} + + +static int qemuParseCommandLineSmp(virDomainDefPtr dom, const char *val) { @@ -9599,6 +9812,10 @@ virDomainDefPtr qemuParseCommandLine(vir } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ + } else if (STREQ(arg, "-tpmdev")) { + WANT_VALUE(); + if (qemuParseCommandLineTPM(def, val) < 0) + goto error; } else { /* something we can't yet parse. Add it to the qemu namespace * cmdline/environment advanced options and hope for the best

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args | 6 +++ tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.xml | 29 +++++++++++++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 2 + 4 files changed, 40 insertions(+) Index: libvirt/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args =================================================================== --- /dev/null +++ libvirt/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/bin/qemu -S -M pc-0.12 -m 2048 -smp 1 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -boot c -usb \ +-tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,cancel-path=/sys/class/misc/tpm0/device/cancel \ +-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 Index: libvirt/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.xml =================================================================== --- /dev/null +++ libvirt/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>TPM-VM</name> + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>512288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.12'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <tpm model='tpm-tis'> + <backend type='passthrough'> + <device path='/dev/tpm0'/> + </backend> + </tpm> + <memballoon model='virtio'/> + </devices> +</domain> Index: libvirt/tests/qemuxml2argvtest.c =================================================================== --- libvirt.orig/tests/qemuxml2argvtest.c +++ libvirt/tests/qemuxml2argvtest.c @@ -918,6 +918,9 @@ mymain(void) DO_TEST("ppc-dtb", QEMU_CAPS_KVM, QEMU_CAPS_DTB); + DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE, + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlconf); Index: libvirt/tests/qemuxml2xmltest.c =================================================================== --- libvirt.orig/tests/qemuxml2xmltest.c +++ libvirt/tests/qemuxml2xmltest.c @@ -270,6 +270,8 @@ mymain(void) DO_TEST_DIFFERENT("metadata"); + DO_TEST("tpm-passthrough"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlconf);

Is anyone planning on reviewing this series? I have forward-ported it now with some nits fixed on the way. Regards, Stefan On 03/21/2013 11:42 AM, Stefan Berger wrote:
Hello!
The following set of patches adds support to libvirt for adding a TPM passthrough device to a QEMU guest. Support for this was recently accepted into QEMU.
This set of patches borrows a lot from the recently added support for rng's.
Regards, Stefan
--- v2->v3: - followed tree to 03122d7b - some bits fixed
v1->v2: - Followed Daniel Berrange's comments (except for the one on de-consolidating the JSON monitor code)

On 03/21/2013 11:42 AM, Stefan Berger wrote:
Hello!
The following set of patches adds support to libvirt for adding a TPM passthrough device to a QEMU guest. Support for this was recently accepted into QEMU.
This set of patches borrows a lot from the recently added support for rng's.
Regards, Stefan
--- v2->v3: - followed tree to 03122d7b - some bits fixed
v1->v2: - Followed Daniel Berrange's comments (except for the one on de-consolidating the JSON monitor code)
I tested this patch series out yesterday and everything is working as expected. Assuming the open issues (mostly nits) that I mentioned in my review are resolved: Reviewed-by: Corey Bryant <coreyb@linux.vnet.ibm.com> Tested-by: Corey Bryant <coreyb@linux.vnet.ibm.com> -- Regards, Corey Bryant
participants (4)
-
Corey Bryant
-
Daniel P. Berrange
-
Eric Blake
-
Stefan Berger