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/module/kvm_intel/parameters/tdx" contains the value 'Y': meaning TDX is enabled in the host kernel. Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; --- 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 83119e871a..5f806c68fb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5098,6 +5098,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/module/kvm_intel/parameters/tdx") < 0) + return false; + + if (modValue[0] != 'Y') + return false; + + return true; +}

+ + /* * Check whether the secure guest functionality is enabled. * See the specific architecture function for details on the verifications made. @@ -5111,7 +5129,8 @@ virQEMUCapsKVMSupportsSecureGuest(void) return virQEMUCapsKVMSupportsSecureGuestS390(); if (ARCH_IS_X86(arch)) - return virQEMUCapsKVMSupportsSecureGuestAMD(); + return virQEMUCapsKVMSupportsSecureGuestAMD() || + virQEMUCapsKVMSupportsSecureGuestINTEL(); return false; }

Extend qemu TDX capability to domain capabilities. Signed-off-by: Chenyi Qiang <chenyi.qiang(a)intel.com&gt; Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; --- docs/formatdomaincaps.rst | 1 + src/conf/domain_capabilities.c | 1 + src/conf/domain_capabilities.h | 1 + src/conf/schemas/domaincaps.rng | 9 +++++++++ src/qemu/qemu_capabilities.c | 15 +++++++++++++++ 5 files changed, 27 insertions(+) diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst index ef752a0f3a..3acc9a12b4 100644 --- a/docs/formatdomaincaps.rst +++ b/docs/formatdomaincaps.rst @@ -669,6 +669,7 @@ capabilities. All features occur as children of the main ``features`` element. <value>vapic</value> </enum> </hyperv> + <tdx supported='yes'/> </features> </domainCapabilities> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index f6e09dc584..0f9ddb1e48 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature, "backup", "async-teardown", "s390-pv", + "tdx", ); static virClass *virDomainCapsClass; diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 01bcfa2e39..cc44cf2363 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -250,6 +250,7 @@ typedef enum { VIR_DOMAIN_CAPS_FEATURE_BACKUP, VIR_DOMAIN_CAPS_FEATURE_ASYNC_TEARDOWN, VIR_DOMAIN_CAPS_FEATURE_S390_PV, + VIR_DOMAIN_CAPS_FEATURE_TDX, VIR_DOMAIN_CAPS_FEATURE_LAST } virDomainCapsFeature; diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng index e7aa4a1066..a5522b1e67 100644 --- a/src/conf/schemas/domaincaps.rng +++ b/src/conf/schemas/domaincaps.rng @@ -308,6 +308,9 @@ <optional> <ref name="s390-pv"/> </optional> + <optional> + <ref name="tdx"/> + </optional> <optional> <ref name="sev"/> </optional> @@ -363,6 +366,12 @@ </element> </define> + <define name="tdx"> + <element name="tdx"> + <ref name="supported"/> + </element> + </define> + <define name="sev"> <element name="sev"> <ref name="supported"/> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8764df5e9d..0b4988256f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6657,6 +6657,20 @@ virQEMUCapsFillDomainFeatureHypervCaps(virQEMUCaps *qemuCaps, } +static void +virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps, + virDomainCaps *domCaps) +{ + if (domCaps->arch == VIR_ARCH_X86_64 && + domCaps->virttype == VIR_DOMAIN_VIRT_KVM && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) && + virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&

+ (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps->machine, "pc-q35-")))

+ domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_YES; +} + + int virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps, virArch hostarch, @@ -6716,6 +6730,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps, virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps); virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps); virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto); + virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps); return 0; } -- 2.34.1

>-----Original Message----- >From: Daniel P. Berrangé <berrange(a)redhat.com&gt; >Subject: Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain >capabilities > >On Mon, Nov 27, 2023 at 04:55:13PM +0800, Zhenzhong Duan wrote: >> Extend qemu TDX capability to domain capabilities. >> >> Signed-off-by: Chenyi Qiang <chenyi.qiang(a)intel.com&gt; >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; >> --- >> docs/formatdomaincaps.rst | 1 + >> src/conf/domain_capabilities.c | 1 + >> src/conf/domain_capabilities.h | 1 + >> src/conf/schemas/domaincaps.rng | 9 +++++++++ >> src/qemu/qemu_capabilities.c | 15 +++++++++++++++ >> 5 files changed, 27 insertions(+) >> >> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst >> index ef752a0f3a..3acc9a12b4 100644 >> --- a/docs/formatdomaincaps.rst >> +++ b/docs/formatdomaincaps.rst >> @@ -669,6 +669,7 @@ capabilities. All features occur as children of the >main ``features`` element. >> <value>vapic</value> >> </enum> >> </hyperv> >> + <tdx supported='yes'/> >> </features> >> </domainCapabilities> >> >> diff --git a/src/conf/domain_capabilities.c >b/src/conf/domain_capabilities.c >> index f6e09dc584..0f9ddb1e48 100644 >> --- a/src/conf/domain_capabilities.c >> +++ b/src/conf/domain_capabilities.c >> @@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature, >> "backup", >> "async-teardown", >> "s390-pv", >> + "tdx", >> ); >> >> static virClass *virDomainCapsClass; >> diff --git a/src/conf/domain_capabilities.h >b/src/conf/domain_capabilities.h >> index 01bcfa2e39..cc44cf2363 100644 >> --- a/src/conf/domain_capabilities.h >> +++ b/src/conf/domain_capabilities.h >> @@ -250,6 +250,7 @@ typedef enum { >> VIR_DOMAIN_CAPS_FEATURE_BACKUP, >> VIR_DOMAIN_CAPS_FEATURE_ASYNC_TEARDOWN, >> VIR_DOMAIN_CAPS_FEATURE_S390_PV, >> + VIR_DOMAIN_CAPS_FEATURE_TDX, >> >> VIR_DOMAIN_CAPS_FEATURE_LAST >> } virDomainCapsFeature; >> diff --git a/src/conf/schemas/domaincaps.rng >b/src/conf/schemas/domaincaps.rng >> index e7aa4a1066..a5522b1e67 100644 >> --- a/src/conf/schemas/domaincaps.rng >> +++ b/src/conf/schemas/domaincaps.rng >> @@ -308,6 +308,9 @@ >> <optional> >> <ref name="s390-pv"/> >> </optional> >> + <optional> >> + <ref name="tdx"/> >> + </optional> >> <optional> >> <ref name="sev"/> >> </optional> >> @@ -363,6 +366,12 @@ >> </element> >> </define> >> >> + <define name="tdx"> >> + <element name="tdx"> >> + <ref name="supported"/> >> + </element> >> + </define> >> + >> <define name="sev"> >> <element name="sev"> >> <ref name="supported"/> >> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >> index 8764df5e9d..0b4988256f 100644 >> --- a/src/qemu/qemu_capabilities.c >> +++ b/src/qemu/qemu_capabilities.c >> @@ -6657,6 +6657,20 @@ >virQEMUCapsFillDomainFeatureHypervCaps(virQEMUCaps *qemuCaps, >> } >> >> >> +static void >> +virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps, >> + virDomainCaps *domCaps) >> +{ >> + if (domCaps->arch == VIR_ARCH_X86_64 && >> + domCaps->virttype == VIR_DOMAIN_VIRT_KVM && >> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) && >> + virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps) && >> + virQEMUCapsGet(qemuCaps, >QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) && > >Checking QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT is >overkill, as we >can assume that is implied to exist by virtue of QEMU_CAPS_TDX_GUEST >existing. Good catch, will remove it. > >> + (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps- >>machine, "pc-q35-"))) > >If QEMU has limited its support for TDX to just q35, then it would be >much better if QEMU patches for TDX provided a way to detect this via >QMP, so we don't need to do these string comparisons. IIUC, QEMU has no limitation on using i440FX with TDX. We had this check in libvirt as we thought i440FX is deprecated.

@Yamahata, Isaku, @Li, Xiaoyao, so now we have two choices: 1. remove this limitation in libvirt

2. add QMP on QEMU side to report this limitation to libvirt Which choice do you like?

QEMU will provides 'tdx-guest' object which is used to launch encrypted VMs on Intel platform using TDX feature. Command line looks like: $QEMU ... \ -object tdx-guest,id=lsec0,debug=on,sept-ve-disable=on,mrconfigid=xxx...xxx,mrowner=xxx...xxx,mrownerconfig=xxx...xxx,quote-generation-service=localhost:1234 \ -machine q35,confidential-guest-support=lsec0 Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; --- src/qemu/qemu_command.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_validate.c | 7 +++++++ 2 files changed, 34 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89905378e4..45223746f5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9645,6 +9645,32 @@ qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd) } +static int +qemuBuildTDXCommandLine(virDomainObj *vm, virCommand *cmd, + virDomainTDXDef *tdx) +{ + g_autoptr(virJSONValue) props = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + + VIR_DEBUG("policy=0x%x", tdx->policy); + + if (qemuMonitorCreateObjectProps(&props, "tdx-guest", "lsec0", + "B:debug", !!(tdx->policy & 0x1), + "b:sept-ve-disable", !!(tdx->policy & 0x10000000),

+ "S:mrconfigid", tdx->mrconfigid, + "S:mrowner", tdx->mrowner, + "S:mrownerconfig", tdx->mrownerconfig, + "S:quote-generation-service", tdx->QGS,

+ NULL) < 0) + return -1; + + if (qemuBuildObjectCommandlineFromJSON(cmd, props, priv->qemuCaps) < 0) + return -1; + + return 0;

+ + static int qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, virDomainSecDef *sec) @@ -9660,6 +9686,7 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, return qemuBuildPVCommandLine(vm, cmd); break; case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + return qemuBuildTDXCommandLine(vm, cmd, &sec->data.tdx); case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index af630796cd..5a9173e8ff 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1323,6 +1323,13 @@ qemuValidateDomainDef(const virDomainDef *def, } break; case VIR_DOMAIN_LAUNCH_SECURITY_TDX: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) ||

+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST)) { + 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: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); -- 2.34.1

-----Original Message----- From: Daniel P. Berrangé <berrange(a)redhat.com&gt; Subject: Re: [PATCH rfcv3 06/11] qemu: force special parameters enabled for TDX guest On Mon, Nov 27, 2023 at 04:55:16PM +0800, Zhenzhong Duan wrote: > TDX guest requires some special parameters to boot, They are: > > "-machine pc-q35-*" > "kernel_irqchip=split" > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; > --- > src/qemu/qemu_validate.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c > index 5a9173e8ff..c4f386fe99 100644 > --- a/src/qemu/qemu_validate.c > +++ b/src/qemu/qemu_validate.c > @@ -1329,6 +1329,16 @@ qemuValidateDomainDef(const virDomainDef *def, > _("INTEL TDX launch security is not supported with 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; > + } Ideally QMP 'MachineInfo' struct would report whether TDX is supported so we don't need to hardcode that.

> + if (def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("INTEL TDX launch security needs split kernel irqchip")); s/INTEL/Intel/ Ideally QEMU would automatically use the correct ioapic impl when no args are given to QEMU. That would let us do if (def->features[VIR_DOMAIN_FEATURE_IOAPIC] == VIR_DOMAIN_IOAPIC_KVM) { thus allowing IOAPIC_NONE (ie QEMU's default) or IOAPIC_QEMU (explicitly requested config). This will make TDX guest "just work" in more scenarios.

> + return -1; > + } > break; > case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > -- > 2.34.1 > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

From: Chenyi Qiang <chenyi.qiang(a)intel.com&gt; Add the new flag VIR_DOMAIN_REBOOT_HARD/VIR_DOMAIN_SHUTDOWN_HARD to carry out a hard reboot, which kills the QEMU process and creates a new one with the same definition.

Hard reboot will be the highest priority to check. If succeed, other reboot policy (i.e. agent and acpi) would be skipped. Otherwise, use the traditional way. To make the shutdown graceful, the workflow of hard reboot is similar to the existing fakeReboot procedure. - In qemuDomainObjPrivatePtr we have a bool hardReboot flag. - The virDomainReboot method sets this flag and then triggers a normal "system_powerdown" if we specify the reboot mode "--mode hard" - The QEMU process is started with '-no-shutdown' so that the guest CPUs pause when it powers off the guest. - When we receive the "SHUTDOWN" event from QEMU monitor, if hardReboot is not set, then check fakeReboot or the last choice to invoke qemuProcessKill command to shutdown normally. - If hardReboot was set, we spawn a background thread, which issues qemuProcessStop to kill the QEMU process and cleanup the working environment. Then issue qemuProcessStart to create a new QEMU process with the same domain definition. At last, issues 'cont' to start the CPUs again. The state transfer during the hardReboot is RUNNING->SHUTDOWN->SHUTOFF->PAUSED->RUNNING, This patch doesn't hide it as the states is not transient. Signed-off-by: Chenyi Qiang <chenyi.qiang(a)intel.com&gt; --- include/libvirt/libvirt-domain.h | 2 + src/qemu/qemu_domain.c | 18 +++++++ src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_driver.c | 76 +++++++++++++++++++++------ src/qemu/qemu_process.c | 88 +++++++++++++++++++++++++++++++- tools/virsh-domain.c | 8 ++- 6 files changed, 177 insertions(+), 19 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1902546bb..5cedbbd1fc 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1520,6 +1520,7 @@ typedef enum { VIR_DOMAIN_SHUTDOWN_INITCTL = (1 << 2), /* Use initctl (Since: 1.0.1) */ VIR_DOMAIN_SHUTDOWN_SIGNAL = (1 << 3), /* Send a signal (Since: 1.0.1) */ VIR_DOMAIN_SHUTDOWN_PARAVIRT = (1 << 4), /* Use paravirt guest control (Since: 1.2.5) */ + VIR_DOMAIN_SHUTDOWN_HARD = (1 << 5), /* Powercycle control (Since: 8.6.0) */ } virDomainShutdownFlagValues; int virDomainShutdown (virDomainPtr domain); @@ -1538,6 +1539,7 @@ typedef enum { VIR_DOMAIN_REBOOT_INITCTL = (1 << 2), /* Use initctl (Since: 1.0.1) */ VIR_DOMAIN_REBOOT_SIGNAL = (1 << 3), /* Send a signal (Since: 1.0.1) */ VIR_DOMAIN_REBOOT_PARAVIRT = (1 << 4), /* Use paravirt guest control (Since: 1.2.5) */ + VIR_DOMAIN_REBOOT_HARD = (1 << 5), /* Powercycle control (Since: 8.6.0) */ } virDomainRebootFlagValues; int virDomainReboot (virDomainPtr domain, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ae19ce884b..b65d85f91b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2614,6 +2614,9 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf, if (priv->fakeReboot) virBufferAddLit(buf, "<fakereboot/>\n"); + if (priv->hardReboot) + virBufferAddLit(buf, "<hardreboot/>\n"); + if (priv->qemuDevices && *priv->qemuDevices) { char **tmp = priv->qemuDevices; virBufferAddLit(buf, "<devices>\n"); @@ -3305,6 +3308,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1; + priv->hardReboot = virXPathBoolean("boolean(./hardreboot)", ctxt) == 1; + if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse qemu device list")); @@ -7445,6 +7450,19 @@ qemuDomainSetFakeReboot(virDomainObj *vm, qemuDomainSaveStatus(vm); } +void +qemuDomainSetHardReboot(virDomainObj *vm, + bool value) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + if (priv->hardReboot == value) + return; + + priv->hardReboot = value; + qemuDomainSaveStatus(vm); +} + static void qemuDomainCheckRemoveOptionalDisk(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1e56e50672..fd6058c5bc 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -137,6 +137,7 @@ struct _qemuDomainObjPrivate { * -no-reboot instead. */ virTristateBool allowReboot; + bool hardReboot; unsigned long migMaxBandwidth; char *origname; @@ -700,6 +701,9 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, void qemuDomainSetFakeReboot(virDomainObj *vm, bool value); +void qemuDomainSetHardReboot(virDomainObj *vm, + bool value); + int qemuDomainCheckDiskStartupPolicy(virQEMUDriver *driver, virDomainObj *vm, size_t diskIndex, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d00d2a27c6..86e8efbfcb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1788,6 +1788,7 @@ qemuDomainShutdownFlagsAgent(virDomainObj *vm, goto endjob; qemuDomainSetFakeReboot(vm, false); + qemuDomainSetHardReboot(vm, false); agent = qemuDomainObjEnterAgent(vm); ret = qemuAgentShutdown(agent, agentFlag); qemuDomainObjExitAgent(vm, agent); @@ -1800,7 +1801,8 @@ qemuDomainShutdownFlagsAgent(virDomainObj *vm, static int qemuDomainShutdownFlagsMonitor(virDomainObj *vm, - bool isReboot) + bool isReboot, + bool hard) { int ret = -1; qemuDomainObjPrivate *priv; @@ -1816,7 +1818,17 @@ qemuDomainShutdownFlagsMonitor(virDomainObj *vm, goto endjob; } - qemuDomainSetFakeReboot(vm, isReboot); + if (hard) { + /* hard reboot control the reboot */ + VIR_DEBUG("Set hard reboot %d in shutdown monitor", isReboot); + qemuDomainSetHardReboot(vm, isReboot); + qemuDomainSetFakeReboot(vm, false); + } else { + /* fake reboot control the reboot */ + VIR_DEBUG("Set fake reboot %d in shutdown monitor", isReboot); + qemuDomainSetFakeReboot(vm, isReboot); + qemuDomainSetHardReboot(vm, false); + } qemuDomainObjEnterMonitor(vm); ret = qemuMonitorSystemPowerdown(priv->mon); qemuDomainObjExitMonitor(vm); @@ -1832,12 +1844,13 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) virDomainObj *vm; int ret = -1; qemuDomainObjPrivate *priv; - bool useAgent = false, agentRequested, acpiRequested; + bool useAgent = false, agentRequested, acpiRequested, hardRequested; bool isReboot = false; bool agentForced; virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | - VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT | + VIR_DOMAIN_SHUTDOWN_HARD, -1); if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; @@ -1851,14 +1864,23 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) priv = vm->privateData; agentRequested = flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; acpiRequested = flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; + hardRequested = flags & VIR_DOMAIN_SHUTDOWN_HARD; + + if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + /* Take hard reboot as the highest priority. + * if failed, consider the agent and acpi */ + if (hardRequested) + ret = qemuDomainShutdownFlagsMonitor(vm, isReboot, true); + + if (!ret) + goto cleanup; /* Prefer agent unless we were requested to not to. */ if (agentRequested || (!flags && priv->agent)) useAgent = true; - if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0) - goto cleanup; - agentForced = agentRequested && !acpiRequested; if (useAgent) { ret = qemuDomainShutdownFlagsAgent(vm, isReboot, agentForced); @@ -1877,7 +1899,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } - ret = qemuDomainShutdownFlagsMonitor(vm, isReboot); + ret = qemuDomainShutdownFlagsMonitor(vm, isReboot, false); } cleanup: @@ -1913,6 +1935,7 @@ qemuDomainRebootAgent(virDomainObj *vm, goto endjob; qemuDomainSetFakeReboot(vm, false); + qemuDomainSetHardReboot(vm, false); agent = qemuDomainObjEnterAgent(vm); ret = qemuAgentShutdown(agent, agentFlag); qemuDomainObjExitAgent(vm, agent); @@ -1925,7 +1948,8 @@ qemuDomainRebootAgent(virDomainObj *vm, static int qemuDomainRebootMonitor(virDomainObj *vm, - bool isReboot) + bool isReboot, + bool hard) { qemuDomainObjPrivate *priv = vm->privateData; int ret = -1; @@ -1936,7 +1960,15 @@ qemuDomainRebootMonitor(virDomainObj *vm, if (virDomainObjCheckActive(vm) < 0) goto endjob; - qemuDomainSetFakeReboot(vm, isReboot); + if (hard) { + VIR_DEBUG("Set hard reboot %d in reboot monitor", isReboot); + qemuDomainSetHardReboot(vm, isReboot); + qemuDomainSetFakeReboot(vm, false); + } else { + VIR_DEBUG("Set fake reboot %d in reboot monitor", isReboot); + qemuDomainSetFakeReboot(vm, isReboot); + qemuDomainSetHardReboot(vm, false); + } qemuDomainObjEnterMonitor(vm); ret = qemuMonitorSystemPowerdown(priv->mon); qemuDomainObjExitMonitor(vm); @@ -1953,12 +1985,13 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) virDomainObj *vm; int ret = -1; qemuDomainObjPrivate *priv; - bool useAgent = false, agentRequested, acpiRequested; + bool useAgent = false, agentRequested, acpiRequested, hardRequested; bool isReboot = true; bool agentForced; virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN | - VIR_DOMAIN_REBOOT_GUEST_AGENT, -1); + VIR_DOMAIN_REBOOT_GUEST_AGENT | + VIR_DOMAIN_REBOOT_HARD, -1); if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; @@ -1972,14 +2005,24 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) priv = vm->privateData; agentRequested = flags & VIR_DOMAIN_REBOOT_GUEST_AGENT; acpiRequested = flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN; + hardRequested = flags & VIR_DOMAIN_REBOOT_HARD; + + if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + /* Take hard reboot as the highest priority. + * This is for TDX which is not allowed to warm reboot. + */ + if (hardRequested) + ret = qemuDomainRebootMonitor(vm, isReboot, true); + + if (!ret) + goto cleanup; /* Prefer agent unless we were requested to not to. */ if (agentRequested || (!flags && priv->agent)) useAgent = true; - if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0) - goto cleanup; - agentForced = agentRequested && !acpiRequested; if (useAgent) ret = qemuDomainRebootAgent(vm, isReboot, agentForced); @@ -1992,7 +2035,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) */ if ((!useAgent) || (ret < 0 && (acpiRequested || !flags))) { - ret = qemuDomainRebootMonitor(vm, isReboot); + ret = qemuDomainRebootMonitor(vm, isReboot, false); } cleanup: @@ -2095,6 +2138,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, } qemuDomainSetFakeReboot(vm, false); + qemuDomainSetHardReboot(vm, false); if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f27f6653f5..9fa679e408 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -502,13 +502,98 @@ qemuProcessFakeReboot(void *opaque) virDomainObjEndAPI(&vm); } +static void +qemuProcessHardReboot(void *opaque) +{ + virDomainObj *vm = opaque; + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + unsigned int stopFlags = 0; + virObjectEvent *event = NULL; + virObjectEvent * event2 = NULL; + + VIR_DEBUG("Handle hard reboot: destroy phase"); + + virObjectLock(vm); + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, 0) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + virDomainObjEndJob(vm); + goto cleanup; + } + + if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) + stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; + + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED, + VIR_ASYNC_JOB_NONE, stopFlags); + virDomainAuditStop(vm, "destroyed"); + + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_DESTROYED); + + virObjectEventStateQueue(driver->domainEventState, event); + /* skip remove inactive domain from active list */ + virDomainObjEndJob(vm); + + VIR_DEBUG("Handle hard reboot: boot phase"); + + if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, + 0) < 0) { + qemuDomainRemoveInactive(driver, vm, 0, false); + goto cleanup; + } + + if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_START, + NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + 0) < 0) { + virDomainAuditStart(vm, "booted", false); + qemuDomainRemoveInactive(driver, vm, 0, false); + qemuProcessEndJob(vm); + goto cleanup; + } + + virDomainAuditStart(vm, "booted", true); + event2 = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_BOOTED); + virObjectEventStateQueue(driver->domainEventState, event2); + + qemuProcessEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); +} void qemuProcessShutdownOrReboot(virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; - if (priv->fakeReboot || + if (priv->hardReboot) { + g_autofree char *name = g_strdup_printf("hard reboot-%s", vm->def->name); + virThread th; + + virObjectRef(vm); + if (virThreadCreateFull(&th, + false, + qemuProcessHardReboot, + name, + false, + vm) < 0) { + VIR_ERROR(_("Failed to create hard reboot thread, killing domain")); + ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT)); + virObjectUnref(vm); + } + } else if (priv->fakeReboot || vm->def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART) { g_autofree char *name = g_strdup_printf("reboot-%s", vm->def->name); virThread th; @@ -5673,6 +5758,7 @@ qemuProcessInit(virQEMUDriver *driver, } else { vm->def->id = qemuDriverAllocateID(driver); qemuDomainSetFakeReboot(vm, false); + qemuDomainSetFakeReboot(vm, false); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 66f933dead..21864c92cb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5915,7 +5915,7 @@ static const vshCmdOptDef opts_shutdown[] = { {.name = "mode", .type = VSH_OT_STRING, .completer = virshDomainShutdownModeCompleter, - .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt") + .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt|hard") }, {.name = NULL} }; @@ -5952,6 +5952,8 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SHUTDOWN_SIGNAL; } else if (STREQ(mode, "paravirt")) { flags |= VIR_DOMAIN_SHUTDOWN_PARAVIRT; + } else if (STREQ(mode, "hard")) { + flags |= VIR_DOMAIN_SHUTDOWN_HARD; } else { vshError(ctl, _("Unknown mode %1$s value, expecting 'acpi', 'agent', 'initctl', 'signal', or 'paravirt'"), mode); @@ -5995,7 +5997,7 @@ static const vshCmdOptDef opts_reboot[] = { {.name = "mode", .type = VSH_OT_STRING, .completer = virshDomainShutdownModeCompleter, - .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt") + .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt|hard") }, {.name = NULL} }; @@ -6031,6 +6033,8 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_REBOOT_SIGNAL; } else if (STREQ(mode, "paravirt")) { flags |= VIR_DOMAIN_REBOOT_PARAVIRT; + } else if (STREQ(mode, "hard")) { + flags |= VIR_DOMAIN_REBOOT_HARD; } else { vshError(ctl, _("Unknown mode %1$s value, expecting 'acpi', 'agent', 'initctl', 'signal' or 'paravirt'"), mode); -- 2.34.1

