Re: [PATCH 5/5] x86/nmi: Don't configure EvtSel repeatedly

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/08/2026 3:20 pm, Jan Beulich wrote:
> On 05.08.2026 14:45, Andrew Cooper wrote:
>> In both setup_{k7,p6}_watchdog(), EvtSel0 is first zeroed, then written with
>> everything but the enable bit, then written with the enable bit.
>>
>> setup_p4_watchdog() is slightly more complicated, owing to what
>> appears to be a bug introduced by commit 2a2bd8de16b6 ("Clean up NMI
>> watchdog handler."), which causes a second bit to be temporarily
>> different too.
>>
>> The middle of the three writes is useless in all cases.  Drop it.
> Spotting the 1st write in setup_p4_watchdog() wasn't quite as easy, as
> MSR_P4_BPU_CCCR0 (as passed to clear_msr_range()) has nothing to do with
> MSR_P4_IQ_CCCR0. Using unrelated MSR names there is as unhelpful as using
> raw hex numbers.

Perf counters on the P4 are utterly insane, but our local logic really
doesn't help matters.

Another option would be to remove P4 watchdog support, in the basis that
we really can't test it.

>
>> While doing this, rename the 'counter' parameter for
>> setup_p6_watchdog().  It is the event which is passed in; the counter
>> is always counter 0.
>>
>> No functional change.
> These sequences of writes almost look as if they were trying to cover for
> errata. Are you sufficiently sure there are none anywhere, for this to
> truly be no functional change?

There is a reason to write logic in this form; it's just not applicable
to us.

The original P6 (besides being 32bit only) had two counters, but the
enable bit was in counter 0 only and controlled both.  I.e. you needed
to write logic to program EvtSel1 without the enable bit (it is strictly
reserved), and then EvtSel0 with the enable bit, in that order.  This is
in the SDM, just well hidden.

I spoke to various people, including PeterZ who wrote the perf
infrastructure in Linux (starting with Westmere, so it never ran on P6),
and Linux has absolutely no logic of this form at all.

I am reasonably confident that it's just copy&paste without due care and
attention which has left us with the code in this form.

>  If so, ...
>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

~Andrew