Re: [RFC PATCH 04/23] x86/cpufeatures: Add SGX1 and SGX2 sub-features

From: Sean Christopherson <seanjc@google.com>
To: Kai Huang <kai.huang@intel.com>
Cc: Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@intel.com>,
	linux-sgx@vger.kernel.org, kvm@vger.kernel.org, x86@kernel.org,
	jarkko@kernel.org, luto@kernel.org, haitao.huang@intel.com,
	pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com
Subject: Re: [RFC PATCH 04/23] x86/cpufeatures: Add SGX1 and SGX2 sub-features
Date: Tue, 12 Jan 2021 15:17:15 -0800	[thread overview]
Message-ID: <X/4t+6JcyTGVEG2e@google.com> (raw)
In-Reply-To: <dea875ea60cdef68fa8fe5b8f8cf3e8ed6a5df2e.camel@intel.com>

On Wed, Jan 13, 2021, Kai Huang wrote:
> On Tue, 2021-01-12 at 18:51 +0100, Borislav Petkov wrote:
> > On Tue, Jan 12, 2021 at 09:15:52AM -0800, Sean Christopherson wrote:
> > > We want the boot_cpu_data.x86_capability memcpy() so that KVM doesn't advertise
> > > support for features that are intentionally disabled in the kernel, e.g. via
> > > kernel params.  Except for a few special cases, e.g. LA57, KVM doesn't enable
> > > features in the guest if they're disabled in the host, even if the features are
> > > supported in hardware.
> > > 
> > > For some features, e.g. SMEP and SMAP, honoring boot_cpu_data is mostly about
> > > respecting the kernel's wishes, i.e. barring hardware bugs, enabling such
> > > features in the guest won't break anything.  But for other features, e.g. XSAVE
> > > based features, enabling them in the guest without proper support in the host
> > > will corrupt guest and/or host state.
> > 
> > Ah ok, that is an important point.
> > 
> > > So it's really the CPUID read that is (mostly) superfluous.
> > 
> > Yeah, but that is cheap, as we established.
> > 
> > Ok then, I don't see anything that might be a problem and I guess we can
> > try that handling of scattered bits in kvm and see how far we'll get.
> 
> Hi Sean, Boris,
> 
> Thanks for all  your feedback.
> 
> Sean,
> 
> Do you want to send me your patch (so that with your SoB), or do you want me to copy
> & paste the code you posted in this series, plus Suggested-by you? Or how do you want
> to proceed?
> 
> Also to me it is better to separate X86_FEATURE_SGX1/2 with rest of KVM changes?

Hmm, I'll split the changes into two proper patches and send them to you off list.

> And do you think adding a dedicated, i.e. kvm_scattered_cpu_caps[], instead of using
> existing kvm_cpu_cap[NCAPINTS] would be helpful to solve the problem caused by adding
> new leaf to x86 core (see my another reply in this thread)?

Probably not, because then we'd have to add new helpers to deal with the new
array, or change all the helpers to take the array as a pointer.  Blasting past
NCAPINTS is a little evil, but it does slot in nicely to the existing code.