RE: [PATCH rfcv4 05/13] conf: add tdx as launch security type

-----Original Message----- From: Daniel P. Berrangé <berrange(a)redhat.com&gt; Subject: Re: [PATCH rfcv4 05/13] conf: add tdx as launch security type On Fri, May 24, 2024 at 02:21:20PM +0800, Zhenzhong Duan wrote: > 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 set to enable TDX debug, bit 28 set to enable > sept-ve-disable, other bits are reserved currently. mrConfigId, mrOwner > and mrOwnerConfig are base64 encoded SHA384 digest. > > For example: > > <launchSecurity type='tdx'> > <policy>0x10000001</policy> > <mrConfigId>xxx</mrConfigId> > <mrOwner>xxx</mrOwner> > <mrOwnerConfig>xxx</mrOwnerConfig> > </launchSecurity> > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; > --- > src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 9 +++++++ > src/conf/schemas/domaincommon.rng | 29 +++++++++++++++++++++ > src/conf/virconftypes.h | 2 ++ > src/qemu/qemu_command.c | 2 ++ > src/qemu/qemu_firmware.c | 1 + > src/qemu/qemu_namespace.c | 1 + > src/qemu/qemu_process.c | 1 + > src/qemu/qemu_validate.c | 1 + > 9 files changed, 88 insertions(+) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index a0912062ff..c557da0c65 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -13649,6 +13654,24 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, > } > > > +static int > +virDomainTDXDefParseXML(virDomainTDXDef *def, > + xmlXPathContextPtr ctxt) > +{ > + if (virXPathULongLongBase("string(./policy)", ctxt, 16, &def->policy) < 0) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("failed to get launch security policy for launch security type TDX")); > + return -1; > + } This makes the 'policy' attribute mandatory, but QEMU is quite happy with it being unset, so we should not require this in libvirt either.

> + > + def->mrconfigid = virXPathString("string(./mrConfigId)", ctxt); > + def->mrowner = virXPathString("string(./mrOwner)", ctxt); > + def->mrownerconfig = virXPathString("string(./mrOwnerConfig)", ctxt); > + > + return 0; > +} > diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng > index d84e030255..f6e1782b33 100644 > --- a/src/conf/schemas/domaincommon.rng > +++ b/src/conf/schemas/domaincommon.rng > @@ -520,6 +520,9 @@ > <value>s390-pv</value> > </attribute> > </group> > + <group> > + <ref name="launchSecurityTDX"/> > + </group> > </choice> > </element> > </define> > @@ -565,6 +568,32 @@ > </interleave> > </define> > > + <define name="launchSecurityTDX"> > + <attribute name="type"> > + <value>tdx</value> > + </attribute> > + <interleave> > + <element name="policy"> > + <ref name="hexuint"/> > + </element> This should be marked <optional> too. > + <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> > + </define> > + > <!-- > Enable or disable perf events for the domain. For each > of the events the following rules apply: 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 :|