Re: [PATCH v4 00/11] KVM SGX virtualization support (KVM part)

Kai Huang
2021-04-07T23:08:32Z

On Thu, 8 Apr 2021 08:49:24 +1200 Kai Huang wrote:
> Hi Paolo, Sean,
>
> Boris has merged x86 part patches to the tip/x86/sgx. This series is KVM part
> patches. Due to some code change in x86 part patches, two KVM patches need
> update so this is the new version. Please help to review. Thanks!
>
> Specifically, x86 patch (x86/sgx: Add helpers to expose ECREATE and EINIT to
> KVM) was changed to return -EINVAL directly w/o setting trapnr when
> access_ok()s fail on any user pointers, so KVM patches:
>
> KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions
> KVM: VMX: Add ENCLS[EINIT] handler to support SGX Launch Control (LC)
>
> were updated to handle this case.
>
> This seris is still based on tip/x86/sgx (which is based on 5.12-rc3), since it
> requires x86 patches to work. I tried to rebase them to latest kvm/queue, but
> found patch
>
> KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions
> KVM: x86: Add capability to grant VM access to privileged SGX aattribute
>
> have merge conflict, but the conflict is quite easy to resolve, so I didn't sent
> out the resolved version. Please let me know how would you like to proceed.

Hi Paolo, Sean,

Be more specifically, the first merge conflict is in patch (KVM: VMX: Add
emulation of SGX Launch Control LE hash MSRs) (sorry I made a mistake about
which patch above). The new added msr_ia32_sgxlepubkeyhash[4] to struct
vcpu_vmx{} conflicts with hyperv's hv_root_ept:

++<<<<<<< HEAD
+#if IS_ENABLED(CONFIG_HYPERV)
+ u64 hv_root_ept;
+#endif
++=======
+ /* SGX Launch Control public key hash */
+ u64 msr_ia32_sgxlepubkeyhash[4];
+ u64 ept_pointer;
++>>>>>>> ebe348a4bbc1 (KVM: VMX: Add emulation of SGX Launch Control LE hash
MSRs)

The second conflict is the last patch (KVM: x86: Add capability to grant VM
access to privileged SGX aattribute), due to new added
KVM_CAP_VM_COPY_ENC_CONTEXT_FROM capability conflicts with
KVM_CAP_SGX_ATTRIBUTE.

They are both easy to resolve.

For the next version, I think I can do:

1) grab SGX x86 part patches from tip/x86/sgx, and rebase all patches to
kvm/queue (basically rebase tip/x86/sgx to kvm/queue + new KVM patches), and
send out the rebased patches.
2) Still base on existing tip/x86/sgx, and let you to handle merge conflict.

Which way do you prefer? Do you have any other suggestions?

>
> Thank you all guys!
>
> Sean Christopherson (11):
> KVM: x86: Export kvm_mmu_gva_to_gpa_{read,write}() for SGX (VMX)
> KVM: x86: Define new #PF SGX error code bit
> KVM: x86: Add support for reverse CPUID lookup of scattered features
> KVM: x86: Add reverse-CPUID lookup support for scattered SGX features
> KVM: VMX: Add basic handling of VM-Exit from SGX enclave
> KVM: VMX: Frame in ENCLS handler for SGX virtualization
> KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions
> KVM: VMX: Add emulation of SGX Launch Control LE hash MSRs
> KVM: VMX: Add ENCLS[EINIT] handler to support SGX Launch Control (LC)
> KVM: VMX: Enable SGX virtualization for SGX1, SGX2 and LC
> KVM: x86: Add capability to grant VM access to privileged SGX
> attribute
>
> Documentation/virt/kvm/api.rst | 23 ++
> arch/x86/include/asm/kvm_host.h | 5 +
> arch/x86/include/asm/vmx.h | 1 +
> arch/x86/include/uapi/asm/vmx.h | 1 +
> arch/x86/kvm/Makefile | 2 +
> arch/x86/kvm/cpuid.c | 89 +++++-
> arch/x86/kvm/cpuid.h | 50 +++-
> arch/x86/kvm/vmx/nested.c | 28 +-
> arch/x86/kvm/vmx/nested.h | 5 +
> arch/x86/kvm/vmx/sgx.c | 500 ++++++++++++++++++++++++++++++++
> arch/x86/kvm/vmx/sgx.h | 34 +++
> arch/x86/kvm/vmx/vmcs12.c | 1 +
> arch/x86/kvm/vmx/vmcs12.h | 4 +-
> arch/x86/kvm/vmx/vmx.c | 109 ++++++-
> arch/x86/kvm/vmx/vmx.h | 2 +
> arch/x86/kvm/x86.c | 23 ++
> include/uapi/linux/kvm.h | 1 +
> 17 files changed, 855 insertions(+), 23 deletions(-)
> create mode 100644 arch/x86/kvm/vmx/sgx.c
> create mode 100644 arch/x86/kvm/vmx/sgx.h
>
> --
> 2.30.2
>