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

Sean Christopherson
2021-01-09T01:02:54Z

On Sat, Jan 09, 2021, Borislav Petkov wrote:
> On Fri, Jan 08, 2021 at 03:55:52PM -0800, Sean Christopherson wrote:
> > To be fair, this is the third time we've got conflicting, direct feedback on
> > this exact issue. I do agree that it doesn't make sense to burn a whole word
> > for just two features, I guess I just feel like whining.
> >
> > [*] https://lore.kernel.org/kvm/20180828102140.GA31102@nazgul.tnic/
> > [*] https://lore.kernel.org/linux-sgx/20190924162520.GJ19317@zn.tnic/
>
> Well, sorry that I confused you guys but in hindsight we probably should
> have stopped you right then and there from imposing kvm requirements on
> the machinery behind *_cpu_has() and kvm should have been a regular user
> of those interfaces like the rest of the kernel code - nothing more.
>
> And if you'd like to do your own X86_FEATURE_* querying but then extend
> it with its own functionality, then that should have been decoupled.
>
> And I will look at your patch later when brain is actually awake but
> I strongly feel that in order to avoid such situations in the future,
> *_cpu_has() internal functionality should be separate from kvm's
> respective CPUID leafs representation. For obvious reasons.

I kinda agree, but I'd prefer not to fully decouple KVM's CPUID stuff. The more
manual definitions/translations we have to create, the more likely it is that
we'll screw something up.

> And if there should be some partial sharing - if that makes sense at all
> - then that should be first agreed upon.

Assuming the code I wrote actually works, I think that gets KVM to the point
where handling scattered features isn't awful, which should eliminate most of
the friction. KVM would still be relying on the internals of *_cpu_has(), but
there are quite a few build-time assertions that help keep things aligned. And,
what's best for the kernel will be what's best for KVM the vast majority of the
time, e.g. I don't anticipate the kernel scattering densely populated words just
for giggles.