[RFC PATCH v2 0/8] LIBVIRT: X86: TDX support

Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes. * What's TDX? TDX stands for Trust Domain Extensions which isolates VMs from the virtual-machine manager (VMM)/hypervisor and any other software on the platform. To support TDX, multiple software components, not only KVM but also QEMU, guest Linux and virtual bios, need to be updated. For more details, please check link[1], there are TDX spec links and public repository link at github for each software component. This patchset is another software component to extend libvirt to support TDX, with which one can start a VM from high level rather than running qemu directly. * The goal of this RFC patch The purpose of this post is to get feedback early on high level design issue of libvirt enhancement for TDX. Referenced much on AMD SEV and S390 PV implemention at link[2][3]. This 2nd version is rebased on upstream + s390 v4 version as shown in [3] to utilize the common launchsecurity framework code. * Patch organization - patch 1-3: Support query of TDX capabilities. - patch 4-6: Add TDX type to launchsecurity framework. - patch 7: Add general loader support for TDX. - patch 8: Add firmware descriptor support for TDX. * Misc Just let you know we have released v2 version of TDX qemu in [1], and the API for libvirt is keeping stable. Using these patches we have succesfully booted and tested a guest both with and without TDX enabled. * Diff to v1: - give up using qmp cmd and check TDX directly on host for TDX capabilities. - use launchsecurity framework to support TDX - use <os>.<loader> for general loader - add auto firmware match feature for TDX A example TDVF fimware description file 70-edk2-x86_64-tdx.json: { "description": "UEFI firmware for x86_64, supporting Intel TDX", "interface-types": [ "uefi" ], "mapping": { "device": "generic", "filename": "/usr/share/OVMF/OVMF_CODE-tdx.fd" }, "targets": [ { "architecture": "x86_64", "machines": [ "pc-q35-*" ] } ], "features": [ "intel-tdx", "verbose-dynamic" ], "tags": [ ] } Links: [1] https://lists.nongnu.org/archive/html/qemu-devel/2021-07/msg01682.html [2] https://github.com/codomania/libvirt/commits/v9 [3] https://www.mail-archive.com/libvir-list@redhat.com/msg219144.html Zhenzhong Duan (8): qemu: Check if INTEL Trust Domain Extention support is enabled qemu: Add TDX capability conf: expose TDX feature in domain capabilities conf: add tdx as launch security type qemu: Add command line and validation for TDX type qemu: force special parameters enabled for TDX guest qemu: Add general loader support qemu: Add firmware descriptor support for TDX docs/formatdomaincaps.html.in | 17 ++++++ docs/schemas/domaincaps.rng | 9 +++ docs/schemas/domaincommon.rng | 18 ++++++ src/conf/domain_capabilities.c | 1 + src/conf/domain_capabilities.h | 1 + src/conf/domain_conf.c | 49 ++++++++++++++++ src/conf/domain_conf.h | 11 ++++ src/conf/virconftypes.h | 2 + src/qemu/qemu_capabilities.c | 44 ++++++++++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 38 +++++++++++++ src/qemu/qemu_firmware.c | 100 ++++++++++++++++++++++++++++++++- src/qemu/qemu_namespace.c | 2 + src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 28 +++++++++ 15 files changed, 319 insertions(+), 3 deletions(-) -- 2.25.1

Implement TDX check in order to generate domain feature capability correctly in case the availability of the feature changed. For INTEL TDX the verification is: - checking if /sys/firmware/tdx_seam/vendor_id contains the value "0x8086": meaning TDX is enabled in the host kernel. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- src/qemu/qemu_capabilities.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0d93cc2052..9085c0b875 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4748,6 +4748,24 @@ virQEMUCapsKVMSupportsSecureGuestAMD(void) } +/* + * Check whether INTEL Trust Domain Extention (x86) is enabled + */ +static bool +virQEMUCapsKVMSupportsSecureGuestINTEL(void) +{ + g_autofree char *modValue = NULL; + + if (virFileReadValueString(&modValue, "/sys/firmware/tdx_seam/vendor_id") < 0) + return false; + + if (STRNEQ(modValue,"0x8086")) + return false; + + return true; +} + + /* * Check whether the secure guest functionality is enabled. * See the specific architecture function for details on the verifications made. @@ -4761,7 +4779,8 @@ virQEMUCapsKVMSupportsSecureGuest(void) return virQEMUCapsKVMSupportsSecureGuestS390(); if (ARCH_IS_X86(arch)) - return virQEMUCapsKVMSupportsSecureGuestAMD(); + return virQEMUCapsKVMSupportsSecureGuestAMD() || + virQEMUCapsKVMSupportsSecureGuestINTEL(); return false; } -- 2.25.1

QEMU_CAPS_TDX_GUEST set means TDX supported with this qemu. Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9085c0b875..6a29ec607a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -637,6 +637,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "confidential-guest-support", "query-display-options", "s390-pv-guest", + "tdx-guest", ); @@ -1356,6 +1357,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-gpu-gl-pci", QEMU_CAPS_VIRTIO_GPU_GL_PCI }, { "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL }, { "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST }, + { "tdx-guest", QEMU_CAPS_TDX_GUEST}, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f99bb211e0..2aa38f55e9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -617,6 +617,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine confidential-guest-support */ QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */ QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */ + QEMU_CAPS_TDX_GUEST, /* -object tdx-guest,... */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.25.1

