linux-kernel - Re: [ RFC, PATCH - 1/2, v2 ] x86-microcode: refactor microcode output messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b647ffbd0911070422g154c63bbo2fec62ba63c3c106@mail.gmail.com>
Date:	Sat, 7 Nov 2009 13:22:19 +0100
From:	Dmitry Adamushko <dmitry.adamushko@...il.com>
To:	Andreas Herrmann <herrmann.der.user@...glemail.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	"H. Peter Anvin" <hpa@...or.com>, Mike Travis <travis@....com>,
	Tigran Aivazian <tigran@...azian.fsnet.co.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <borislav.petkov@....com>,
	Andreas Mohr <andi@...as.de>, Jack Steiner <steiner@....com>
Subject: Re: [ RFC, PATCH - 1/2, v2 ] x86-microcode: refactor microcode output 
	messages

2009/11/6 Andreas Herrmann <herrmann.der.user@...glemail.com>:
> On Fri, Nov 06, 2009 at 01:56:31PM +0100, Dmitry Adamushko wrote:
>> 2009/11/6 Andreas Herrmann <herrmann.der.user@...glemail.com>:
>
>   <snip>
>
>> >> (CPU3 belongs to both sets) unless summarize_cpu_info() is utterly
>> >> broken.
>> >
>> > I didn't check that yet.
>>
>> Yeah, this behavior is likely due to a missing cpumask_clear() in
>> summarize_cpu_info().
>
> Yeah, that fixes the wrong messages.
> The other problem of not-updated CPU microcode after suspend/resume persists.
>
>> should be as follows:
>>
>>       if (!alloc_cpumask_var(&cpulist, GFP_KERNEL))
>>               return;
>>
>> +    cpumask_clear(cpulist);
>
> Better use zalloc_cpumask instead of alloc/clear.

ok.

>
>> >> sure, my test is somewhat limited... anyway, first of all I'd like to
>> >> get a clear understanding of your logs. Thanks for yout test btw. :-))
>> >
>> > I'll send you full logs asap.
>>
>> Thanks. Maybe it's something about a particular sequence of actions
>> that triggers this behavior. Or was it reproducible with the very
>> first pm-suspend invocation after "modprobe microcode.ko"?
>
> The sequence is:
>
> 1. loading microcode.ko
> 2. setting cpu2 offline
> 3. setting cpu2 online
> 4. suspend (pm-suspend)
> 5. resume
>
> microcode of CPU2 is not updated:
>
>  # for i in `seq 0 3`; do lsmsr -c $i PATCH_LEVEL; done
>  PATCH_LEVEL          = 0x0000000001000083
>  PATCH_LEVEL          = 0x0000000001000083
>  PATCH_LEVEL          = 0x0000000001000065
>  PATCH_LEVEL          = 0x0000000001000083
>
> dmesg attached.

It looks like the microcode of CPU2 was not updated at step (3) [ and
not cached in uci->mc so that there was nothing to be loaded at resume
time later on ].

...
platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
microcode: size 1936, total_size 960
microcode: CPU2: patch mismatch (processor_rev_id: 1020, equiv_cpu_id: 1022)
microcode: size 968, total_size 960
PM: Syncing filesystems ... done.
...

These messages are from ->request_microcode_fw() but then there is
nothing from ->apply_microcode(). I'd expect to see "microcode: CPU%d:
updated (new patch_level=0x%x)".

So either request_fw() -> generic_load_microcode() somehow fails to
find/cache a ucode (while it could do this at microcode-load time) or
apply_microcode_on_target() -> smp_call_function_single() fails in
this context. I made a test (some changes to load a cached ->mc at
cpu-online time) to verify the latter hypothesis and it didn't reveal
any problems or it requires some special conditions (also my kernel is
-rc5+).


>
> As I've said, that test used to pass with all CPUs updated to new
> ucode in the past (at least that I think so ;-( -- but in contrast to
> my previous mail this doesn't seem to be related to your patch. I
> tested latest mainline and the test fails as well ... seems that I
> need to do some debugging.

Ok. Then by instrumenting ->request_microcode_fs() and
apply_microcode_on_target() we would get a hint on what's wrong.


>
>
> Regards,
> Andreas
>
> PS1: You should remove the needless newline from the patch level string:
>
>  static int version_snprintf(char *buf, int len, struct cpu_signature *csig)
>  {
> -       return snprintf(buf, len, "patch_level=0x%x\n", csig->rev);
> +       return snprintf(buf, len, "patch_level=0x%x", csig->rev);
>  }

ack.



-- Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/