Re: [libvirt] [PATCH v6 0/9] x86: Secure Encrypted Virtualization (AMD)
On 05/29/2018 10:28 AM, Brijesh Singh wrote: ... 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 ... 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. I have not seen any other feedbacks, hence I will go with Erik's suggestions. I am reworking the patch to expose a new API "virNodeGetSEVParameters()". The API can be used to retrieve the SEV certificates etc. The signature looks like this: # define VIR_NODE_SEV_PDH "pdh" # define VIR_NODE_SEV_CERT_CHAIN"cert-chain" # define VIR_NODE_SEV_CBITPOS "cbipos" # define VIR_NODE_SEV_REDUCED_PHYS_BITS "reduced-phys-bits" int virNodeGetSEVParameters (virConnectPtr conn, virTypedParameterPtr params, int *nparams, unsigned int flags) If anyone have better idea then please let me know. thanks -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 0/9] x86: Secure Encrypted Virtualization (AMD)
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 ... 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 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 /* the value is same as what is obtained via virConnectGetDomainCapabilities() /* the value is same as what is obtained via virConnectGetDomainCapabilities() .. /* guest owners diffie-hellman key */ (optional) .. /* guest owners session blob */ (optional) .. /* guest policy */ (optional) Now that I'm looking at the design, why do we need to report , 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
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 > > > ... > > > > > > 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 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 > > > /* the value is same as what is obtained via > virConnectGetDomainCapabilities() > /* the value is same as what is > obtained via virConnectGetDomainCapabilities() >.. /* guest owners diffie-hellman key */ (optional) >.. /* guest owners session blob */ (optional) >.. /* guest policy */ (optional) > Now that I'm looking at the design, why do we need to report , 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 0/9] x86: Secure Encrypted Virtualization (AMD)
On Wed, 2018-05-23 at 16:18 -0500, Brijesh Singh wrote: > This patch series provides support for launching an encrypted guest using > AMD's new Secure Encrypted Virtualization (SEV) feature. Please don't CC random developers when you post patches. It's okay to include recipients that you know for sure wouldn't get the mail otherwise, or have explicitly asked to be CC'd, but all libvirt developers are subscribed to and follow libvir-list, which makes CC'ing them unnecessary. Some people like to be CC'd when respins of patches they've reviewed are posted, but that's not universally accepted as a best practice and some developers downright hate it. So, unless you're certain the other party will appreciate it, please don't do that. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 0/9] x86: Secure Encrypted Virtualization (AMD)
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 ... If 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 /* the value is same as what is obtained via virConnectGetDomainCapabilities() /* the value is same as what is obtained via virConnectGetDomainCapabilities() .. /* guest owners diffie-hellman key */ (optional) .. /* guest owners session blob */ (optional) .. /* guest policy */ (optional) 4. Libvirt generate the QEMU cli arg to enable the SEV feature, a typical args looks like this: # $QEMU .. -machine memory-encryption=sev0 \ -object sev-guest,id=sev0,dh-cert-file= 5. Libvirt generates lifecycle VIR_DOMAIN_EVENT_SUSPENDED_PAUSED event 6. mgmt tool gets the VIR_DOMAIN_EVENT_SUSPENDED_PAUSED and calls virDomainGetLaunchSecretInfo() to retrieve the measurement of encrypted memory. 7. (optional) mgmt tool can provide the measurement value to guest owner, which can validate the measurement and gives GO/NO-GO answer. If mgmt tool gets GO then it resumes the guest otherwise it calls destroy() to kill the guest. 8. mgmt tool resumes the guest TODO: * SEV guest require to use DMA apis for the virtio devices. In order to use the DMA apis the virtio devices must have this tag It is a bit unclear to me where these changes need to go. Do we need to modify the libvirt to automatically add these when SEV is enabled or we ask mgmt tool to make sure that it creates XML with right tag to enable the DMA APIs for virtio devices. I am looking for some suggestions. Using these patches we have succesfully booted and tested a guest both with and without SEV enabled. SEV Firmware API spec is available at: https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf Change since v5: * drop the seperate test patch and merge the code with other patches. * rename the xml from sev -> launch-security-sev * make policy field mandatory * address multiple feedback from previous reviews. Changes since v4: * add /dev/sev in shared device list Changes since v3: * rename QEMU_CAPS_SEV -> QEMU_CAPS_SEV_GUEST * update caps_2.12.0.x86_64.replies to include query-sev-capabilities data Changes since v2: * make cbitpos, policy and reduced-phys-bits as unsigned int * update virDomainGetLaunchSecurityInfo to accept virTypedParameterPtr *params instead of virTypedParameterPtr params. Changes since v1: * rename -> for domain * add more information about policy and other fields in domaincaps.html * split the domain_conf support in two patches * add virDomainGetLaunchInfo() to retrieve the SEV measurement * extend virsh command to show the domain's launch security information * add test cases to validate newly added element * fix issues reported with 'make check' and 'make syntax-check' The complete git tree is available at: https://github.com/codomania/libvirt/tree/v6 Brijesh Singh (9): qemu: provide support to query the SEV capability qemu: introduce SEV feature in hypervisor capabilities conf: introduce launch-security element in domain qemu/cgroup: add /dev/sev in shared devices list qemu: add support to launch SEV guest libvirt: add new public API to get launch security info remote: implement the remote protocol for launch security qemu: Add support to launch security info virsh: implement new command for launch security docs/drvqemu.html.in | 1 + docs/formatdomain.html.in | 115 ++ docs/formatdomaincaps.html.in | 40 +++ docs/schemas/domaincaps.rng| 20 docs/schemas/domaincommon.rng | 39 ++ include/libvirt/libvirt-domain.h | 17 +++ src/conf/domain_capabilities.c | 20 src/conf/domain_capabilities.h | 14 +++ src/conf/domain_conf.c | 133 + src/conf/domain_conf.h | 27 + src/driver-hypervisor.h| 7 ++ src/libvirt-domain.c | 48
