Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760433AbZDXRBc (ORCPT ); Fri, 24 Apr 2009 13:01:32 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753701AbZDXRBO (ORCPT ); Fri, 24 Apr 2009 13:01:14 -0400 Received: from ey-out-2122.google.com ([74.125.78.25]:5310 "EHLO ey-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753282AbZDXRBN (ORCPT ); Fri, 24 Apr 2009 13:01:13 -0400 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=dxAAttMVDQPDTDGzhSxZCyyUj4eFfHoZVI6iHtkSBgXuU0on7mvazigXn605vsjovl AUCqOzaKpanDEsFKGh+9Xp+D+fCqyhuE68K4RrjGULikUEeC4f9UTiN25M1PZl+YHGRU kaY6B3rquJFQ9cMivx5lgORPAE7k3A13qs7ro= MIME-Version: 1.0 In-Reply-To: References: <1240258569.6195.8.camel@earth> <1240344440.5861.10.camel@earth> <1240439073.12721.23.camel@earth> <20090423082704.GD599@elte.hu> Date: Fri, 24 Apr 2009 19:01:11 +0200 Message-ID: Subject: Re: [PATCH] x86 microcode: work_on_cpu and cleanup of the synchronization logic From: Dmitry Adamushko To: Hugh Dickins Cc: Ingo Molnar , Andrew Morton , Rusty Russell , Andreas Herrmann , Peter Oruba , Arjan van de Ven , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2992 Lines: 66 2009/4/24 Dmitry Adamushko : > 2009/4/24 Hugh Dickins : >> On Fri, 24 Apr 2009, Dmitry Adamushko wrote: >>> 2009/4/23 Hugh Dickins : >>> > >>> > I guess your mutex Synchronization works out, but are interrupts >>> > still disabled around the critical wrmsr()s, wherever they're getting >>> > called from? >>> >>> Yes, *msr() calls are only done from functions that are now being >>> called via smp_call_function_single(). The later seems to always do it >>> with disabled interrupts. The only exception is mc_sysdev_resume() >>> calling ->apply_microcode() directly but this one in turn is always >>> called with disabled interrupts. >>> >>> But now that you mentioned it I wonder if we may actually need a >>> spinlock there... can we have multi-threaded cpus/cores with (all | >>> some) shared msr registers? >> >> Good thinking, yes we can and do, unless I'm misinterpreting the >> evidence. Though P4 Xeon and Atom startup messages give the opposite >> impression, claiming to update all cpus from lower revision, more >> careful tests starting from "maxcpus=1" and then "echo 1 >online" >> (which, unless you've fiddled around putting the microcode_ctl'ed >> microcode.dat into /lib/firmware/intel-ucode/wherever, isn't able >> to update at online time on Intel) shows that the later onlined >> siblings already have the updated microcode applied to their >> previously onlined siblings. Which isn't surprising, but I'd >> been lulled into thinking the opposite by the startup sequence. > > Ah, stupid me :-/ These differences in behavior during the startup and > the later update reveal a real bug in my patch. > > this part: > > mutex_lock(µcode_mutex); > error = sysdev_driver_register(&cpu_sysdev_class, &mc_sysdev_driver); > mutex_unlock(µcode_mutex); > > sysdev_driver_register() calls mc_sysdev_driver's ->add() (which is > mc_sysdev_add()) for each cpu in a loop. Obviously, "microcode_mutex" > can't help to serialize these calls, oops. A very obvious thing but I > missed it. Well, never mind about this one. I might have really been in an altered state of mind and now need to wait until it comes back to (let's call it) normality. All ucode operations are synchronous so obviously we don't need to serialize actions that run sequentially one after another. Sorry for the noise here. But then I wonder why behavior (the fact that all threads seem to upgrade to a newer version during the startup but they seem to already be 'up-to-date' if onlined later) during the startup is different... Too pity that I can't see it with my setups (heh, I perhaps could play with it by actually downgrading cpus to older ucode). -- Best regards, Dmitry Adamushko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/