Date: Thu, 16 Jul 2009 18:51:36 +0200
From: Borislav Petkov <borislav.petkov@amd.com>
To: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [x86, msr]: execute on the correct CPU subset (was:
 Re: [PATCH] [x86, msr]: remove code duplication)
Message-ID: <20090716165136.GA31204@aftab>
References: <1246890592-12256-1-git-send-email-borislav.petkov@amd.com>
 <4A526D38.7000803@zytor.com>
 <20090707103723.GD5668@aftab>
 <4A537010.3050905@zytor.com>
 <20090708111911.GC19315@aftab>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
In-Reply-To: <20090708111911.GC19315@aftab>
User-Agent: Mutt/1.5.20 (2009-06-14)
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2379
Lines: 67

Hi,

On Wed, Jul 08, 2009 at 01:19:11PM +0200, Borislav Petkov wrote:
> On Tue, Jul 07, 2009 at 08:56:00AM -0700, H. Peter Anvin wrote:
> > Borislav Petkov wrote:
> > > 
> > > Actually, the more important question is why am I executing anything on
> > > my own CPU without first checking if it is in the cpumask _at_ _all_?!
> > > /me ducks behind the sofa.
> > > 
> > > The right thing to do should be something like the following:
> > > 
> > > preempt_disable();
> > > this_cpu = raw_smp_processor_id();
> > > 
> > > if (cpumask_test_cpu(this_cpu, mask)) {
> > > 	local_irq_disable();
> > > 	msr_func(&rv);
> > > 	local_irq_enable();
> > > }
> > > 
> > > smp_call_function_many(mask, msr_func, &rv, 1);
> > > preempt_enable();
> > > 
> > 
> > I don't see why you're disabling local IRQs.
> 
> I guess I was trying to be overly careful but can't seem to think of a
> case when this would be appropriate. Hmm...
> 
> --
> From: Borislav Petkov <borislav.petkov@amd.com>
> Date: Mon, 6 Jul 2009 16:08:34 +0200
> Subject: [PATCH] [x86, msr]: execute on the correct CPU subset
> 
> rdmsr_on_cpus/wrmsr_on_cpus were erroneously executing on the current
> CPU even in the case where it wasn't in the supplied bitmask. Add a
> check for that and handle accordingly.
> 
> While at it, since rdmsr_on_cpus and wrmsr_on_cpus are almost identical,
> fold them into a common __rwmsr_on_cpus helper passing a function
> pointer arg to the actual MSR operation.
> 
> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/lib/msr.c |   53 +++++++++++++++++++--------------------------------
>  1 files changed, 20 insertions(+), 33 deletions(-)

any comments on that one, NAK/AK? Since it is a fix and not changing the
interface to external users, it might be a good idea to include it in
2.6.31, IMHO, no?

-- 
Regards/Gruss,
Boris.

Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. M?nchen, Germany
 Research | Gesch?ftsf?hrer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis M?nchen
  (OSRC)  | Registergericht M?nchen, HRB Nr. 43632

--
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/