On Tue, Mar 26, 2019 at 05:17:40AM -0700, Huang, Kai wrote: > On Wed, 2019-03-20 at 18:21 +0200, Jarkko Sakkinen wrote: > > From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > > > > Similar to other large Intel features such as VMX and TXT, SGX must be > > explicitly enabled in IA32_FEATURE_CONTROL MSR to be truly usable. > > Clear all SGX related capabilities if SGX is not fully enabled in > > IA32_FEATURE_CONTROL or if the SGX1 instruction set isn't supported > > (impossible on bare metal, theoretically possible in a VM if the VMM is > > doing something weird). > > > > Like SGX itself, SGX Launch Control must be explicitly enabled via a > > flag in IA32_FEATURE_CONTROL. Clear the SGX_LC capability if Launch > > Control is not fully enabled (or obviously if SGX itself is disabled). > > > > Note that clearing X86_FEATURE_SGX_LC creates a bit of a conundrum > > regarding the SGXLEPUBKEYHASH MSRs, as it may be desirable to read the > > MSRs even if they are not writable, e.g. to query the configured key, > > but clearing the capability leaves no breadcrum for discerning whether > > or not the MSRs exist. But, such usage will be rare (KVM is the only > > known case at this time) and not performance critical, so it's not > > unreasonable to require the use of rdmsr_safe(). Clearing the cap bit > > eliminates the need for an additional flag to track whether or not > > Launch Control is truly enabled, which is what we care about the vast > > majority of the time. > > [Resend. Somehow my last reply doesn't show up in my mailbox so not sure whether I sent it > successfully or not. Sorry if you receving duplicated mails.] > > However this is not consistent with HW behavior. If LC feature flag is not present, then MSRs should > have hash of Intel's key, which is not always the case here, when you expose SGX to KVM. Enclave in > KVM guest will get unexpected EINIT error when launing Intel enclave, if on HW MSRs are configured > to 3rd party value but locked to readonly. Intel doesn't have a singular key. The internal reset value of the LE pubkey hash MSRs is micro-architectural, i.e. can change without warning on any given processor. All current processors with SGX support may use the same reset value, but it's not something that customers should/can rely on. That being said, this in no way impacts KVM's ability to virtualize SGX, e.g. KVM can directly do CPUID and {RD,WR}MSR to probe the capabilities of the platform as needed. > My opition is we already have enough cases that violates HW behavior in > SGX virtualization, let's not have one more. What are the other cases? > Besides, why do we "need an additional flag to track whether or not > Launch Control is truly enabled"? Doesn't driver only need to know whether > MSRs are writable? Yes, and that's why we're overloading X86_FEATURE_SGX_LC to be set if and only if SGX_LC is supported *and* enabled, e.g. so that the kernel can simply check X86_FEATURE_SGX_LC without having to also probe the MSRs.