On Tue, 2023-04-04 at 17:59 -0700, Sean Christopherson wrote:
> *** WARNING *** ABI breakage.
>
> Stop adjusting the guest's CPUID info for the allowed XFRM (a.k.a. XCR0)
> for SGX enclaves. Past me didn't understand the roles and responsibilities
> between userspace and KVM with respect to CPUID leafs, i.e. I thought I was
> being helpful by having KVM adjust the entries.
>
> This is clearly an ABI breakage, but QEMU (tries to) do the right thing,
> and AFAIK no other VMMs support SGX (yet), so I'm hoping we can excise the
> ugly before userspace starts depending on the bad behavior.
>
> Compile tested only (don't have an SGX system these days).
I'll look into this, and at the meantime ...
>
> Note, QEMU commit 301e90675c ("target/i386: Enable support for XSAVES
> based features") completely broke SGX by using allowed XSS instead of
> XCR0, and no one has complained. That gives me hope that this one will
> go through as well.
...
Actually we got complain around half year ago:
https://github.com/gramineproject/gramine/issues/955#issuecomment-1272829510
>
> I belive the QEMU fix is below. I'll post a patch at some point unless
> someone wants to do the dirty work and claim the patch as their own.
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 6576287e5b..f083ff4335 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5718,8 +5718,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> } else {
> *eax &= env->features[FEAT_SGX_12_1_EAX];
> *ebx &= 0; /* ebx reserve */
> - *ecx &= env->features[FEAT_XSAVE_XSS_LO];
> - *edx &= env->features[FEAT_XSAVE_XSS_HI];
> + *ecx &= env->features[FEAT_XSAVE_XCR0_LO];
> + *edx &= env->features[FEAT_XSAVE_XCR0_HI];
>
> /* FP and SSE are always allowed regardless of XSAVE/XCR0. */
> *ecx |= XSTATE_FP_MASK | XSTATE_SSE_MASK;
And since then Yang posted a patch to Qemu mailing list to fix:
https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg04990.html
I thought it had been merged, but it seems it hasn't :)
>
> Sean Christopherson (3):
> KVM: VMX: Don't rely _only_ on CPUID to enforce XCR0 restrictions for
> ECREATE
> KVM: x86: Don't adjust guest's CPUID.0x12.1 (allowed SGX enclave XFRM)
> KVM: x86: Open code supported XCR0 calculation in
> kvm_vcpu_after_set_cpuid()
>
> arch/x86/kvm/cpuid.c | 43 ++++++++++--------------------------------
> arch/x86/kvm/vmx/sgx.c | 3 ++-
> 2 files changed, 12 insertions(+), 34 deletions(-)
>
>
> base-commit: 27d6845d258b67f4eb3debe062b7dacc67e0c393
> --
> 2.40.0.348.gf938b09366-goog
>
