lkml.org 
[lkml]   [2012]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 1/2] x86, microcode: Sanitize per-cpu microcode reloading interface
    Date
    > From: Borislav Petkov <bp@amd64.org>
    > Date: Tue, 19 Jun 2012 18:03:30 +0200
    > Subject: [PATCH 1/2] x86, microcode: Sanitize per-cpu microcode
    > reloading interface
    > To: X86-ML <x86@kernel.org>
    > Cc: "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
    > Thomas Gleixner <tglx@linutronix.de>, LKML
    > <linux-kernel@vger.kernel.org>, Andreas Herrmann
    > <andreas.herrmann3@amd.com>, Borislav Petkov
    > <borislav.petkov@amd.com>, Henrique de Moraes Holschuh
    > <hmh@hmh.eng.br>, Peter Zijlstra <peterz@infradead.org>,
    > stable@vger.kernel.org
    >
    > From: Borislav Petkov <borislav.petkov@amd.com>
    >
    > Microcode reloading in a per-core manner is a very bad idea for both
    > major x86 vendors. And the thing is, we have such interface with which
    > we can end up with different microcode versions applied on different
    > cores of an otherwise homogeneous wrt (family,model,stepping) system.
    >
    > So turn off the possibility of doing that per core and allow it only
    > system-wide.
    >
    > This is a minimal fix which we'd like to see in stable too thus the
    > more-or-less arbitrary decision to allow system-wide reloading only on
    > the BSP:
    >
    > $ echo 1 > /sys/devices/system/cpu/cpu0/microcode/reload
    > ...
    >
    > and disable the interface on the other cores:
    >
    > $ echo 1 > /sys/devices/system/cpu/cpu23/microcode/reload
    > -bash: echo: write error: Invalid argument
    >
    > Also, allowing the reload only from one CPU (the BSP in
    > that case) doesn't allow the reload procedure to degenerate
    > into an O(n^2) deal when triggering reloads from all
    > /sys/devices/system/cpu/cpuX/microcode/reload sysfs nodes
    > simultaneously.
    >
    > A more generic fix will follow.
    >
    > Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
    > Cc: Peter Zijlstra <peterz@infradead.org>
    > Cc: stable@vger.kernel.org
    > Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
    > ---
    > arch/x86/kernel/microcode_core.c | 26 +++++++++++++++++++-------
    > 1 file changed, 19 insertions(+), 7 deletions(-)
    >
    > diff --git a/arch/x86/kernel/microcode_core.c
    > b/arch/x86/kernel/microcode_core.c
    > index fbdfc6917180..24b852b61be3 100644
    > --- a/arch/x86/kernel/microcode_core.c
    > +++ b/arch/x86/kernel/microcode_core.c
    > @@ -298,19 +298,31 @@ static ssize_t reload_store(struct device *dev,
    > const char *buf, size_t size)
    > {
    > unsigned long val;
    > - int cpu = dev->id;
    > - ssize_t ret = 0;
    > + int cpu;
    > + ssize_t ret = 0, tmp_ret;
    > +
    > + /* allow reload only from the BSP */
    > + if (boot_cpu_data.cpu_index != dev->id)
    > + return -EINVAL;

    With the /sys/devices/system/cpu/microcode/reload interface in your patch 2/2, this will be broken, right? With the new interface, reload_store() can be executed on any cpu or dev. I think you need to remove this check if working with the patch 2/2.

    Thanks.

    -Fenghua


    \
     
     \ /
      Last update: 2012-06-20 02:01    [W:4.690 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site