[RFC PATCH 3/7] conf: introduce TrustDomain element in domain

[Duan, Zhenzhong]

> -----Original Message-----
> From: Pavel Hrdina <phrdina at redhat.com>
> Sent: Friday, June 18, 2021 8:09 PM
> To: Duan, Zhenzhong <zhenzhong.duan at intel.com>
> Cc: libvir-list at redhat.com; Yamahata, Isaku <isaku.yamahata at intel.com>;
> Tian, Jun J <jun.j.tian at intel.com>; Qiang, Chenyi <chenyi.qiang at intel.com>
> Subject: Re: [RFC PATCH 3/7] conf: introduce TrustDomain element in
> domain
> 
> On Fri, Jun 18, 2021 at 04:50:48PM +0800, Zhenzhong Duan wrote:
> > The TrustDomain element can be used to define the security model to
> > use when launching a domain. Only type 'tdx' is supported currently.
> >
> > 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' in TrustDomain. In 'policy', bit 0
> > is used to enable TDX debug, other bits are reserved currently.
> >
> > For example:
> >
> >  <TrustDomain type='tdx'>
> >    <policy>0x0001</policy>
> >  </TrustDomain>
> 
> Any reason why you are adding a new element that basically copies exactly
> what <launchSecurity> is doing?
> 
> In libvirt it will essentially use `confidential-guest-support` which is used for
> launchSecurity so there is no need to duplicate the code and the XML
> element.
> 
> It could look like this:
> 
>   <launchSecurity type='tdx'>
>     <policy>0x0001</policy>
>   </launchSecurity>
> 
> We would have to reorganize the <launchSecurity> documentation a little bit
> but otherwise there is nothing blocking us to have single element to specify
> different type of encrypted/confidential/secure/... VMs.

We had ever made a patch working as you suggested. It has advantage of only
using one element for all. The main reason I chose the other way is because this way 
having quite less code change, as it avoid many mux and branch code. See below:

Using < launchSecurity>:

docs/schemas/domaincommon.rng                 |  90 +++++----
 src/conf/domain_conf.c                        | 182 +++++++++++++++---
 src/conf/domain_conf.h                        |  19 +-
 src/conf/virconftypes.h                       |   6 +
 src/qemu/qemu_cgroup.c                        |   3 +-
 src/qemu/qemu_command.c                       |   5 +-
 src/qemu/qemu_driver.c                        |   4 +-
 src/qemu/qemu_firmware.c                      |   5 +-
 src/qemu/qemu_namespace.c                     |   4 +-
 src/qemu/qemu_process.c                       |  12 +-
 src/qemu/qemu_validate.c                      |  30 ++-
 src/security/security_dac.c                   |   4 +-

vs. Using <TrustDomain>:

docs/schemas/domaincommon.rng                 | 16 ++++
 src/conf/domain_conf.c                        | 84 +++++++++++++++++++
 src/conf/domain_conf.h                        | 15 ++++
 src/conf/virconftypes.h                       |  2 +
 src/qemu/qemu_validate.c                      |  8 ++

I'm also irresolute on which to choose, I'll use <launchSecurity> if you think it's better.

> We already have patches for similar feature for s390 which will also reuse this
>  element and in the future any other CPU architecture can reuse it.

Hmm, I didn’t find those patches for s390, aren't they upstream yet? Appreciate if you
could show me a link for reference.

Thanks
Zhenzhong