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

Jan Beulich <[email protected]>
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <[email protected]>
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 <[email protected]>
>> Reviewed-by: Jan Beulich <[email protected]>
> 
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.