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

Kai Huang
2021-01-13T01:05:58Z

On Tue, 2021-01-12 at 15:17 -0800, Sean Christopherson wrote:
> 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.

Thanks.

>
> > 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.

Sure. Thanks.