On Mon, Jun 21, 2021 at 04:14:40AM +0000, Duan, Zhenzhong wrote: > > > > -----Original Message----- > > From: Pavel Hrdina <phrdina@xxxxxxxxxx> > > Sent: Friday, June 18, 2021 8:09 PM > > To: Duan, Zhenzhong <zhenzhong.duan@xxxxxxxxx> > > Cc: libvir-list@xxxxxxxxxx; Yamahata, Isaku <isaku.yamahata@xxxxxxxxx>; > > Tian, Jun J <jun.j.tian@xxxxxxxxx>; Qiang, Chenyi <chenyi.qiang@xxxxxxxxx> > > 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. Reusing <launchSecurity> is likely to be better for downstream consumers of libvirt too. So the extra code in libvirt is not a big deal. 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 :|
