Messages in this thread | | | From | "Yu, Fenghua" <> | Subject | RE: [PATCH 1/2] x86, microcode: Sanitize per-cpu microcode reloading interface | Date | Tue, 19 Jun 2012 23:15:56 +0000 |
| |
> 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
| |