-----Original Message----- From: Daniel P. Berrangé <berrange(a)redhat.com&gt; Subject: Re: [PATCH rfcv3 08/11] qemu: make hard reboot as the TDX default reboot mode On Mon, Nov 27, 2023 at 04:55:18PM +0800, Zhenzhong Duan wrote: > From: Chenyi Qiang <chenyi.qiang(a)intel.com&gt; > > Signed-off-by: Chenyi Qiang <chenyi.qiang(a)intel.com&gt; > --- > src/qemu/qemu_driver.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 86e8efbfcb..ba1bb4ecb1 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -2013,7 +2013,9 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) > /* Take hard reboot as the highest priority. > * This is for TDX which is not allowed to warm reboot. > */ > - if (hardRequested) > + if ((vm->def->sec && > + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_TDX) || > + hardRequested) > ret = qemuDomainRebootMonitor(vm, isReboot, true); > > if (!ret) > @@ -3617,7 +3619,12 @@ processGuestPanicEvent(virQEMUDriver *driver, > G_GNUC_FALLTHROUGH; > > case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART: > - qemuDomainSetFakeReboot(vm, true); > + if (vm->def->sec && > + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_TDX) { > + qemuDomainSetHardReboot(vm, true); > + } else { > + qemuDomainSetFakeReboot(vm, true); > + } > qemuProcessShutdownOrReboot(vm); I suspect we'll need todo this with SEV too, since IIUC it does not permit soft reboot either.

After hard reboot, domid is increased by 1 as a new domain. Hard reboot simulate TD-guest reboot by calling qemuProcessStop and qemuProcessStart which will release and recreate domain resource including domid.

Define origin_id to save domid and restore it when recreate domain. For persistent domain also need to save domid in newDef. Also add logic to keep same domid when libvirt restart. Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; --- src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 11 +++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 08e82c5380..8c33b335bd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4159,6 +4159,7 @@ void virDomainObjAssignDef(virDomainObj *domain, else virDomainDefFree(domain->newDef); domain->newDef = g_steal_pointer(def); + domain->newDef->origin_id = domain->def->id; return; } @@ -17874,6 +17875,9 @@ virDomainDefParseIDs(virDomainDef *def, return -1; } + /* Restore origin_id to support hard reboot */ + def->origin_id = def->id; + /* Extract domain name */ if (!(def->name = virXPathString("string(./name[1])", ctxt))) { virReportError(VIR_ERR_NO_NAME, NULL); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3b01850eb4..afd54a07eb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2957,6 +2957,7 @@ struct _virDomainVirtioOptions { struct _virDomainDef { int virtType; /* enum virDomainVirtType */ int id; + int origin_id; unsigned char uuid[VIR_UUID_BUFLEN]; unsigned char genid[VIR_UUID_BUFLEN]; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a9bba19852..6ec649a261 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5757,8 +5757,15 @@ qemuProcessInit(virQEMUDriver *driver, goto cleanup; } } else { - vm->def->id = qemuDriverAllocateID(driver); - qemuDomainSetFakeReboot(vm, false); + if (vm->def->origin_id > 0 && priv->hardReboot) + vm->def->id = vm->def->origin_id; + else + vm->def->id = qemuDriverAllocateID(driver); + vm->def->origin_id = vm->def->id; + if (vm->newDef) + vm->newDef->origin_id = vm->def->id; + + qemuDomainSetHardReboot(vm, false); qemuDomainSetFakeReboot(vm, false); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); -- 2.34.1