>-----Original Message-----
>From: Daniel P. Berrangé <berra...@redhat.com>
>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.d...@intel.com>
>> ---
>> 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.
Yes, but I am trying to align with SEV which has same issue.
So aligning with SEV vs. making TDX's 'policy' optional, you prefer the 2nd?
Pls confirm.
Thanks
Zhenzhong
>
>> +
>> + 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 :|
