On Tue, Sep 24, 2019 at 08:21:19PM +0200, Borislav Petkov wrote:
> On Tue, Sep 24, 2019 at 10:43:11AM -0700, Sean Christopherson wrote:
> > The intent of running on every CPU is to verify MSR_IA32_FEATURE_CONTROL
> > is correctly configured on all CPUs. It's extremely unlikely that
> > firmware would misconfigure or fail to write the MSR on only APs, but if
> > that does happen we'll spam dmesg and possibly panic or hang the kernel.
> >
> > The severity of the fallout is why we're being paranoid. KVM is similarly
> > paranoid about VMX enabling since it'll BUG() on an unexpected fault due
> > to a misconfigured FEATURE_CONTROL.
>
> None of that is in the commit message or written anywhere AFAICT. And my
> crystal ball doesn't show it either so please write down properly why
> this is needed. Better over the function as a comment I'd say.
Added a remark:
The check is done for every CPU, not just BSP, in order to verify that
MSR_IA32_FEATURE_CONTROL is correctly configured on all CPUs. The other
parts of the kernel, like the enclave driver, expect the same
configuration from all CPUs.
I think here is not necessary to go into KVM implementation details to
make a case for this one. This is just a sane contract/expectation for
anything using SGX and thus it is better to validate it before anything
gets to use it.
/Jarkko
