Message ID | 20230405234556.696927-3-seanjc@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp661556vqo; Wed, 5 Apr 2023 16:52:19 -0700 (PDT) X-Google-Smtp-Source: AKy350aqTLz/940KBRfaytm5NeWKpgTlFWKmm9lL/RSsOGIgXTYns4dg4WQuFj2mhWObR4xy65yf X-Received: by 2002:a05:6a20:671d:b0:d9:1ec8:e9bb with SMTP id q29-20020a056a20671d00b000d91ec8e9bbmr957953pzh.28.1680738739136; Wed, 05 Apr 2023 16:52:19 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680738739; cv=none; d=google.com; s=arc-20160816; b=mK8AMs8Pa0DwDFb5yj8ZZhQNP88QMrc+D/VC5iilQuC7qaLPTbQysr5OjESpGMWero tqusejAab0nGWnoNIVhscm6CrL+nQ9xM5s16BFZE2niPViFcgd/65Co7BkYzb7V7i4Nx F8Kib+0WFQYIli+dxbXigCkbDT81HieXLn3f8/lJUjAffWFx8FugIiFSiObrm2hKzfBG 6SNJPi0jW9wB4MC882/mn/CJEdxEUWHPbilZEdFsE+pEQs+t35Hlx9aRgJK6kC6KliGd 2NixHdBamGZEw9fmczi4CYY+czxsfH1xObpMjYZTPHNu9ZUd0GkQDmnWc3MdN1s3LRol WaZQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:reply-to:dkim-signature; bh=0eLsdRSWRS14XpdXI5Oe0msGImkWJM9UiPZLdHxJ9wo=; b=ZLd3JoRiw5Ynxa9ARxnhMRBjg35oH0568aXrBYc/zxFm9SjUPLpov5VZ9si8ZEzug/ WPl/uM3UkWbQ0E6D+GgCDKX7J0dIw1/Waf5JhNtFOi2JXxZGxmFtBX/44EYC9zAyUcOH 8V9LeqkQ6QmRQ+VRSVuL7e8gONu5RmIn8ekcOWdtiZqsYJXuCmspG/j6dYnvplu10GWm iHj7S9UNaImq1Z2gQlcorouZGqtnxYOGUVjIhzQ63eDGSIPReeTYxv0k6UHYarFIvECy +7l5XmyW3YnLMm79l72M07E7n2QD2Vrc+kwFAjbUWFEdF1PJcp74X7E7o+UsrtlQag1I hWxg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=ZzdivL5R; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b12-20020aa7950c000000b0058003199fbbsi13885072pfp.63.2023.04.05.16.52.06; Wed, 05 Apr 2023 16:52:19 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=ZzdivL5R; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234033AbjDEXqL (ORCPT <rfc822;a1648639935@gmail.com> + 99 others); Wed, 5 Apr 2023 19:46:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36810 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233678AbjDEXqG (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 5 Apr 2023 19:46:06 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9332A76A7 for <linux-kernel@vger.kernel.org>; Wed, 5 Apr 2023 16:46:03 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id y144-20020a253296000000b00b69ce0e6f2dso37261324yby.18 for <linux-kernel@vger.kernel.org>; Wed, 05 Apr 2023 16:46:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680738362; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=0eLsdRSWRS14XpdXI5Oe0msGImkWJM9UiPZLdHxJ9wo=; b=ZzdivL5RkQKVveq4bXKtR9D8nwwhzP80YxQP/yYQDaIHyF6AVChFfUxn0gCL6GEd4r wl6WXiTj/vpKn0EDm31ZR6Ob/4bXPB2JsoGngpHGCl9nkdU2cKIzeRI1SBL3Y3y9id71 Ba+DE2lW3kItrlyVzDFtsKmbp4bdsroFLmMCtfjAG92KcAIjJ1XvcfAGcAFx+cHKGiE1 99z7Ad2NSoslbBS6JIHPXeEZqECo7mfVPVVYwK3TDjh2U3sJSAjuzv+sRN31/3v233B7 6y1F7FKudKtQJpJQ/SjeKY0ro6NhQBdPgAtm0DfOHnaRZ6hENZwDxaocyuk661beWb4P yyNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680738362; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=0eLsdRSWRS14XpdXI5Oe0msGImkWJM9UiPZLdHxJ9wo=; b=ULe1JxiIWawdB37pv49tl3a+wmNml3NtlG81x2+KHfGTGty2QGyHZIhS6g/0WMsQx1 NTj/rcQNBzSWTqXJmzQCW1cCGg1wmOAr1LGqVALwkPhVKIIafAVeCatme77tN9ovM5rA QUsM5ScCeeH2TRLipW45jCTIKcFpTyYKk8Z9RZ66TyTITp+x5eFnE6U2EeqEWGKiz3yr KcpDwbquvf6tL2NQ/BtFciv0zqYiz6c6SfaF0S3gdlQNH50QFfMWbyF2MgqrhkA78hCA n3Eb0vFAeLKw81AggxbDxLPANC8cFZXAp+iPGO1bQC2TW63SzSZbut5naLEpAhw08ioy zZOQ== X-Gm-Message-State: AAQBX9cRyOv/anpwfWdpOY0ILn1X+efPf4LFUUQXstLaGvCc+iMpokvw pxAEYM8zondgLrGAQnEkyeBzCKug2DA= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a05:690c:714:b0:54c:88:64a1 with SMTP id bs20-20020a05690c071400b0054c008864a1mr1349340ywb.0.1680738362747; Wed, 05 Apr 2023 16:46:02 -0700 (PDT) Reply-To: Sean Christopherson <seanjc@google.com> Date: Wed, 5 Apr 2023 16:45:56 -0700 In-Reply-To: <20230405234556.696927-1-seanjc@google.com> Mime-Version: 1.0 References: <20230405234556.696927-1-seanjc@google.com> X-Mailer: git-send-email 2.40.0.348.gf938b09366-goog Message-ID: <20230405234556.696927-3-seanjc@google.com> Subject: [PATCH 2/2] KVM: VMX: Inject #GP, not #UD, if SGX2 ENCLS leafs are unsupported From: Sean Christopherson <seanjc@google.com> To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Binbin Wu <binbin.wu@linux.intel.com>, Kai Huang <kai.huang@intel.com> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.7 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1762382304141787077?= X-GMAIL-MSGID: =?utf-8?q?1762382304141787077?= |
Series |
KVM: VMX: Fixes for faults on ENCLS emulation
|
|
Commit Message
Sean Christopherson
April 5, 2023, 11:45 p.m. UTC
Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD.
SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a
"soft" disable, e.g. if software disables machine check reporting, i.e.
having SGX but not SGX1 is effectively "SGX completely unsupported" and
and thus #UDs.
Fixes: 9798adbc04cf ("KVM: VMX: Frame in ENCLS handler for SGX virtualization")
Cc: Binbin Wu <binbin.wu@linux.intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/vmx/sgx.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
Comments
On Wed, 2023-04-05 at 16:45 -0700, Sean Christopherson wrote: > Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD. > SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a > "soft" disable, e.g. if software disables machine check reporting, i.e. > having SGX but not SGX1 is effectively "SGX completely unsupported" and > and thus #UDs. If I recall correctly, this is an erratum which can clear SGX1 in CPUID while the SGX flag is still in CPUID? But I am not sure whether this part is relevant to this patch? Because SDM already says ENCLS causes #UD if SGX1 isn't present. This patch changes "unsupported leaf" from causing #UD to causing #GP, which is also documented in SDM. > > Fixes: 9798adbc04cf ("KVM: VMX: Frame in ENCLS handler for SGX virtualization") > Cc: Binbin Wu <binbin.wu@linux.intel.com> > Cc: Kai Huang <kai.huang@intel.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/vmx/sgx.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c > index f881f6ff6408..1c092ab89c33 100644 > --- a/arch/x86/kvm/vmx/sgx.c > +++ b/arch/x86/kvm/vmx/sgx.c > @@ -350,11 +350,12 @@ static int handle_encls_einit(struct kvm_vcpu *vcpu) > > static inline bool encls_leaf_enabled_in_guest(struct kvm_vcpu *vcpu, u32 leaf) > { > - if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX)) > - return false; > - > + /* > + * ENCLS #UDs if SGX1 isn't supported, i.e. this point will be reached Why #UDs instead of #UD? Is #UD a verb? > + * if and only if the SGX1 leafs are enabled. > + */ Is it better to move "ENCLS #UDs if SGX1 isn't supported" part to ... > if (leaf >= ECREATE && leaf <= ETRACK) > - return guest_cpuid_has(vcpu, X86_FEATURE_SGX1); > + return true; > > if (leaf >= EAUG && leaf <= EMODT) > return guest_cpuid_has(vcpu, X86_FEATURE_SGX2); > @@ -373,9 +374,11 @@ int handle_encls(struct kvm_vcpu *vcpu) > { > u32 leaf = (u32)kvm_rax_read(vcpu); > > - if (!encls_leaf_enabled_in_guest(vcpu, leaf)) { > + if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX) || > + !guest_cpuid_has(vcpu, X86_FEATURE_SGX1)) { > kvm_queue_exception(vcpu, UD_VECTOR); ... above here, where the actual code reside? > - } else if (!sgx_enabled_in_guest_bios(vcpu) || !is_paging(vcpu)) { > + } else if (!encls_leaf_enabled_in_guest(vcpu, leaf) || > + !sgx_enabled_in_guest_bios(vcpu) || !is_paging(vcpu)) { > kvm_inject_gp(vcpu, 0); > } else { > if (leaf == ECREATE) > -- > 2.40.0.348.gf938b09366-goog >
On Thu, Apr 06, 2023, Huang, Kai wrote: > On Wed, 2023-04-05 at 16:45 -0700, Sean Christopherson wrote: > > Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD. > > SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a > > "soft" disable, e.g. if software disables machine check reporting, i.e. > > having SGX but not SGX1 is effectively "SGX completely unsupported" and > > and thus #UDs. > > If I recall correctly, this is an erratum which can clear SGX1 in CPUID while > the SGX flag is still in CPUID? Nope, not an erratum, architectural behavior. > But I am not sure whether this part is relevant to this patch? Because SDM > already says ENCLS causes #UD if SGX1 isn't present. This patch changes > "unsupported leaf" from causing #UD to causing #GP, which is also documented in > SDM. I wanted to capture why SGX1 is different and given special treatment in the SDM. I.e. to explain why SGX1 leafs are an exception to the "#GP if leaf unsupported" clause. > > Fixes: 9798adbc04cf ("KVM: VMX: Frame in ENCLS handler for SGX virtualization") > > Cc: Binbin Wu <binbin.wu@linux.intel.com> > > Cc: Kai Huang <kai.huang@intel.com> > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/kvm/vmx/sgx.c | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c > > index f881f6ff6408..1c092ab89c33 100644 > > --- a/arch/x86/kvm/vmx/sgx.c > > +++ b/arch/x86/kvm/vmx/sgx.c > > @@ -350,11 +350,12 @@ static int handle_encls_einit(struct kvm_vcpu *vcpu) > > > > static inline bool encls_leaf_enabled_in_guest(struct kvm_vcpu *vcpu, u32 leaf) > > { > > - if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX)) > > - return false; > > - > > + /* > > + * ENCLS #UDs if SGX1 isn't supported, i.e. this point will be reached > > Why #UDs instead of #UD? Is #UD a verb? Heh, it is now ;-) I can reword to something like /* * ENCLS generates a #UD if SGX1 isn't supported ... */ if my made up grammar is confusing. > > + * if and only if the SGX1 leafs are enabled. > > + */ > > Is it better to move "ENCLS #UDs if SGX1 isn't supported" part to ... > > > if (leaf >= ECREATE && leaf <= ETRACK) > > - return guest_cpuid_has(vcpu, X86_FEATURE_SGX1); > > + return true; > > > > if (leaf >= EAUG && leaf <= EMODT) > > return guest_cpuid_has(vcpu, X86_FEATURE_SGX2); > > @@ -373,9 +374,11 @@ int handle_encls(struct kvm_vcpu *vcpu) > > { > > u32 leaf = (u32)kvm_rax_read(vcpu); > > > > - if (!encls_leaf_enabled_in_guest(vcpu, leaf)) { > > + if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX) || > > + !guest_cpuid_has(vcpu, X86_FEATURE_SGX1)) { > > kvm_queue_exception(vcpu, UD_VECTOR); > > ... above here, where the actual code reside? My goal was to document why encls_leaf_enabled_in_guest() unconditionally returns true for SGX1 leafs, i.e. why it doesn't query X86_FEATURE_SGX1. I'm definitely not opposed to also adding a comment here, but I do want to keep the comment in encls_leaf_enabled_in_guest().
On Thu, 2023-04-06 at 11:00 -0700, Sean Christopherson wrote: > On Thu, Apr 06, 2023, Huang, Kai wrote: > > On Wed, 2023-04-05 at 16:45 -0700, Sean Christopherson wrote: > > > Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD. > > > SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a > > > "soft" disable, e.g. if software disables machine check reporting, i.e. > > > having SGX but not SGX1 is effectively "SGX completely unsupported" and > > > and thus #UDs. > > > > If I recall correctly, this is an erratum which can clear SGX1 in CPUID while > > the SGX flag is still in CPUID? > > Nope, not an erratum, architectural behavior. I found the relevant section in SDM: All supported IA32_MCi_CTL bits for all the machine check banks must be set for Intel SGX to be available (CPUID.SGX_Leaf.0:EAX[SGX1] == 1). Any act of clearing bits from '1 to '0 in any of the IA32_MCi_CTL register may disable Intel SGX (set CPUID.SGX_Leaf.0:EAX[SGX1] to 0) until the next reset. Looking at the code, it seems currently KVM won't clear SGX1 bit in CPUID when guest disables IA32_MCi_CTL (writing 0). Should we do that? > > > But I am not sure whether this part is relevant to this patch? Because SDM > > already says ENCLS causes #UD if SGX1 isn't present. This patch changes > > "unsupported leaf" from causing #UD to causing #GP, which is also documented in > > SDM. > > I wanted to capture why SGX1 is different and given special treatment in the SDM. > I.e. to explain why SGX1 leafs are an exception to the "#GP if leaf unsupported" > clause. OK. > > > > Fixes: 9798adbc04cf ("KVM: VMX: Frame in ENCLS handler for SGX virtualization") > > > Cc: Binbin Wu <binbin.wu@linux.intel.com> > > > Cc: Kai Huang <kai.huang@intel.com> > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > --- > > > arch/x86/kvm/vmx/sgx.c | 15 +++++++++------ > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c > > > index f881f6ff6408..1c092ab89c33 100644 > > > --- a/arch/x86/kvm/vmx/sgx.c > > > +++ b/arch/x86/kvm/vmx/sgx.c > > > @@ -350,11 +350,12 @@ static int handle_encls_einit(struct kvm_vcpu *vcpu) > > > > > > static inline bool encls_leaf_enabled_in_guest(struct kvm_vcpu *vcpu, u32 leaf) > > > { > > > - if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX)) > > > - return false; > > > - > > > + /* > > > + * ENCLS #UDs if SGX1 isn't supported, i.e. this point will be reached > > > > Why #UDs instead of #UD? Is #UD a verb? > > Heh, it is now ;-) I can reword to something like > > /* > * ENCLS generates a #UD if SGX1 isn't supported ... > */ > > if my made up grammar is confusing. > > > > + * if and only if the SGX1 leafs are enabled. > > > + */ > > > > Is it better to move "ENCLS #UDs if SGX1 isn't supported" part to ... > > > > > if (leaf >= ECREATE && leaf <= ETRACK) > > > - return guest_cpuid_has(vcpu, X86_FEATURE_SGX1); > > > + return true; > > > > > > if (leaf >= EAUG && leaf <= EMODT) > > > return guest_cpuid_has(vcpu, X86_FEATURE_SGX2); > > > @@ -373,9 +374,11 @@ int handle_encls(struct kvm_vcpu *vcpu) > > > { > > > u32 leaf = (u32)kvm_rax_read(vcpu); > > > > > > - if (!encls_leaf_enabled_in_guest(vcpu, leaf)) { > > > + if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX) || > > > + !guest_cpuid_has(vcpu, X86_FEATURE_SGX1)) { > > > kvm_queue_exception(vcpu, UD_VECTOR); > > > > ... above here, where the actual code reside? > > My goal was to document why encls_leaf_enabled_in_guest() unconditionally returns > true for SGX1 leafs, i.e. why it doesn't query X86_FEATURE_SGX1. I'm definitely > not opposed to also adding a comment here, but I do want to keep the comment in > encls_leaf_enabled_in_guest(). Sure. Anyway, Reviewed-by: Kai Huang <kai.huang@intel.com>
On Wed, Apr 12, 2023, Kai Huang wrote: > On Thu, 2023-04-06 at 11:00 -0700, Sean Christopherson wrote: > > On Thu, Apr 06, 2023, Huang, Kai wrote: > > > On Wed, 2023-04-05 at 16:45 -0700, Sean Christopherson wrote: > > > > Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD. > > > > SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a > > > > "soft" disable, e.g. if software disables machine check reporting, i.e. > > > > having SGX but not SGX1 is effectively "SGX completely unsupported" and > > > > and thus #UDs. > > > > > > If I recall correctly, this is an erratum which can clear SGX1 in CPUID while > > > the SGX flag is still in CPUID? > > > > Nope, not an erratum, architectural behavior. > > I found the relevant section in SDM: > > All supported IA32_MCi_CTL bits for all the machine check banks must be set > for Intel SGX to be available (CPUID.SGX_Leaf.0:EAX[SGX1] == 1). Any act of > clearing bits from '1 to '0 in any of the IA32_MCi_CTL register may disable > Intel SGX (set CPUID.SGX_Leaf.0:EAX[SGX1] to 0) until the next reset. > > Looking at the code, it seems currently KVM won't clear SGX1 bit in CPUID when > guest disables IA32_MCi_CTL (writing 0). Should we do that? No, the behavior isn't strictly required: clearing bits *may* disable Intel SGX. And there is zero benefit to the guest, the behavior exists in bare metal purely to allow the CPU to provide security guarantees. On the flip side, emulating the disabling of SGX without causing problems, e.g. when userspace sets MSRs, would be quite tricky.
On Wed, 2023-04-12 at 07:38 -0700, Sean Christopherson wrote: > On Wed, Apr 12, 2023, Kai Huang wrote: > > On Thu, 2023-04-06 at 11:00 -0700, Sean Christopherson wrote: > > > On Thu, Apr 06, 2023, Huang, Kai wrote: > > > > On Wed, 2023-04-05 at 16:45 -0700, Sean Christopherson wrote: > > > > > Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD. > > > > > SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a > > > > > "soft" disable, e.g. if software disables machine check reporting, i.e. > > > > > having SGX but not SGX1 is effectively "SGX completely unsupported" and > > > > > and thus #UDs. > > > > > > > > If I recall correctly, this is an erratum which can clear SGX1 in CPUID while > > > > the SGX flag is still in CPUID? > > > > > > Nope, not an erratum, architectural behavior. > > > > I found the relevant section in SDM: > > > > All supported IA32_MCi_CTL bits for all the machine check banks must be set > > for Intel SGX to be available (CPUID.SGX_Leaf.0:EAX[SGX1] == 1). Any act of > > clearing bits from '1 to '0 in any of the IA32_MCi_CTL register may disable > > Intel SGX (set CPUID.SGX_Leaf.0:EAX[SGX1] to 0) until the next reset. > > > > Looking at the code, it seems currently KVM won't clear SGX1 bit in CPUID when > > guest disables IA32_MCi_CTL (writing 0). Should we do that? > > No, the behavior isn't strictly required: clearing bits *may* disable Intel SGX. > And there is zero benefit to the guest, the behavior exists in bare metal purely > to allow the CPU to provide security guarantees. On the flip side, emulating the > disabling of SGX without causing problems, e.g. when userspace sets MSRs, would be > quite tricky. Yeah my thinking too. Agreed.
On Wed, 2023-04-12 at 11:15 +0000, Huang, Kai wrote: > On Thu, 2023-04-06 at 11:00 -0700, Sean Christopherson wrote: > > On Thu, Apr 06, 2023, Huang, Kai wrote: > > > On Wed, 2023-04-05 at 16:45 -0700, Sean Christopherson wrote: > > > > Per Intel's SDM, unsupported ENCLS leafs result in a #GP, not a #UD. > > > > SGX1 is a special snowflake as the SGX1 flag is used by the CPU as a > > > > "soft" disable, e.g. if software disables machine check reporting, i.e. > > > > having SGX but not SGX1 is effectively "SGX completely unsupported" and > > > > and thus #UDs. > > > [...] > > Reviewed-by: Kai Huang <kai.huang@intel.com> > > Tested by disabling SGX2 in the guest and running SGX2 leaf inside, and in my testing the #GP is injected to the guest due to leaf not enabled. Tested-by: Kai Huang <kai.huang@intel.com>
diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c index f881f6ff6408..1c092ab89c33 100644 --- a/arch/x86/kvm/vmx/sgx.c +++ b/arch/x86/kvm/vmx/sgx.c @@ -350,11 +350,12 @@ static int handle_encls_einit(struct kvm_vcpu *vcpu) static inline bool encls_leaf_enabled_in_guest(struct kvm_vcpu *vcpu, u32 leaf) { - if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX)) - return false; - + /* + * ENCLS #UDs if SGX1 isn't supported, i.e. this point will be reached + * if and only if the SGX1 leafs are enabled. + */ if (leaf >= ECREATE && leaf <= ETRACK) - return guest_cpuid_has(vcpu, X86_FEATURE_SGX1); + return true; if (leaf >= EAUG && leaf <= EMODT) return guest_cpuid_has(vcpu, X86_FEATURE_SGX2); @@ -373,9 +374,11 @@ int handle_encls(struct kvm_vcpu *vcpu) { u32 leaf = (u32)kvm_rax_read(vcpu); - if (!encls_leaf_enabled_in_guest(vcpu, leaf)) { + if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX) || + !guest_cpuid_has(vcpu, X86_FEATURE_SGX1)) { kvm_queue_exception(vcpu, UD_VECTOR); - } else if (!sgx_enabled_in_guest_bios(vcpu) || !is_paging(vcpu)) { + } else if (!encls_leaf_enabled_in_guest(vcpu, leaf) || + !sgx_enabled_in_guest_bios(vcpu) || !is_paging(vcpu)) { kvm_inject_gp(vcpu, 0); } else { if (leaf == ECREATE)