From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751811AbZKGMWR (ORCPT ); Sat, 7 Nov 2009 07:22:17 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751782AbZKGMWR (ORCPT ); Sat, 7 Nov 2009 07:22:17 -0500 Received: from mail-ew0-f228.google.com ([209.85.219.228]:46923 "EHLO mail-ew0-f228.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751744AbZKGMWP convert rfc822-to-8bit (ORCPT ); Sat, 7 Nov 2009 07:22:15 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=sX3wcKUFyoo8RGlBOGUDyM+oDxIR58JSxtXRtAsiWdFKGi77X6suQkJd0HqcREqinh ftzraA2ymk8+AiUhkzvIhxG4Yyl8kIT/+LmTiVg1lA72FJdl1XMX7eQc39W+9oNMbSFA Pxe/EY/waBIxeSbMdPry86QPr+IZqmmJ0uHvA= MIME-Version: 1.0 In-Reply-To: <20091106194626.GC18592@alberich.amd.com> References: <1257192496.5941.8.camel@dimm> <20091105153742.GA18592@alberich.amd.com> <20091106123403.GB18592@alberich.amd.com> <20091106194626.GC18592@alberich.amd.com> Date: Sat, 7 Nov 2009 13:22:19 +0100 Message-ID: Subject: Re: [ RFC, PATCH - 1/2, v2 ] x86-microcode: refactor microcode output messages From: Dmitry Adamushko To: Andreas Herrmann Cc: linux-kernel@vger.kernel.org, Ingo Molnar , "H. Peter Anvin" , Mike Travis , Tigran Aivazian , Thomas Gleixner , Borislav Petkov , Andreas Mohr , Jack Steiner Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2009/11/6 Andreas Herrmann : > On Fri, Nov 06, 2009 at 01:56:31PM +0100, Dmitry Adamushko wrote: >> 2009/11/6 Andreas Herrmann : > >   > >> >> (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