Re: [PATCH] x86/nmi: Fix mis-classification of watchdog NMIs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.08.2026 17:55, Andrew Cooper wrote:
> On 17/08/2026 9:30 am, Jan Beulich wrote:
>> On 13.08.2026 19:43, Andrew Cooper wrote:
>>> It used to be the case that cpu_data[] inherited the BSP's cpuid_level until
>>> the AP had calculated it itself.  Following the rework, cpuid_level has a
>>> placeholder 1 until it is caluclated propely.
>>>
>>> setup_apic_nmi_watchdog() happens to be called on the BSP after SMP bringup,
>>> meaning that the first call is on CPU1.  It is also positioned in the window
>>> where cpu_data[] is garbage.
>>>
>>> As a result, setup_p6_watchdog()'s one-time calculation of the performance
>>> counter width falls back into Pentium compatibility mode assuming 32bit
>>> counters.  This causes a watchdog NMI which is delayed a little (e.g. from 
>>> an
>>> SMI), to appear as if it hadn't overflowed, and therefore be (mis)classifed 
>>> as
>>> not a watchdog NMI.  On systems where unknown NMIs are treated as fatal, 
>>> this
>>> results in a spurious crash.
>>>
>>> Switch setup_p6_watchdog() to use boot_cpu_data.cpuid_level, which is how 
>>> this
>>> is checked almost everywhere else.
>>>
>>> core2_vpmu_init() used the same pattern to look at leaf 0xa.  Despite being
>>> init code and only running on the BSP, {boot,current}_cpu_data are different
>>> objects, so switch it over to checking boot_cpu_data.cpuid_level too.
>>>
>>> Fixes: 7126b7f806d5 ("x86/CPU: re-work populating of cpu_data[]")
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Thanks.
> 
>>
>>> I hate this fix, but it's the only thing which I consider remotely safe to
>>> backport.  Recent attempts to alter CPUID ordering have 0 success at being
>>> bug-free.
>> How that? Collecting CPUID output should be doable almost first thing. There
>> are no (or in case of doubt: there should not be any) dependencies on about
>> anything else. Of course re-collecting may still be necessary after ucode
>> loading. Yet from what you say I must be missing something crucial.
> 
> I was referring to commits in Xen rearranging the boot sequence with
> respect to feature handling.  There have been many failures recently.
> 
>> With that in mind, I'm also questioning the Fixes: tag (without this being a
>> request to drop or change it): Using another CPU's data isn't much better
>> than using partially unset data. Unless we assumed full symmetry, at which
>> point re-obtaining of most data on the APs would be entirely useless. Hence
>> the issue was pre-existing, with one bug there hiding the issue addressed
>> here.
> 
> Copying the BSP is less bad than the current behaviour.  It's not
> necessarily ideal, but it's a damsight better default than the arbitrary
> 1 that this patch puts in place.
> 
> Even with the very old 64bit systems where mixing steppings was
> commonplace, I'm not aware of a vendor supported combination where
> max_leaf was different.
> 
> 
> The issue was not prexisting.  Prior to your rearrangement, the NMI
> watchdog setup found the counter width in CPUID and used it, without
> falling back into original Pentium compatibility mode.

FTAOD - with "pre-existing" I meant the using of the (possibly) wrong data,
not the particular issue of the NMI watchdog being affected.

Jan