Re: [RFC PATCH 3/7] conf: introduce TrustDomain element in domain
On Mon, Jun 21, 2021 at 04:14:40AM +, Duan, Zhenzhong wrote: > > > > -Original Message- > > From: Pavel Hrdina > > Sent: Friday, June 18, 2021 8:09 PM > > To: Duan, Zhenzhong > > Cc: libvir-list@redhat.com; Yamahata, Isaku ; > > Tian, Jun J ; Qiang, Chenyi > > 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: > > > > > > > > >0x0001 > > > > > > > Any reason why you are adding a new element that basically copies exactly > > what 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: > > > > > > 0x0001 > > > > > > We would have to reorganize the 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 : > > 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 if you > think it's better. Reusing 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 :|
Re: [RFC PATCH 3/7] conf: introduce TrustDomain element in domain
On Mon, Jun 21, 2021 at 04:14:40AM +, Duan, Zhenzhong wrote: > > > > -Original Message- > > From: Pavel Hrdina > > Sent: Friday, June 18, 2021 8:09 PM > > To: Duan, Zhenzhong > > Cc: libvir-list@redhat.com; Yamahata, Isaku ; > > Tian, Jun J ; Qiang, Chenyi > > 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: > > > > > > > > >0x0001 > > > > > > > Any reason why you are adding a new element that basically copies exactly > > what 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: > > > > > > 0x0001 > > > > > > We would have to reorganize the 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 element as well so we would definitely prefer reusing element. > vs. Using : > > 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 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 signature.asc Description: PGP signature
RE: [RFC PATCH 3/7] conf: introduce TrustDomain element in domain
> -Original Message- > From: Pavel Hrdina > Sent: Friday, June 18, 2021 8:09 PM > To: Duan, Zhenzhong > Cc: libvir-list@redhat.com; Yamahata, Isaku ; > Tian, Jun J ; Qiang, Chenyi > 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: > > > > > >0x0001 > > > > Any reason why you are adding a new element that basically copies exactly > what 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: > > > 0x0001 > > > We would have to reorganize the 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 : 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 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
RE: [RFC PATCH 3/7] conf: introduce TrustDomain element in domain
> -Original Message- > From: Peter Krempa > Sent: Friday, June 18, 2021 7:17 PM > To: Duan, Zhenzhong > Cc: libvir-list@redhat.com; Yamahata, Isaku ; > Tian, Jun J ; Qiang, Chenyi > Subject: Re: [RFC PATCH 3/7] conf: introduce TrustDomain element in > domain > > On Fri, Jun 18, 2021 at 16:50:48 +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: > > > > > >0x0001 > > > > > > Signed-off-by: Zhenzhong Duan > > --- > > 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 ++ > > .../genericxml2xmlindata/trust-domain-tdx.xml | 21 + > > tests/genericxml2xmltest.c| 1 + > > 7 files changed, 147 insertions(+) > > create mode 100644 tests/genericxml2xmlindata/trust-domain-tdx.xml > > > > diff --git a/docs/schemas/domaincommon.rng > > b/docs/schemas/domaincommon.rng index 5ea14b6dbf..2b39a01e84 > 100644 > > --- a/docs/schemas/domaincommon.rng > > +++ b/docs/schemas/domaincommon.rng > > @@ -89,6 +89,9 @@ > > > > > > > > + > > + > > All our RNG schema entries ... > > > + > > > > > > > > @@ -518,6 +521,19 @@ > > > > > > > > + > > + > > + > > +tdx > > + > > + > > + > > + > > + > > + > > + > > + > > + > >
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: > > >0x0001 > Any reason why you are adding a new element that basically copies exactly what 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: 0x0001 We would have to reorganize the 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 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. Pavel > Signed-off-by: Zhenzhong Duan > --- > 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 ++ > .../genericxml2xmlindata/trust-domain-tdx.xml | 21 + > tests/genericxml2xmltest.c| 1 + > 7 files changed, 147 insertions(+) > create mode 100644 tests/genericxml2xmlindata/trust-domain-tdx.xml > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 5ea14b6dbf..2b39a01e84 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -89,6 +89,9 @@ > > > > + > + > + > > > > @@ -518,6 +521,19 @@ > > > > + > + > + > +tdx > + > + > + > + > + > + > + > + > + > 2.25.1 > signature.asc Description: PGP signature
Re: [RFC PATCH 3/7] conf: introduce TrustDomain element in domain
On Fri, Jun 18, 2021 at 16:50:48 +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: > > >0x0001 > > > Signed-off-by: Zhenzhong Duan > --- > 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 ++ > .../genericxml2xmlindata/trust-domain-tdx.xml | 21 + > tests/genericxml2xmltest.c| 1 + > 7 files changed, 147 insertions(+) > create mode 100644 tests/genericxml2xmlindata/trust-domain-tdx.xml > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 5ea14b6dbf..2b39a01e84 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -89,6 +89,9 @@ > > > > + > + All our RNG schema entries ... > + > > > > @@ -518,6 +521,19 @@ > > > > + > + > + > +tdx > + > + > + > + > + > + > + > + > + >
[RFC PATCH 3/7] conf: introduce TrustDomain element in domain
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: 0x0001 Signed-off-by: Zhenzhong Duan --- 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 ++ .../genericxml2xmlindata/trust-domain-tdx.xml | 21 + tests/genericxml2xmltest.c| 1 + 7 files changed, 147 insertions(+) create mode 100644 tests/genericxml2xmlindata/trust-domain-tdx.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5ea14b6dbf..2b39a01e84 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -89,6 +89,9 @@ + + + @@ -518,6 +521,19 @@ + + + +tdx + + + + + + + + +
