Re: [Part2 PATCH v9 00/38] x86: Secure Encrypted Virtualization (AMD)

2018-01-11 Thread Paolo Bonzini
On 05/12/2017 02:04, Brijesh Singh wrote:
> This part of Secure Encrypted Virtualization (SEV) patch series focuses on KVM
> changes required to create and manage SEV guests.
> 
> SEV is an extension to the AMD-V architecture which supports running encrypted
> virtual machine (VMs) under the control of a hypervisor. Encrypted VMs have 
> their
> pages (code and data) secured such that only the guest itself has access to
> 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
> guest's data will be incorrectly decrypted, leading to unintelligible data.
> This security model ensures that hypervisor will no longer able to inspect or
> alter any guest code or data.
> 
> The key management of this feature is handled by a separate processor known as
> the AMD Secure Processor (AMD-SP) which is present on AMD SOCs. The SEV Key
> Management Specification (see below) provides a set of commands which can be
> used by hypervisor to load virtual machine keys through the AMD-SP driver.
> 
> The patch series adds a new ioctl in KVM driver (KVM_MEMORY_ENCRYPT_OP). The
> ioctl will be used by qemu to issue SEV guest-specific commands defined in Key
> Management Specification.
> 
> The following links provide additional details:
> 
> AMD Memory Encryption white paper:
> http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf
> 
> AMD64 Architecture Programmer's Manual:
> http://support.amd.com/TechDocs/24593.pdf
> SME is section 7.10
> SEV is section 15.34
> 
> SEV Key Management:
> http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf
> 
> KVM Forum Presentation:
> http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf
> 
> SEV Guest BIOS support:
>   SEV support has been add to EDKII/OVMF BIOS
>   https://github.com/tianocore/edk2

Merged!  Thanks,

Paolo


Re: [Part2 PATCH v9 00/38] x86: Secure Encrypted Virtualization (AMD)

2017-12-21 Thread Brijesh Singh


On 12/21/17 9:51 AM, Brijesh Singh wrote:
>
> On 12/21/17 7:06 AM, Paolo Bonzini wrote:
> 
>
> Hi Paolo,
>  
>
>> Hi Brijesh,
>>
>> I have a couple comments:
>>
>> 1) how is MSR_AMD64_SEV's value passed to the guest, and where is it in
>> the manual?
> It is a non interceptable read-only MSR set by the HW when SEV feature
> is enabled in VMRUN instruction.

I just checked both PPR and Family 17 manual and it seems both are still
missing this MSR definition. I will ping doc team to get it updated. thanks
>> 2) ECX should be 0 in the guest's 0x8000_001f leaf, because we don't
>> support nested SEV guests.  Likewise, EAX bit 2 should be 0 since you
>> don't emulate the page flush MSR.
> IIRC, I do clear both EAX Page_Flush and nested virtualization case from
> Qemu SEV feature is enabled.
>> Both can be fixed on top (and I can do the second myself of course), so
>> there should be no need for a v10.  
> Thanks
>



Re: [Part2 PATCH v9 00/38] x86: Secure Encrypted Virtualization (AMD)

2017-12-21 Thread Brijesh Singh


On 12/21/17 7:06 AM, Paolo Bonzini wrote:


Hi Paolo,
 

> Hi Brijesh,
>
> I have a couple comments:
>
> 1) how is MSR_AMD64_SEV's value passed to the guest, and where is it in
> the manual?

It is a non interceptable read-only MSR set by the HW when SEV feature
is enabled in VMRUN instruction.
>
> 2) ECX should be 0 in the guest's 0x8000_001f leaf, because we don't
> support nested SEV guests.  Likewise, EAX bit 2 should be 0 since you
> don't emulate the page flush MSR.

IIRC, I do clear both EAX Page_Flush and nested virtualization case from
Qemu SEV feature is enabled.
> Both can be fixed on top (and I can do the second myself of course), so
> there should be no need for a v10.  

Thanks



Re: [Part2 PATCH v9 00/38] x86: Secure Encrypted Virtualization (AMD)

