Re: [PATCH for 4.19?] x86/Intel: unlock CPUID earlier for the BSP

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.06.2024 12:14, Andrew Cooper wrote:
> On 14/06/2024 7:27 am, Jan Beulich wrote:
>> On 13.06.2024 18:17, Andrew Cooper wrote:
>>> On 13/06/2024 9:19 am, Jan Beulich wrote:
>>>> Intel CPUs have a MSR bit to limit CPUID enumeration to leaf two. If
>>>> this bit is set by the BIOS then CPUID evaluation does not work when
>>>> data from any leaf greater than two is needed; early_cpu_init() in
>>>> particular wants to collect leaf 7 data.
>>>>
>>>> Cure this by unlocking CPUID right before evaluating anything which
>>>> depends on the maximum CPUID leaf being greater than two.
>>>>
>>>> Inspired by (and description cloned from) Linux commit 0c2f6d04619e
>>>> ("x86/topology/intel: Unlock CPUID before evaluating anything").
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>> ---
>>>> While I couldn't spot anything, it kind of feels as if I'm overlooking
>>>> further places where we might be inspecting in particular leaf 7 yet
>>>> earlier.
>>>>
>>>> No Fixes: tag(s), as imo it would be too many that would want
>>>> enumerating.
>>> I also saw that go by, but concluded that Xen doesn't need it, hence why
>>> I left it alone.
>>>
>>> The truth is that only the BSP needs it.  APs sort it out in the
>>> trampoline via trampoline_misc_enable_off, because they need to clear
>>> XD_DISABLE in prior to enabling paging, so we should be taking it out of
>>> early_init_intel().
>> Except for the (odd) case also mentioned to Roger, where the BSP might have
>> the bit clear but some (or all) AP(s) have it set.
> 
> Fine I suppose.  It's a single MSR adjustment once per CPU.
> 
>>
>>> But, we don't have an early BSP-only early hook, and I'm not overwhelmed
>>> at the idea of exporting it from intel.c
>>>
>>> I was intending to leave it alone until I can burn this whole
>>> infrastructure to the ground and make it work nicely with policies, but
>>> that's not a job for this point in the release...
>> This last part reads like the rest of your reply isn't an objection to me
>> putting this in with Roger's R-b, but it would be nice if you could
>> confirm this understanding of mine. Without this last part it, especially
>> the 2nd from last paragraph, certainly reads a little like an objection.
> 
> I'm -1 to this generally.  It's churn without fixing anything AFAICT,

How that? We clearly do the adjustment too late right now for the BSP.
All the leaf-7 stuff added to early_cpu_init() (iirc in part in the course
of speculation work) is useless on a system where firmware set that flag.

Jan

> and is moving in a direction which will need undoing in the future.
> 
> But, because it doesn't fix anything, I don't think it's appropriate to
> be tagged as 4.19 even if you and roger feel strongly enough to put it
> into 4.20.
> 
> ~Andrew