Extend qemu TDX capability to domain capabilities. Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- docs/formatdomaincaps.html.in | 17 +++++++++++++++++ docs/schemas/domaincaps.rng | 9 +++++++++ src/conf/domain_capabilities.c | 1 + src/conf/domain_capabilities.h | 1 + src/qemu/qemu_capabilities.c | 16 ++++++++++++++++ 5 files changed, 44 insertions(+) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 62f1940e6a..3f057af515 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -570,6 +570,7 @@ <cbitpos>47</cbitpos> <reduced-phys-bits>1</reduced-phys-bits> </sev> + <tdx supported='yes'/> </features> </domainCapabilities> </pre> @@ -635,6 +636,22 @@ a look at <a href="formatdomain.html#launchSecurity">SEV in domain XML</a> </p> + <h4><a id="featureTDX">TDX capabilities</a></h4> + + <p>Trust Domain Extensions(TDX) capabilities are exposed under the + <code>tdx</code> element. + TDX is an Intel technology that extends Virtual Machines Extensions (VMX) + to with a new kind of virtual machine guest called Trust Domain (TD). A TD + runs in a CPU model which protects the confidentiality of its memory contents + and its CPU state from any other software, including the hosting Virtual Machine + Monitor (VMM), unless explicitly shared by the TD itself.</p> + + <p> + For more details on the TDX feature, please follow resources in the + Intel developer's document. In order to use TDX with libvirt have + a look at <a href="formatdomain.html#launchSecurity">TDX in domain XML</a> + </p> + <dl> <dt><code>cbitpos</code></dt> <dd>When memory encryption is enabled, one of the physical address bits diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index d7ee60dd16..60001b3c43 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -253,6 +253,9 @@ <optional> <ref name="sev"/> </optional> + <optional> + <ref name="tdx"/> + </optional> </element> </define> @@ -307,6 +310,12 @@ </element> </define> + <define name="tdx"> + <element name="tdx"> + <ref name="supported"/> + </element> + </define> + <define name="value"> <zeroOrMore> <element name="value"> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 83d3320980..2380eacde9 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -43,6 +43,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature, "backingStoreInput", "backup", "s390-pv", + "tdx", ); static virClass *virDomainCapsClass; diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 34b9b8a693..cd3f5be472 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -180,6 +180,7 @@ typedef enum { VIR_DOMAIN_CAPS_FEATURE_BACKING_STORE_INPUT, VIR_DOMAIN_CAPS_FEATURE_BACKUP, VIR_DOMAIN_CAPS_FEATURE_S390_PV, + VIR_DOMAIN_CAPS_FEATURE_TDX, VIR_DOMAIN_CAPS_FEATURE_LAST } virDomainCapsFeature; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6a29ec607a..e9906a2f32 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6351,6 +6351,21 @@ virQEMUCapsFillDomainFeatureS390PVCaps(virQEMUCaps *qemuCaps, } +static void +virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps, + virDomainCaps *domCaps) +{ + if (ARCH_IS_X86(qemuCaps->arch)) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) && + virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps)) + domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_YES; + else + domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_NO; + } +} + + int virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps, virArch hostarch, @@ -6398,6 +6413,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps, virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps); virQEMUCapsFillDomainFeatureSEVCaps(qemuCaps, domCaps); virQEMUCapsFillDomainFeatureS390PVCaps(qemuCaps, domCaps); + virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps); return 0; } -- 2.25.1

