Re: [PATCH 2/2] KVM: VMX: Inject #GP, not #UD, if SGX2 ENCLS leafs are unsupported

[Huang, Kai]

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.