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

[Pavel Hrdina]

On Mon, Jun 21, 2021 at 04:14:40AM +0000, Duan, Zhenzhong wrote:
> 
> 
> > -----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 +-

This is not an issue and most likely some of these changes would be
required with new <TrustDomain> element as well so we would definitely
prefer reusing <launchSecurity> element.

> 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.

v1 series: https://listman.redhat.com/archives/libvir-list/2021-May/msg00570.html

v2 series: https://listman.redhat.com/archives/libvir-list/2021-June/msg00583.html

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210621/6101ee64/attachment-0001.sig>