2017-12-21 Thread Paolo Bonzini
On 05/12/2017 02:04, Brijesh Singh wrote:
> This part of Secure Encrypted Virtualization (SEV) patch series focuses on KVM
> changes required to create and manage SEV guests.
> 
> SEV is an extension to the AMD-V architecture which supports running encrypted
> virtual machine (VMs) under the control of a hypervisor. Encrypted VMs have 
> their
> pages (code and data) secured such that only the guest itself has access to
> 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
> guest's data will be incorrectly decrypted, leading to unintelligible data.
> This security model ensures that hypervisor will no longer able to inspect or
> alter any guest code or data.
> 
> The key management of this feature is handled by a separate processor known as
> the AMD Secure Processor (AMD-SP) which is present on AMD SOCs. The SEV Key
> Management Specification (see below) provides a set of commands which can be
> used by hypervisor to load virtual machine keys through the AMD-SP driver.
> 
> The patch series adds a new ioctl in KVM driver (KVM_MEMORY_ENCRYPT_OP). The
> ioctl will be used by qemu to issue SEV guest-specific commands defined in Key
> Management Specification.

Hi Brijesh,

I have a couple comments:

1) how is MSR_AMD64_SEV's value passed to the guest, and where is it in
the manual?

2) ECX should be 0 in the guest's 0x8000_001f leaf, because we don't
support nested SEV guests.  Likewise, EAX bit 2 should be 0 since you
don't emulate the page flush MSR.

Both can be fixed on top (and I can do the second myself of course), so
there should be no need for a v10.  But MSR_AMD64_SEV is leaving me
quite puzzled.

Thanks,

Paolo

> The following links provide additional details:
> 
> AMD Memory Encryption white paper:
> http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf
> 
> AMD64 Architecture Programmer's Manual:
> http://support.amd.com/TechDocs/24593.pdf
> SME is section 7.10
> SEV is section 15.34
> 
> SEV Key Management:
> http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf
> 
> KVM Forum Presentation:
> http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf
> 
> SEV Guest BIOS support:
>   SEV support has been add to EDKII/OVMF BIOS
>   https://github.com/tianocore/edk2
> 
> --
> 
> The series applies on kvm/next commit : 4fbd8d194f06 (Linux 4.15-rc1)
> 
> Complete tree is available at:
> repo: https://github.com/codomania/kvm.git
> branch: sev-v9-p2
> 
> TODO:
> * Add SEV guest migration command support
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Paolo Bonzini 
> Cc: "Radim Krčmář" 
> Cc: Joerg Roedel 
> Cc: Borislav Petkov 
> Cc: Tom Lendacky 
> Cc: Herbert Xu 
> Cc: David S. Miller 
> Cc: Gary Hook 
> Cc: x...@kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-crypto@vger.kernel.org
> 
> Changes since v8:
>  * Rebase the series to kvm/next branch
>  * Update SEV asid allocation to limit the ASID between SEV_MIN_ASID to 
> SEV_MAX_ASID
>(EPYC BIOS provide option to change the SEV_MIN_ASID -- which can be used 
> to
>limit the number of SEV-enable guest)
> 
> Changes since v7:
>  * Rebase the series to kvm/next branch
>  * move the FW error enum definition in include/uapi/linux/psp-sev.h so that
>both userspace and kernel can share it.
>  * (ccp) drop cmd_buf arg from sev_platform_init()
>  * (ccp) apply some cleanup/fixup from Boris
>  * (ccp) add some comments in FACTORY_RESET command handling
>  * (kvm) some fixup/cleanup from Boris
>  * (kvm) acquire the kvm->lock when modifying the sev->regions_list
> 
> Changes since v6:
>  * (ccp): Extend psp_device structure to track the FW INIT and SHUTDOWN 
> states.
>  * (ccp): Init and Uninit SEV FW during module load/unload
>  * (ccp): Avoid repeated k*alloc() for init and status command buffer
>  * (kvm): Rework DBG command to fix the compilation warning seen with gcc7.x
>  * (kvm): Convert the SEV doc in rst format
> 
> Changes since v5:
>  * split the PSP driver support into multiple patches
>  * multiple improvements from Boris
>  * remove mem_enc_enabled() ops
> 
> Changes since v4:
>  * Fixes to address kbuild robot errors
>  * Add 'sev' module params to allow enable/disable SEV feature
>  * Update documentation
>  * Multiple fixes to address v4 feedbacks
>  * Some coding style changes to address checkpatch reports
> 
> Changes since v3:
>  * Re-design the PSP interface support patch
>  * Rename the ioctls based on the feedbacks
>  * Improve documentation
>  * Fix i386 build issues
>  * Add