On 05/28/2018 05:06 AM, Erik Skultety wrote:
> 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.
>
>
>>
IIRC, Dan asked other folks to comment their preferences on APIs vs xml.
I don't remember seeing any strong preferences hence I didn't rework on
that code. But if majority of folks think that we should do APIs then I
can certainly rework it in next patch.
>> 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.
>
We had some discussion about this on QEMU mailing list. To make SEV
launch migration safe, we require user to provide the cbitpos and
reduced-phys-bit in domain XML as an input to the launch flow. The
launch flow should not make a platform dependent call to obtain these
value. If user does not know the cbitpos for its given platform then it
can obtain it through the domain capabilities.
> Still, does anyone have another idea for an improvement?
>
> Erik
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list