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

Kai Huang
2021-01-12T02:02:30Z

On Mon, 11 Jan 2021 11:20:11 -0800 Sean Christopherson wrote:
> On Mon, Jan 11, 2021, Borislav Petkov wrote:
> > On Mon, Jan 11, 2021 at 09:54:17AM -0800, Sean Christopherson wrote:
> > > It would be possible for KVM to break the dependency on X86_FEATURE_* bit
> > > offsets by defining a translation layer, but I strongly feel that adding manual
> > > translations will do more harm than good as it increases the odds of us botching
> > > a translation or using the wrong feature flag, creates potential namespace
> > > conflicts, etc...
> >
> > Ok, lemme see if we might encounter more issues down the road...
> >
> > +enum kvm_only_cpuid_leafs {
> > + CPUID_12_EAX = NCAPINTS,
> > + NR_KVM_CPU_CAPS,
> > +
> > + NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> > +};
> > +
> >
> > What happens when we decide to allocate a separate leaf for CPUID_12_EAX
> > down the road?
>
> Well, mechanically, that would generate a build failure if the kernel does the
> obvious things and names the 'enum cpuid_leafs' entry CPUID_12_EAX. That would
> be an obvious clue that KVM should be updated.
>
> If the kernel named the enum entry something different, and we botched the code
> review, KVM would continue to work, but would unnecessarily copy the bits it
> cares about to its own word. E.g. the boot_cpu_has() checks and translation to
> __X86_FEATURE_* would still be valid. As far as failure modes go, that's not
> terrible.

Should we add a dedicated, i.e. kvm_scattered_cpu_caps[], instead of using
existing kvm_cpu_cap[NCAPINTS]? If so this issue can be avoided??

>
> > You do it already here
> >
> > Subject: [PATCH 04/13] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption
> >
> > for the AMD leaf.
> >
> > I'm thinking this way around - from scattered to a hw one - should be ok
> > because that should work easily. The other way around, taking a hw leaf
> > and scattering it around x86_capability[] array elems would probably be
> > nasty but with your change that should work too.
> >
> > Yah, I'm just hypothesizing here - I don't think this "other way around"
> > will ever happen...
> >
> > Hmm, yap, I can cautiously say that with your change we should be ok...
> >
> > Thx.
> >
> > --
> > Regards/Gruss,
> > Boris.
> >
> > https://people.kernel.org/tglx/notes-about-netiquette