When 'tdx' is used, the VM will launched with Intel TDX feature enabled. TDX feature supports running encrypted VM (Trust Domain, TD) under the control of KVM. A TD runs in a CPU model which protects the confidentiality of its memory and its CPU state from other software There is a child element 'policy' and three optional element for tdx type. In 'policy', bit 0 is used to enable TDX debug, other bits are reserved currently. mrconfigid, mrowner and mrownerconfig are hex string of 48 * 2 length each. For example: <launchSecurity type='tdx'> <policy>0x0001</policy> <mrconfigid>xxx...xxx</mrconfigid> <mrowner>xxx...xxx</mrowner> <mrownerconfig>xxx...xxx</mrownerconfig> </launchSecurity> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- docs/schemas/domaincommon.rng | 16 ++++++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 9 +++++++ src/conf/virconftypes.h | 2 ++ 4 files changed, 74 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b81c51728d..fd77601886 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -486,6 +486,7 @@ <choice> <value>sev</value> <value>s390-pv</value> + <value>tdx</value> </choice> </attribute> <interleave> @@ -519,6 +520,21 @@ <data type="string"/> </element> </optional> + <optional> + <element name="mrconfigid"> + <data type="string"/> + </element> + </optional> + <optional> + <element name="mrowner"> + <data type="string"/> + </element> + </optional> + <optional> + <element name="mrownerconfig"> + <data type="string"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 92ab22d3fd..9510aa7b1f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1402,6 +1402,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity, "", "sev", "s390-pv", + "tdx", ); static virClass *virDomainObjClass; @@ -3502,6 +3503,10 @@ virDomainSecDefFree(virDomainSecDef *def) g_free(def->data.sev.dh_cert); g_free(def->data.sev.session); break; + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + g_free(def->data.tdx.mrconfigid); + g_free(def->data.tdx.mrowner); + g_free(def->data.tdx.mrownerconfig); case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: @@ -14773,6 +14778,29 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, } +static int +virDomainTDXDefParseXML(virDomainTDXDef *def, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + unsigned long policy; + + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get launch security policy for " + "launch security type TDX")); + return -1; + } + + def->policy = policy; + def->mrconfigid = virXPathString("string(./mrconfigid)", ctxt); + def->mrowner = virXPathString("string(./mrowner)", ctxt); + def->mrownerconfig = virXPathString("string(./mrownerconfig)", ctxt); + + return 0; +} + + static virDomainSecDef * virDomainSecDefParseXML(xmlNodePtr lsecNode, xmlXPathContextPtr ctxt) @@ -14792,6 +14820,10 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode, if (virDomainSEVDefParseXML(&sec->data.sev, lsecNode, ctxt) < 0) return NULL; break; + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + if (virDomainTDXDefParseXML(&sec->data.tdx, ctxt) < 0) + return NULL; + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: if ((n = virXPathNodeSet("./*", ctxt, NULL)) < 0) return NULL; @@ -26932,6 +26964,21 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) break; } + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: { + virDomainTDXDef *tdx = &sec->data.tdx; + + virBufferAsprintf(&childBuf, "<policy>0x%04x</policy>\n", tdx->policy); + + if (tdx->mrconfigid) + virBufferEscapeString(&childBuf, "<mrconfigid>%s</mrconfigid>\n", tdx->mrconfigid); + if (tdx->mrowner) + virBufferEscapeString(&childBuf, "<mrowner>%s</mrowner>\n", tdx->mrowner); + if (tdx->mrownerconfig) + virBufferEscapeString(&childBuf, "<mrownerconfig>%s</mrownerconfig>\n", tdx->mrownerconfig); + + break; + } + case VIR_DOMAIN_LAUNCH_SECURITY_PV: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5c22f252d0..b29045d0c4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2646,6 +2646,7 @@ typedef enum { VIR_DOMAIN_LAUNCH_SECURITY_NONE, VIR_DOMAIN_LAUNCH_SECURITY_SEV, VIR_DOMAIN_LAUNCH_SECURITY_PV, + VIR_DOMAIN_LAUNCH_SECURITY_TDX, VIR_DOMAIN_LAUNCH_SECURITY_LAST, } virDomainLaunchSecurity; @@ -2661,10 +2662,18 @@ struct _virDomainSEVDef { unsigned int reduced_phys_bits; }; +struct _virDomainTDXDef { + unsigned int policy; + char *mrconfigid; + char *mrowner; + char *mrownerconfig; +}; + struct _virDomainSecDef { virDomainLaunchSecurity sectype; union { virDomainSEVDef sev; + virDomainTDXDef tdx; } data; }; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 21420ba8ea..e920f9a945 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -202,6 +202,8 @@ typedef struct _virDomainResourceDef virDomainResourceDef; typedef struct _virDomainSEVDef virDomainSEVDef; +typedef struct _virDomainTDXDef virDomainTDXDef; + typedef struct _virDomainSecDef virDomainSecDef; typedef struct _virDomainShmemDef virDomainShmemDef; -- 2.25.1

QEMU will provides 'tdx-guest' object which is used to launch encrypted VMs on Intel platform using TDX feature. A typical TDX guest launch command line looks like: $QEMU ... \ -object tdx-guest,id=tdx0,debug=on \ -machine q35,confidential-guest-support=tdx0,kvm-type=tdx Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- src/qemu/qemu_command.c | 33 +++++++++++++++++++++++++++++++++ src/qemu/qemu_firmware.c | 1 + src/qemu/qemu_namespace.c | 1 + src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 10 ++++++++++ 5 files changed, 46 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index db78deb122..2bc8173d58 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6979,6 +6979,9 @@ qemuBuildMachineCommandLine(virCommand *cmd, case VIR_DOMAIN_LAUNCH_SECURITY_PV: virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); break; + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + virBufferAddLit(&buf, ",confidential-guest-support=lsec0,kvm-type=tdx"); + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: @@ -9897,6 +9900,33 @@ qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd) } +static int +qemuBuildTDXCommandLine(virDomainObj *vm, virCommand *cmd, + virDomainTDXDef *tdx) +{ + g_autoptr(virJSONValue) props = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + qemuDomainObjPrivate *priv = vm->privateData; + + VIR_DEBUG("policy=0x%x", tdx->policy); + + if (qemuMonitorCreateObjectProps(&props, "tdx-guest", "lsec0", + "B:debug", !!(tdx->policy & 1), + "S:mrconfigid", tdx->mrconfigid, + "S:mrowner", tdx->mrowner, + "S:mrownerconfig", tdx->mrownerconfig, + NULL) < 0) + return -1; + + if (qemuBuildObjectCommandlineFromJSON(&buf, props, priv->qemuCaps) < 0) + return -1; + + virCommandAddArg(cmd, "-object"); + virCommandAddArgBuffer(cmd, &buf); + return 0; +} + + static int qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, virDomainSecDef *sec) @@ -9911,6 +9941,9 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, case VIR_DOMAIN_LAUNCH_SECURITY_PV: return qemuBuildPVCommandLine(vm, cmd); break; + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + return qemuBuildTDXCommandLine(vm, cmd, &sec->data.tdx); + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 77c452746f..e144b36f94 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1070,6 +1070,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, } break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 42865a6497..e902f0eecc 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -608,6 +608,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm, VIR_DEBUG("Set up launch security for SEV"); break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f2a523e4f7..b5324c85a1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6706,6 +6706,7 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuProcessPrepareSEVGuestInput(vm); case VIR_DOMAIN_LAUNCH_SECURITY_PV: + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7482bedee6..309d48e62f 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1234,6 +1234,16 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } break; + case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) || + !virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("INTEL TDX launch security is not supported with " + "this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: -- 2.25.1

TDX guest requires some special parameters to boot, They are: "-machine q35-*" "pic=no" "kernel_irqchip=split" Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_validate.c | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2bc8173d58..c53b0e237d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6980,7 +6980,7 @@ qemuBuildMachineCommandLine(virCommand *cmd, virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); break; case VIR_DOMAIN_LAUNCH_SECURITY_TDX: - virBufferAddLit(&buf, ",confidential-guest-support=lsec0,kvm-type=tdx"); + virBufferAddLit(&buf, ",confidential-guest-support=lsec0,kvm-type=tdx,pic=no"); break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 309d48e62f..2cb05dc5b2 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1243,6 +1243,17 @@ qemuValidateDomainDef(const virDomainDef *def, "this QEMU binary")); return -1; } + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Intel TDX is supported with q35 machine types only")); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP) || + def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("INTEL TDX launch security needs split kernel irqchip")); + return -1; + } break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; -- 2.25.1

Intel TDX requires a general loader to hold its firmware TDVF. Add new loader type VIR_DOMAIN_LOADER_TYPE_GENERIC and VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC to support this feature. XML looks like: <os> <loader type='generic'>/path/to/TDVF-binary</loader> </os> Qemu command line looks like: $QEMU ... \ -device loader,file=/path/to/TDVF-binary Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 2 ++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_command.c | 5 +++++ src/qemu/qemu_namespace.c | 1 + 6 files changed, 14 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fd77601886..9d0b51ee12 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -319,6 +319,7 @@ <choice> <value>rom</value> <value>pflash</value> + <value>generic</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9510aa7b1f..fbbbe708d4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1311,6 +1311,7 @@ VIR_ENUM_IMPL(virDomainLoader, "none", "rom", "pflash", + "generic", ); VIR_ENUM_IMPL(virDomainIOAPIC, @@ -1333,6 +1334,7 @@ VIR_ENUM_IMPL(virDomainOsDefFirmware, "none", "bios", "efi", + "generic", ); VIR_ENUM_IMPL(virDomainOsDefFirmwareFeature, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b29045d0c4..99b74683a0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2163,6 +2163,7 @@ typedef enum { VIR_DOMAIN_LOADER_TYPE_NONE = 0, VIR_DOMAIN_LOADER_TYPE_ROM, VIR_DOMAIN_LOADER_TYPE_PFLASH, + VIR_DOMAIN_LOADER_TYPE_GENERIC, VIR_DOMAIN_LOADER_TYPE_LAST } virDomainLoader; @@ -2246,6 +2247,7 @@ typedef enum { VIR_DOMAIN_OS_DEF_FIRMWARE_NONE = 0, VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS = VIR_DOMAIN_LOADER_TYPE_ROM, VIR_DOMAIN_OS_DEF_FIRMWARE_EFI = VIR_DOMAIN_LOADER_TYPE_PFLASH, + VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC = VIR_DOMAIN_LOADER_TYPE_GENERIC, VIR_DOMAIN_OS_DEF_FIRMWARE_LAST } virDomainOsDefFirmware; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e9906a2f32..d3c30a17e7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5888,6 +5888,9 @@ virQEMUCapsFillDomainLoaderCaps(virDomainCapsLoader *capsLoader, VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type, VIR_DOMAIN_LOADER_TYPE_PFLASH); + VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type, + VIR_DOMAIN_LOADER_TYPE_GENERIC); + VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->readonly, VIR_TRISTATE_BOOL_YES, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c53b0e237d..99812e37d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9640,6 +9640,11 @@ qemuBuildDomainLoaderCommandLine(virCommand *cmd, qemuBuldDomainLoaderPflashCommandLine(cmd, loader, qemuCaps); break; + case VIR_DOMAIN_LOADER_TYPE_GENERIC: + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "loader,file=%s", loader->path); + break; + case VIR_DOMAIN_LOADER_TYPE_NONE: case VIR_DOMAIN_LOADER_TYPE_LAST: /* nada */ diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index e902f0eecc..aa635b1375 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -569,6 +569,7 @@ qemuDomainSetupLoader(virDomainObj *vm, if (loader) { switch ((virDomainLoader) loader->type) { case VIR_DOMAIN_LOADER_TYPE_ROM: + case VIR_DOMAIN_LOADER_TYPE_GENERIC: *paths = g_slist_prepend(*paths, g_strdup(loader->path)); break; -- 2.25.1

Add a firmware descriptor support for TDVF, then libvirt can auto match TDVF fimware with td-guest. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- docs/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_firmware.c | 101 +++++++++++++++++++++++++++++++++- src/qemu/qemu_validate.c | 7 +++ 4 files changed, 108 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9d0b51ee12..8232025bf7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -275,6 +275,7 @@ <choice> <value>bios</value> <value>efi</value> + <value>generic</value> </choice> </attribute> </optional> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d3c30a17e7..a01d4c26db 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5934,6 +5934,8 @@ virQEMUCapsFillDomainOSCaps(virDomainCapsOS *os, VIR_DOMAIN_CAPS_ENUM_SET(os->firmware, VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS); if (autoFirmwares & (1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_EFI)) VIR_DOMAIN_CAPS_ENUM_SET(os->firmware, VIR_DOMAIN_OS_DEF_FIRMWARE_EFI); + if (autoFirmwares & (1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC)) + VIR_DOMAIN_CAPS_ENUM_SET(os->firmware, VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC); if (virQEMUCapsFillDomainLoaderCaps(capsLoader, secure, firmwaresAlt ? firmwaresAlt : firmwares, diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index e144b36f94..28e006eb82 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -84,12 +84,18 @@ struct _qemuFirmwareMappingMemory { char *filename; }; +typedef struct _qemuFirmwareMappingGeneric qemuFirmwareMappingGeneric; +struct _qemuFirmwareMappingGeneric { + char *filename; +}; + typedef enum { QEMU_FIRMWARE_DEVICE_NONE = 0, QEMU_FIRMWARE_DEVICE_FLASH, QEMU_FIRMWARE_DEVICE_KERNEL, QEMU_FIRMWARE_DEVICE_MEMORY, + QEMU_FIRMWARE_DEVICE_GENERIC, QEMU_FIRMWARE_DEVICE_LAST } qemuFirmwareDevice; @@ -101,6 +107,7 @@ VIR_ENUM_IMPL(qemuFirmwareDevice, "flash", "kernel", "memory", + "generic", ); @@ -112,6 +119,7 @@ struct _qemuFirmwareMapping { qemuFirmwareMappingFlash flash; qemuFirmwareMappingKernel kernel; qemuFirmwareMappingMemory memory; + qemuFirmwareMappingGeneric generic; } data; }; @@ -135,6 +143,7 @@ typedef enum { QEMU_FIRMWARE_FEATURE_SECURE_BOOT, QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC, QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC, + QEMU_FIRMWARE_FEATURE_INTEL_TDX, QEMU_FIRMWARE_FEATURE_LAST } qemuFirmwareFeature; @@ -151,7 +160,8 @@ VIR_ENUM_IMPL(qemuFirmwareFeature, "requires-smm", "secure-boot", "verbose-dynamic", - "verbose-static" + "verbose-static", + "intel-tdx" ); @@ -213,6 +223,13 @@ qemuFirmwareMappingMemoryFreeContent(qemuFirmwareMappingMemory *memory) } +static void +qemuFirmwareMappingGenericFreeContent(qemuFirmwareMappingGeneric *generic) +{ + g_free(generic->filename); +} + + static void qemuFirmwareMappingFreeContent(qemuFirmwareMapping *mapping) { @@ -226,6 +243,9 @@ qemuFirmwareMappingFreeContent(qemuFirmwareMapping *mapping) case QEMU_FIRMWARE_DEVICE_MEMORY: qemuFirmwareMappingMemoryFreeContent(&mapping->data.memory); break; + case QEMU_FIRMWARE_DEVICE_GENERIC: + qemuFirmwareMappingGenericFreeContent(&mapping->data.generic); + break; case QEMU_FIRMWARE_DEVICE_NONE: case QEMU_FIRMWARE_DEVICE_LAST: break; @@ -424,6 +444,25 @@ qemuFirmwareMappingMemoryParse(const char *path, } +static int +qemuFirmwareMappingGenericParse(const char *path, + virJSONValue *doc, + qemuFirmwareMappingGeneric *generic) +{ + const char *filename; + + if (!(filename = virJSONValueObjectGetString(doc, "filename"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing 'filename' in '%s'"), + path); + } + + generic->filename = g_strdup(filename); + + return 0; +} + + static int qemuFirmwareMappingParse(const char *path, virJSONValue *doc, @@ -469,6 +508,10 @@ qemuFirmwareMappingParse(const char *path, if (qemuFirmwareMappingMemoryParse(path, mapping, &fw->mapping.data.memory) < 0) return -1; break; + case QEMU_FIRMWARE_DEVICE_GENERIC: + if (qemuFirmwareMappingGenericParse(path, mapping, &fw->mapping.data.generic) < 0) + return -1; + break; case QEMU_FIRMWARE_DEVICE_NONE: case QEMU_FIRMWARE_DEVICE_LAST: @@ -740,6 +783,19 @@ qemuFirmwareMappingMemoryFormat(virJSONValue *mapping, } +static int +qemuFirmwareMappingGenericFormat(virJSONValue *mapping, + qemuFirmwareMappingGeneric *generic) +{ + if (virJSONValueObjectAppendString(mapping, + "filename", + generic->filename) < 0) + return -1; + + return 0; +} + + static int qemuFirmwareMappingFormat(virJSONValue *doc, qemuFirmware *fw) @@ -764,6 +820,10 @@ qemuFirmwareMappingFormat(virJSONValue *doc, if (qemuFirmwareMappingMemoryFormat(mapping, &fw->mapping.data.memory) < 0) return -1; break; + case QEMU_FIRMWARE_DEVICE_GENERIC: + if (qemuFirmwareMappingGenericFormat(mapping, &fw->mapping.data.generic) < 0) + return -1; + break; case QEMU_FIRMWARE_DEVICE_NONE: case QEMU_FIRMWARE_DEVICE_LAST: @@ -905,6 +965,7 @@ qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw) case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS: return QEMU_FIRMWARE_OS_INTERFACE_BIOS; case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI: + case VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC: return QEMU_FIRMWARE_OS_INTERFACE_UEFI; case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE: case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST: @@ -932,6 +993,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, bool supportsSEVES = false; bool supportsSecureBoot = false; bool hasEnrolledKeys = false; + bool supportsTDX = false; int reqSecureBoot; int reqEnrolledKeys; @@ -995,6 +1057,10 @@ qemuFirmwareMatchDomain(const virDomainDef *def, hasEnrolledKeys = true; break; + case QEMU_FIRMWARE_FEATURE_INTEL_TDX: + supportsTDX = true; + break; + case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: case QEMU_FIRMWARE_FEATURE_NONE: @@ -1069,8 +1135,14 @@ qemuFirmwareMatchDomain(const virDomainDef *def, return false; } break; - case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + if (!supportsTDX) { + VIR_DEBUG("Domain requires TDX, firmware '%s' doesn't support it", + path); + return false; + } + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: @@ -1093,6 +1165,7 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver, const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash; const qemuFirmwareMappingKernel *kernel = &fw->mapping.data.kernel; const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory; + const qemuFirmwareMappingGeneric *generic = &fw->mapping.data.generic; size_t i; switch (fw->mapping.device) { @@ -1149,6 +1222,17 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver, def->os.loader->path); break; + case QEMU_FIRMWARE_DEVICE_GENERIC: + if (!def->os.loader) + def->os.loader = g_new0(virDomainLoaderDef, 1); + + def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_GENERIC; + def->os.loader->path = g_strdup(generic->filename); + + VIR_DEBUG("decided on loader '%s'", + def->os.loader->path); + break; + case QEMU_FIRMWARE_DEVICE_NONE: case QEMU_FIRMWARE_DEVICE_LAST: break; @@ -1183,6 +1267,7 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver, case QEMU_FIRMWARE_FEATURE_SECURE_BOOT: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: + case QEMU_FIRMWARE_FEATURE_INTEL_TDX: case QEMU_FIRMWARE_FEATURE_LAST: break; } @@ -1216,6 +1301,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw, case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: + case QEMU_FIRMWARE_FEATURE_INTEL_TDX: case QEMU_FIRMWARE_FEATURE_LAST: break; } @@ -1411,6 +1497,7 @@ qemuFirmwareGetSupported(const char *machine, qemuFirmware *fw = firmwares[i]; const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash; const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory; + const qemuFirmwareMappingGeneric *generic = &fw->mapping.data.generic; const char *fwpath = NULL; const char *nvrampath = NULL; size_t j; @@ -1421,7 +1508,10 @@ qemuFirmwareGetSupported(const char *machine, for (j = 0; j < fw->ninterfaces; j++) { switch (fw->interfaces[j]) { case QEMU_FIRMWARE_OS_INTERFACE_UEFI: - *supported |= 1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_EFI; + if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_GENERIC) + *supported |= 1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC; + else + *supported |= 1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_EFI; break; case QEMU_FIRMWARE_OS_INTERFACE_BIOS: *supported |= 1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS; @@ -1449,6 +1539,7 @@ qemuFirmwareGetSupported(const char *machine, case QEMU_FIRMWARE_FEATURE_SECURE_BOOT: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: + case QEMU_FIRMWARE_FEATURE_INTEL_TDX: case QEMU_FIRMWARE_FEATURE_LAST: break; } @@ -1464,6 +1555,10 @@ qemuFirmwareGetSupported(const char *machine, fwpath = memory->filename; break; + case QEMU_FIRMWARE_DEVICE_GENERIC: + fwpath = generic->filename; + break; + case QEMU_FIRMWARE_DEVICE_KERNEL: case QEMU_FIRMWARE_DEVICE_NONE: case QEMU_FIRMWARE_DEVICE_LAST: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 2cb05dc5b2..ca507868ad 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1254,6 +1254,13 @@ qemuValidateDomainDef(const virDomainDef *def, _("INTEL TDX launch security needs split kernel irqchip")); return -1; } + if ((!def->os.loader || + def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_GENERIC) && + def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_GENERIC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("INTEL TDX launch security needs generic loader type")); + return -1; + } break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; -- 2.25.1

On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes.
* What's TDX? TDX stands for Trust Domain Extensions which isolates VMs from the virtual-machine manager (VMM)/hypervisor and any other software on the platform.
To support TDX, multiple software components, not only KVM but also QEMU, guest Linux and virtual bios, need to be updated. For more details, please check link[1], there are TDX spec links and public repository link at github for each software component.
This patchset is another software component to extend libvirt to support TDX, with which one can start a VM from high level rather than running qemu directly.
* The goal of this RFC patch The purpose of this post is to get feedback early on high level design issue of libvirt enhancement for TDX. Referenced much on AMD SEV and S390 PV implemention at link[2][3]. This 2nd version is rebased on upstream + s390 v4 version as shown in [3] to utilize the common launchsecurity framework code.
* Patch organization - patch 1-3: Support query of TDX capabilities. - patch 4-6: Add TDX type to launchsecurity framework. - patch 7: Add general loader support for TDX. - patch 8: Add firmware descriptor support for TDX.
* Misc Just let you know we have released v2 version of TDX qemu in [1], and the API for libvirt is keeping stable. Using these patches we have succesfully booted and tested a guest both with and without TDX enabled.
Overall looks good. It's missing documentation and the QEMU patches are missing documentation as well. I was looking into Intel specification but I failed to find the necessary info there as well. What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data is supposed to be stored there, what are the limitation and so on. What I could gather these are exposed in the VM and are used for measurement but that's it. Another thing that I've missed in v1, QEMU patches are introducing new `-machine pic=no` option and for TDX PIC has to be disabled. The libvirt patches are putting it on the QEMU command line but it is not reflected in the VM XML, so I would say we need to introduce new hypervisor feature [1]: <features> ... <pic state='on|off'/> ... </features> [1] <https://libvirt.org/formatdomain.html#hypervisor-features>
* Diff to v1: - give up using qmp cmd and check TDX directly on host for TDX capabilities. - use launchsecurity framework to support TDX - use <os>.<loader> for general loader - add auto firmware match feature for TDX
A example TDVF fimware description file 70-edk2-x86_64-tdx.json: { "description": "UEFI firmware for x86_64, supporting Intel TDX", "interface-types": [ "uefi" ], "mapping": { "device": "generic",
I think using 'loader' as that's the actual device in QEMU used with this firmware will be better. The patches posted to QEMU doesn't extend `docs/interop/firmware.json` so this example may change once some specific format is accepted by QEMU community. You will most likely need to add the firmware descriptor to QEMU project as well (`pc-bios/descriptors/70-edk2-x86_64-tdx.json`). NOTE: The name should not use `edk2` if it's not edk2 based firmware. Pavel
"filename": "/usr/share/OVMF/OVMF_CODE-tdx.fd" }, "targets": [ { "architecture": "x86_64", "machines": [ "pc-q35-*" ] } ], "features": [ "intel-tdx", "verbose-dynamic" ], "tags": [
] }
Links: [1] https://lists.nongnu.org/archive/html/qemu-devel/2021-07/msg01682.html [2] https://github.com/codomania/libvirt/commits/v9 [3] https://www.mail-archive.com/libvir-list@redhat.com/msg219144.html
Zhenzhong Duan (8): qemu: Check if INTEL Trust Domain Extention support is enabled qemu: Add TDX capability conf: expose TDX feature in domain capabilities conf: add tdx as launch security type qemu: Add command line and validation for TDX type qemu: force special parameters enabled for TDX guest qemu: Add general loader support qemu: Add firmware descriptor support for TDX
docs/formatdomaincaps.html.in | 17 ++++++ docs/schemas/domaincaps.rng | 9 +++ docs/schemas/domaincommon.rng | 18 ++++++ src/conf/domain_capabilities.c | 1 + src/conf/domain_capabilities.h | 1 + src/conf/domain_conf.c | 49 ++++++++++++++++ src/conf/domain_conf.h | 11 ++++ src/conf/virconftypes.h | 2 + src/qemu/qemu_capabilities.c | 44 ++++++++++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 38 +++++++++++++ src/qemu/qemu_firmware.c | 100 ++++++++++++++++++++++++++++++++- src/qemu/qemu_namespace.c | 2 + src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 28 +++++++++ 15 files changed, 319 insertions(+), 3 deletions(-)
-- 2.25.1

-----Original Message----- From: Pavel Hrdina <phrdina@redhat.com> Sent: Wednesday, July 21, 2021 10:23 PM To: Duan, Zhenzhong <zhenzhong.duan@intel.com> Cc: libvir-list@redhat.com; pkrempa@redhat.com; berrange@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com> Subject: Re: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support
On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes.
* What's TDX? TDX stands for Trust Domain Extensions which isolates VMs from the virtual-machine manager (VMM)/hypervisor and any other software on the platform. [...] * Misc Just let you know we have released v2 version of TDX qemu in [1], and the API for libvirt is keeping stable. Using these patches we have succesfully booted and tested a guest both with and without TDX enabled.
Overall looks good. It's missing documentation and the QEMU patches are missing documentation as well. I was looking into Intel specification but I failed to find the necessary info there as well. What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data is supposed to be stored there, what are the limitation and so on. Oh, yes. Thanks for point out. We will add the doc both for qemu and libvirt.
What I could gather these are exposed in the VM and are used for measurement but that's it.
Another thing that I've missed in v1, QEMU patches are introducing new `- machine pic=no` option and for TDX PIC has to be disabled. The libvirt patches are putting it on the QEMU command line but it is not reflected in the VM XML, so I would say we need to introduce new hypervisor feature [1]:
<features> ... <pic state='on|off'/> ... </features>
[1] <https://libvirt.org/formatdomain.html#hypervisor-features>
Will add this feature.
* Diff to v1: - give up using qmp cmd and check TDX directly on host for TDX capabilities. - use launchsecurity framework to support TDX - use <os>.<loader> for general loader - add auto firmware match feature for TDX
A example TDVF fimware description file 70-edk2-x86_64-tdx.json: { "description": "UEFI firmware for x86_64, supporting Intel TDX", "interface-types": [ "uefi" ], "mapping": { "device": "generic",
I think using 'loader' as that's the actual device in QEMU used with this firmware will be better. The patches posted to QEMU doesn't extend `docs/interop/firmware.json` so this example may change once some specific format is accepted by QEMU community.
Will do.
You will most likely need to add the firmware descriptor to QEMU project as well (`pc-bios/descriptors/70-edk2-x86_64-tdx.json`). NOTE: The name should not use `edk2` if it's not edk2 based firmware.
I see, will do. Thanks very much for your suggestions. Regards Zhenzhong

-----Original Message----- From: Pavel Hrdina <phrdina@redhat.com> Sent: Wednesday, July 21, 2021 10:23 PM To: Duan, Zhenzhong <zhenzhong.duan@intel.com> Cc: libvir-list@redhat.com; pkrempa@redhat.com; berrange@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com> Subject: Re: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support
On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes. [...] * Misc Just let you know we have released v2 version of TDX qemu in [1], and the API for libvirt is keeping stable. Using these patches we have succesfully booted and tested a guest both with and without TDX enabled.
Overall looks good. It's missing documentation and the QEMU patches are missing documentation as well. I was looking into Intel specification but I failed to find the necessary info there as well. What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data is supposed to be stored there, what are the limitation and so on.
What I could gather these are exposed in the VM and are used for measurement but that's it.
Another thing that I've missed in v1, QEMU patches are introducing new `- machine pic=no` option and for TDX PIC has to be disabled. The libvirt patches are putting it on the QEMU command line but it is not reflected in the VM XML, so I would say we need to introduce new hypervisor feature [1]:
<features> ... <pic state='on|off'/> ... </features>
[1] <https://libvirt.org/formatdomain.html#hypervisor-features>
* Diff to v1: - give up using qmp cmd and check TDX directly on host for TDX capabilities. - use launchsecurity framework to support TDX - use <os>.<loader> for general loader - add auto firmware match feature for TDX
A example TDVF fimware description file 70-edk2-x86_64-tdx.json: { "description": "UEFI firmware for x86_64, supporting Intel TDX", "interface-types": [ "uefi" ], "mapping": { "device": "generic",
I think using 'loader' as that's the actual device in QEMU used with this firmware will be better. The patches posted to QEMU doesn't extend `docs/interop/firmware.json` so this example may change once some specific format is accepted by QEMU community. Hi Pavel,
Just want to clarify you want 'generic' changing to 'loader' only in 70-edk2-x86_64-tdx.json Or also want all the 'generic' and '_GENERIC' string in ('[RFC PATCH v2 8/8] qemu: Add firmware descriptor support for TDX') to be changed? Thanks Zhenzhong

On Wed, Jul 28, 2021 at 02:54:09AM +0000, Duan, Zhenzhong wrote:
-----Original Message----- From: Pavel Hrdina <phrdina@redhat.com> Sent: Wednesday, July 21, 2021 10:23 PM To: Duan, Zhenzhong <zhenzhong.duan@intel.com> Cc: libvir-list@redhat.com; pkrempa@redhat.com; berrange@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com> Subject: Re: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support
On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes. [...] * Misc Just let you know we have released v2 version of TDX qemu in [1], and the API for libvirt is keeping stable. Using these patches we have succesfully booted and tested a guest both with and without TDX enabled.
Overall looks good. It's missing documentation and the QEMU patches are missing documentation as well. I was looking into Intel specification but I failed to find the necessary info there as well. What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data is supposed to be stored there, what are the limitation and so on.
What I could gather these are exposed in the VM and are used for measurement but that's it.
Another thing that I've missed in v1, QEMU patches are introducing new `- machine pic=no` option and for TDX PIC has to be disabled. The libvirt patches are putting it on the QEMU command line but it is not reflected in the VM XML, so I would say we need to introduce new hypervisor feature [1]:
<features> ... <pic state='on|off'/> ... </features>
[1] <https://libvirt.org/formatdomain.html#hypervisor-features>
* Diff to v1: - give up using qmp cmd and check TDX directly on host for TDX capabilities. - use launchsecurity framework to support TDX - use <os>.<loader> for general loader - add auto firmware match feature for TDX
A example TDVF fimware description file 70-edk2-x86_64-tdx.json: { "description": "UEFI firmware for x86_64, supporting Intel TDX", "interface-types": [ "uefi" ], "mapping": { "device": "generic",
I think using 'loader' as that's the actual device in QEMU used with this firmware will be better. The patches posted to QEMU doesn't extend `docs/interop/firmware.json` so this example may change once some specific format is accepted by QEMU community. Hi Pavel,
Just want to clarify you want 'generic' changing to 'loader' only in 70-edk2-x86_64-tdx.json Or also want all the 'generic' and '_GENERIC' string in ('[RFC PATCH v2 8/8] qemu: Add firmware descriptor support for TDX') to be changed?
Hi, correct, from libvirt POV we will export it as 'generic' type so in libvirt code it makes sense to use 'generic' and '_GENERIC'. In QEMU in the file 70-edk2-x86_64-tdx.json we want to probably use 'loader' because that's the '-device' type. Pavel
Thanks Zhenzhong
participants (3)
-
Duan, Zhenzhong
-
Pavel Hrdina
-
Zhenzhong Duan