Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage!

[Borislav Petkov]

On Mon, Nov 21, 2016 at 03:37:16PM +0100, Peter Zijlstra wrote:
> Yeah, but this one does a printk() when it hits the contidion it checks
> for, so not tracing it would be fine I think.

FWIW, there's already:

static inline void notrace
native_write_msr(unsigned int msr, u32 low, u32 high)
{
        __native_write_msr_notrace(msr, low, high);
        if (msr_tracepoint_active(__tracepoint_write_msr))
                do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
}

so could be mirrored.

> Also, Boris, why do we need to redo that rdmsr until we see that bit
> set? Can't we simply do the rdmsr once and then be done with it?

Hmm, so the way I read the definition of those bits -
K8_INTP_C1E_ACTIVE_MASK - it looks like they get set when the cores
enter halt:

28 C1eOnCmpHalt: C1E on chip multi-processing halt. Read-write.

1=When all cores of a processor have entered the halt state, the processor
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
generates an IO cycle as specified by IORd, IOMsgData, and IOMsgAddr.
When this bit is set, SmiOnCmpHalt and IntPndMsg must be 0, otherwise
the behavior is undefined. For revision DA-C, this bit is only supported
for dual-core processors. For revision C3 and E, this bit is supported
for any number of cores. See 2.4.3.3.3 [Hardware Initiated C1E].

27 SmiOnCmpHalt: SMI on chip multi-processing halt. Read-write.

1=When all cores of the processor have entered the halt state, the
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

processor generates an SMI-trigger IO cycle as specified by IORd,
IOMsgData, and IOMsgAddr. When this bit is set C1eOnCmpHalt and
IntPndMsg must be 0, otherwise the behavior is undefined. The status is
stored in SMMFEC4[SmiOnCmpHaltSts]. See 2.4.3.3.1 [SMI Initiated C1E].

But the thing is, we do it only once until amd_e400_c1e_detected is true.

FWIW, I'd love it if we could do

	if (cpu_has_bug(c, X86_BUG_AMD_APIC_C1E))

but I'm afraid we need to look at that MSR at least once until we set
the boolean.

I could ask if that is really the case and whether we can detect it
differently...

In any case, I have a box here in case you want me to test patches.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.