Re: [libvirt] [PATCH 3/4] conf: introduce sev element in domain

On Tue, Feb 27, 2018 at 05:15:30PM +0000, Daniel P. Berrangé wrote: > On Tue, Feb 27, 2018 at 11:07:25AM -0600, Brijesh Singh wrote: > > > > > > On 02/27/2018 05:10 AM, Daniel P. Berrangé wrote: > > > On Mon, Feb 26, 2018 at 11:53:35AM -0600, Brijesh Singh wrote: > > > > Secure Encrypted Virtualization (sev) element is used to provide the guest > > > > owners input parameters used for creating an encrypted VM using AMD SEV > > > > feature. SEV feature supports running encrypted VM under the control of > > > > KVM. Encrypted VMs have their pages (code and data) secured such that only > > > > the guest itself has access to the unencrypted version. Each encrypted VM > > > > is associated with a unique encryption key; if its data is accessed to a > > > > different entity using a different key the encrypted guests data will be > > > > incorrectly decrypted, leading to unintelligible data. > > > > > > > > QEMU >= 2.12 provides 'sev-guest' object which supports launching encrypted > > > > VMs. A typical command line > > > > > > > > # $QEMU ... \ > > > > -machine memory-encryption=sev0 \ > > > > -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 \ > > > > ... > > > > > > > > Signed-off-by: Brijesh Singh <brijesh.singh(a)amd.com&gt; > > > > --- > > > > docs/formatdomain.html.in | 71 +++++++++++++++++++++++++++++++++++++++++++ > > > > src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++++++++++ > > > > src/conf/domain_conf.h | 18 +++++++++++ > > > > src/qemu/qemu_command.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++ > > > > 4 files changed, 230 insertions(+) > > > > > > In general we'd expect to see additions to the test suite for any XML > > > changes. eg a qemuxml2xmltest and qemuxml2argvtest addition. > > > > > > > > > Sure, this is my first stab at libvirt and will look into getting familiar > > with test and add them in next round. > > > > > > > > > > > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > > > > index 6fd2189cd2f4..d18e3fb1d976 100644 > > > > --- a/docs/formatdomain.html.in > > > > +++ b/docs/formatdomain.html.in > > > > @@ -8195,6 +8195,77 @@ qemu-kvm -net nic,model=? /dev/null > > > > <p>Note: DEA/TDEA is synonymous with DES/TDES.</p> > > > > + <h3><a id="sev">Secure Encrypted Virtualization (SEV)</a></h3> > > > > + > > > > + <p> > > > > + The contents of the <code>sev</code> element is used to provide the > > > > + guest owners input used for creating an encrypted VM using the AMD > > > > + Secure Encrypted Virtualization (SEV) feature. > > > > + > > > > + SEV is an extension to the AMD-V architecture which supports running > > > > + encrypted virtual machine (VMs) under the control of KVM. Encrypted > > > > + VMs have their pages (code and data) secured such that only the guest > > > > + itself has access to the unencrypted version. Each encrypted VM is > > > > + associated with a unique encryption key; if its data is accessed to a > > > > + different entity using a different key the encrypted guests data will > > > > + be incorrectly decrypted, leading to unintelligible data. > > > > + </p> > > > > + <pre> > > > > +&lt;domain&gt; > > > > + ... > > > > + &lt;sev&gt; > > > > + &lt;policy&gt; 1 &lt;/policy&gt; > > > > + &lt;cbitpos&gt; 47 &lt;/cbitpos&gt; > > > > + &lt;reduced-phys-bits&gt; 5 &lt;/reduced-phys-bits&gt; > > > > + &lt;session&gt; ... &lt;/session&gt; > > > > + &lt;dh-cert&gt; ... &lt;/dh&gt; > > > > + &lt;/sev&gt; > > > > > > Minor nitpick - since this inheranted SEV specific, I think we could do > > > with having a generic top level element with a type=sev. eg > > > > > > <launch-security type="sev"> > > > <policy>...</policy> > > > <cbitpos>..</cbitpos> > > > ...etc... > > > </launch> > > > > > > then we can plug in custom data if other vendors invent competing > > > solutions to AMD's SEV. > > > > > > > I am okay with this, how about <memory-encryption> instead of > > <launch-security>, are you okay with it ? > > Memory encryption is a very specific feature. It occurs to me that there > could in future be other features that use launch time validation, that > are not memory encryption related. <launch-security> is IMHO still rather specific than generic, since we might need to enable features in the future, which might/might no rely on security, but add additional attributes to the launch validation, in which case I think going for something like <launch-control> or simply <launch> and having a structure similar to the one below:

By having the separate <sev> element you can make the sub-elements depend on this parent element, since you can't expect other vendors to favour <cbitpos> which add burden to the documentation to make it clear. Of course, the price you pay for this is a more complex XML structure. <launch> <security> <sev> <sev_specific_elements/> </sev> </security>

<other_security_unrelated_validation_options/> </launch>