Re: [libvirt] [PATCH v6 0/9] x86: Secure Encrypted Virtualization (AMD)

On Wed, May 23, 2018 at 04:18:25PM -0500, Brijesh Singh wrote: > This patch series provides support for launching an encrypted guest using > AMD's new Secure Encrypted Virtualization (SEV) feature. > > SEV is an extension to the AMD-V architecture which supports running > multiple VMs under the control of a hypervisor. When enabled, SEV feature > allows the memory contents of a virtual machine (VM) to be transparently > encrypted with a key unique to the guest VM. > > At very high level the flow looks this: > > 1. mgmt tool calls virConnectGetDomainCapabilities. This returns an XML document > that includes the following > > <feature> > ... > <sev supported='yes'> > <cbitpos> </cbitpos> > <reduced-phys-bits> </reduced-phys-bits> > <pdh> </pdh> > <cert-chain> </cert-chain> > </feature>o It's sad that ^this didn't get more attention since Dan suggested a separate API to get the certificates, because I myself don't feel like having 2k/8k base64 strings reported in domain capabilities is a good idea, it should only report that the capability is supported with the given qemu and that the bit stuff which is hypervisor specific. For the certificates, which are not QEMU specific, rather those are platform dependent and static, we should introduce a virNodeGetSEVInfo (or something like that - I'm not good with names, but I think we struggle with this in general) getter for that which would work on top of virTypedParams so that it's extensible. Moreover, it would also be okay IMHO to feed the return data of this new API not only with the certs but also with the bit-related stuff, even though that's already obtained through capabilities. >

> If <sev> is provided then we indicate that hypervisor is capable of launching > SEV guest. > > 2. (optional) mgmt tool can provide the PDH and Cert-chain to guest owner in case > if guest owner wish to establish a secure connection with SEV firmware to > negotiate a key used for validating the measurement. > > 3. mgmt tool requests to start a guest calling virCreateXML(), passing VIR_DOMAIN_START_PAUSED. > The xml would include > > <launch-security type='sev'> > <cbitpos> </cbitpos> /* the value is same as what is obtained via virConnectGetDomainCapabilities() > <reduced-phys-bits> </reduced-phys-bits> /* the value is same as what is obtained via virConnectGetDomainCapabilities() > <dh-cert> .. </dh> /* guest owners diffie-hellman key */ (optional) > <session> ..</session> /* guest owners session blob */ (optional) > <policy> ..</policy> /* guest policy */ (optional) > </launch-security> Now that I'm looking at the design, why do we need to report <cbitpos>, <reduced-phys-bits> in the domain XML if that is platform specific, it's static and we already report it withing capabilities? If it can't be used for a per-guest configuration, i.e. it can change, I don't think we should expose the same information on multiple places.

Still, does anyone have another idea for an improvement? Erik