When searching for microcode in the ramdisk image we need to adjust the
start address after paging has been turned on (in 32-bit mode).
Signed-off-by: Boris Ostrovsky <[email protected]>
---
arch/x86/kernel/cpu/microcode/core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index c4bb2f7..bc8c3345 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -227,8 +227,11 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa)
* Set start only if we have an initrd image. We cannot use initrd_start
* because it is not set that early yet.
*/
- if (size)
+ if (size) {
start = params->hdr.ramdisk_image;
+ if (!use_pa)
+ start += PAGE_OFFSET;
+ }
# else /* CONFIG_X86_64 */
size = (unsigned long)boot_params.ext_ramdisk_size << 32;
--
2.5.5
On Mon, Dec 19, 2016 at 10:32:13AM -0500, Boris Ostrovsky wrote:
> When searching for microcode in the ramdisk image we need to adjust the
> start address after paging has been turned on (in 32-bit mode).
I need more info:
* Is this fixing a real issue?
* how do you reproduce this?
* kernel version
* .config
* initrd you're using
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On 12/19/2016 10:37 AM, Borislav Petkov wrote:
> On Mon, Dec 19, 2016 at 10:32:13AM -0500, Boris Ostrovsky wrote:
>> When searching for microcode in the ramdisk image we need to adjust the
>> start address after paging has been turned on (in 32-bit mode).
> I need more info:
>
> * Is this fixing a real issue?
>
> * how do you reproduce this?
>
> * kernel version
>
> * .config
>
> * initrd you're using
>
> Thanks.
Before you ask --- this is baremetal, from this morning's mainline. Your
series from yesterday is not applied but I don't think it had a fix for
this problem.
config attached. I'll see how I can get you the initrd.
Here is the splat.
[ 1.865226] BUG: unable to handle kernel paging request at 1aae3000
[ 1.871484] IP: find_cpio_data+0x69/0x220
[ 1.875482] *pdpt = 0000000000000000 *pde = 0000000000000000
[ 1.875483]
[ 1.882701] Oops: 0000 [#1] SMP
[ 1.885834] Modules linked in:
[ 1.888883] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.9.0upstream-11751-ge313a9b-dirty #3
[ 1.897215] Hardware name: MSI MS-7680/H61M-P23 (MS-7680), BIOS V17.0
03/14/2011
[ 1.904595] task: f2faf4c0 task.stack: f2fb0000
[ 1.909115] EIP: find_cpio_data+0x69/0x220
[ 1.913201] EFLAGS: 00210212 CPU: 0
[ 1.916682] EAX: 00000025 EBX: 1aae3000 ECX: 00000026 EDX: f2fb1e64
[ 1.922935] ESI: 054fbe4d EDI: 00000000 EBP: f2fb1e90 ESP: f2fb1e00
[ 1.929188] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[ 1.934574] CR0: 80050033 CR2: 1aae3000 CR3: 01b06000 CR4: 000406f0
[ 1.940827] Call Trace:
[ 1.943268] ? kmemleak_alloc+0x3a/0x50
[ 1.947096] ? __d_instantiate+0x30/0xe0
[ 1.951011] find_microcode_in_initrd+0x5e/0x70
[ 1.955529] save_microcode_in_initrd_intel+0x28/0x5c
[ 1.960569] ? debugfs_create_mode_unsafe+0x2e/0x80
[ 1.965435] ? debugfs_create_u64+0x2d/0x40
[ 1.969607] save_microcode_in_initrd+0x20/0x3c
[ 1.974127] do_one_initcall+0x3c/0x160
[ 1.977955] ? centaur_init_mtrr+0x11/0x11
[ 1.982041] ? kernel_init_freeable+0x132/0x1ff
[ 1.986562] ? parse_args+0x245/0x3a0
[ 1.990215] kernel_init_freeable+0x150/0x1ff
[ 1.994562] ? kernel_init_freeable+0x1ff/0x1ff
[ 1.999080] ? rest_init+0x70/0x70
[ 2.002474] kernel_init+0xb/0x100
[ 2.005867] ? schedule_tail_wrapper+0x9/0xc
[ 2.010126] ret_from_fork+0x19/0x24
[ 2.013693] Code: c7 45 e8 00 00 00 00 c7 45 ec 00 00 00 00 66 c7 45
f0 00 00 e8 f9 e5 00 00 83 7d 08 6e 89 45 8c 76 28 8b 5d 94 8d 55 d4 89
55 84 <80> 3b 00 b8 06 00 00 00 8d 7d a0 75 62 83 6d 08 04 83 c3 04 83
[ 2.032443] EIP: find_cpio_data+0x69/0x220 SS:ESP: 0068:f2fb1e00
[ 2.038432] CR2: 000000001aae3000
[ 2.041738] ---[ end trace 36446d1f414ac27d ]---
[ 2.046343] Kernel panic - not syncing: Fatal exception
[ 2.051559] ---[ end Kernel panic - not syncing: Fatal exception
On Mon, Dec 19, 2016 at 11:10:29AM -0500, Boris Ostrovsky wrote:
> config attached. I'll see how I can get you the initrd.
Wait a bit, lemme see if I can repro with my initrd here.
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On Mon, Dec 19, 2016 at 05:40:27PM +0100, Borislav Petkov wrote:
> On Mon, Dec 19, 2016 at 11:10:29AM -0500, Boris Ostrovsky wrote:
> > config attached. I'll see how I can get you the initrd.
>
> Wait a bit, lemme see if I can repro with my initrd here.
Hmm, it boots here (btw, this is with the 4 patches applied).
[ 0.000000] Linux version 4.9.0upstream-11409-gf3302378ed83-dirty (boris@pd) (gcc version 6.2.1 20161124 (Debian 6.2.1-5) ) #1 SMP Mon Dec 19 18:50:17 CET 2016
[ 0.000000] x86/fpu: Legacy x87 FPU detected.
[ 0.000000] e820: BIOS-provided physical RAM map:
...
[ 0.000000] RAMDISK: [mem 0x37845000-0x37c19fff]
[ 0.000000] Allocated new RAMDISK: [mem 0x36470000-0x36844f5e]
[ 0.000000] Move RAMDISK from [mem 0x37845000-0x37c19f5e] to [mem 0x36470000-0x36844f5e]
...
[ 0.243326] smpboot: CPU0: AMD E-350 Processor (family: 0x14, model: 0x1, stepping: 0x0)
[ 0.243637] Performance Events: AMD PMU driver.
...
[ 1.307957] microcode: microcode updated early to new patch_level=0x05000029
[ 1.308212] microcode: CPU0: patch_level=0x05000029
[ 1.308374] microcode: CPU1: patch_level=0x05000029
[ 1.308726] microcode: Microcode Update Driver: v2.2.
$ uname -a
Linux x1 4.9.0upstream-11409-gf3302378ed83-dirty #1 SMP Mon Dec 19 18:50:17 CET 2016 i686 GNU/Linux
So I guess I wanna take a look at the initrd now...
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On 12/19/2016 01:07 PM, Borislav Petkov wrote:
> On Mon, Dec 19, 2016 at 05:40:27PM +0100, Borislav Petkov wrote:
>> On Mon, Dec 19, 2016 at 11:10:29AM -0500, Boris Ostrovsky wrote:
>>> config attached. I'll see how I can get you the initrd.
>> Wait a bit, lemme see if I can repro with my initrd here.
> Hmm, it boots here (btw, this is with the 4 patches applied).
>
> [ 0.000000] Linux version 4.9.0upstream-11409-gf3302378ed83-dirty (boris@pd) (gcc version 6.2.1 20161124 (Debian 6.2.1-5) ) #1 SMP Mon Dec 19 18:50:17 CET 2016
> [ 0.000000] x86/fpu: Legacy x87 FPU detected.
> [ 0.000000] e820: BIOS-provided physical RAM map:
> ...
> [ 0.000000] RAMDISK: [mem 0x37845000-0x37c19fff]
> [ 0.000000] Allocated new RAMDISK: [mem 0x36470000-0x36844f5e]
> [ 0.000000] Move RAMDISK from [mem 0x37845000-0x37c19f5e] to [mem 0x36470000-0x36844f5e]
> ...
> [ 0.243326] smpboot: CPU0: AMD E-350 Processor (family: 0x14, model: 0x1, stepping: 0x0)
IIUIC find_microcode_in_initrd() is called with paging on only on Intel
(which is where I observed it).
> [ 0.243637] Performance Events: AMD PMU driver.
> ...
> [ 1.307957] microcode: microcode updated early to new patch_level=0x05000029
> [ 1.308212] microcode: CPU0: patch_level=0x05000029
> [ 1.308374] microcode: CPU1: patch_level=0x05000029
> [ 1.308726] microcode: Microcode Update Driver: v2.2.
>
> $ uname -a
> Linux x1 4.9.0upstream-11409-gf3302378ed83-dirty #1 SMP Mon Dec 19 18:50:17 CET 2016 i686 GNU/Linux
>
> So I guess I wanna take a look at the initrd now...
It'll have to wait until (my) evening, I'll ping you then.
-boris
On Mon, Dec 19, 2016 at 01:12:25PM -0500, Boris Ostrovsky wrote:
> IIUIC find_microcode_in_initrd() is called with paging on only on Intel
> (which is where I observed it).
Ah, that was an important fact. Yes, I can repro it now.
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On Mon, Dec 19, 2016 at 07:43:40PM +0100, Borislav Petkov wrote:
> On Mon, Dec 19, 2016 at 01:12:25PM -0500, Boris Ostrovsky wrote:
> > IIUIC find_microcode_in_initrd() is called with paging on only on Intel
> > (which is where I observed it).
>
> Ah, that was an important fact. Yes, I can repro it now.
Ok, questions:
* does your guest relocate the ramdisk?
I.e., do you see something like this in dmesg before the splat:
[ 0.000000] RAMDISK: [mem 0x7f84c000-0x7ffcffff]
[ 0.000000] Allocated new RAMDISK: [mem 0x3647a000-0x36bfd9e6]
[ 0.000000] Move RAMDISK from [mem 0x7f84c000-0x7ffcf9e6] to [mem 0x3647a000-0x36bfd9e6]
^^^^^^^^^^^^^^
If not, then I know what happens.
Also, does it work if you change these lines:
if (!use_pa && relocated_ramdisk)
start = initrd_start;
to:
if (!use_pa)
start = initrd_start;
Because if that works, I can actually simplify that function radically.
But more tomorrow.
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On 12/19/2016 06:32 PM, Borislav Petkov wrote:
> On Mon, Dec 19, 2016 at 07:43:40PM +0100, Borislav Petkov wrote:
>> On Mon, Dec 19, 2016 at 01:12:25PM -0500, Boris Ostrovsky wrote:
>>> IIUIC find_microcode_in_initrd() is called with paging on only on Intel
>>> (which is where I observed it).
>>
>> Ah, that was an important fact. Yes, I can repro it now.
>
> Ok, questions:
>
> * does your guest relocate the ramdisk?
This is not a guest. I crashed with baremetal kernel.
>
> I.e., do you see something like this in dmesg before the splat:
>
> [ 0.000000] RAMDISK: [mem 0x7f84c000-0x7ffcffff]
> [ 0.000000] Allocated new RAMDISK: [mem 0x3647a000-0x36bfd9e6]
> [ 0.000000] Move RAMDISK from [mem 0x7f84c000-0x7ffcf9e6] to [mem 0x3647a000-0x36bfd9e6]
> ^^^^^^^^^^^^^^
>
> If not, then I know what happens.
>
> Also, does it work if you change these lines:
>
> if (!use_pa && relocated_ramdisk)
> start = initrd_start;
>
> to:
>
> if (!use_pa)
> start = initrd_start;
Yes, it does.
I also thought it might be better but I haven't gone through the code to
make sure this would always work.
I can run more tests tomorrow if you want.
-boris
>
>
> Because if that works, I can actually simplify that function radically.
>
> But more tomorrow.
>
> Thanks.
>
On 12/19/2016 08:27 PM, Boris Ostrovsky wrote:
>
>
> On 12/19/2016 06:32 PM, Borislav Petkov wrote:
>> On Mon, Dec 19, 2016 at 07:43:40PM +0100, Borislav Petkov wrote:
>>> On Mon, Dec 19, 2016 at 01:12:25PM -0500, Boris Ostrovsky wrote:
>>>> IIUIC find_microcode_in_initrd() is called with paging on only on Intel
>>>> (which is where I observed it).
>>>
>>> Ah, that was an important fact. Yes, I can repro it now.
>>
>> Ok, questions:
>>
>> * does your guest relocate the ramdisk?
>
>
> This is not a guest. I crashed with baremetal kernel.
>
>
>>
>> I.e., do you see something like this in dmesg before the splat:
>>
>> [ 0.000000] RAMDISK: [mem 0x7f84c000-0x7ffcffff]
>> [ 0.000000] Allocated new RAMDISK: [mem 0x3647a000-0x36bfd9e6]
>> [ 0.000000] Move RAMDISK from [mem 0x7f84c000-0x7ffcf9e6] to [mem
>> 0x3647a000-0x36bfd9e6]
>> ^^^^^^^^^^^^^^
Sorry, forgot about this: I see the first line but not the other two (so
the relocation did not occur).
-boris
>>
>> If not, then I know what happens.
>>
>> Also, does it work if you change these lines:
>>
>> if (!use_pa && relocated_ramdisk)
>> start = initrd_start;
>>
>> to:
>>
>> if (!use_pa)
>> start = initrd_start;
>
> Yes, it does.
>
> I also thought it might be better but I haven't gone through the code to
> make sure this would always work.
>
> I can run more tests tomorrow if you want.
>
> -boris
>
>>
>>
>> Because if that works, I can actually simplify that function radically.
>>
>> But more tomorrow.
>>
>> Thanks.
>>
On Mon, Dec 19, 2016 at 08:40:00PM -0500, Boris Ostrovsky wrote:
> Sorry, forgot about this: I see the first line but not the other two (so the
> relocation did not occur).
Good, that's what I expected. Now it makes sense.
So the reason why it didn't trigger here is that my Intel 32-bit .config
always relocates the ramdisk. And that was taken care of. But not the
case where it didn't relocate it. I.e., your case.
The fix below should generalize the situation to *always*
read initrd_start when we're running late, with VAs and after
reserve_initrd() has run and potentially moved the ramdisk.
> > I also thought it might be better but I haven't gone through the code to
> > make sure this would always work.
> >
> > I can run more tests tomorrow if you want.
Yes please. Here's a minimal patch for 4.10. Please run it on your setup
to verify it fixes your issue. It boots fine on my boxes here, FWIW.
---
From: Borislav Petkov <[email protected]>
Date: Tue, 20 Dec 2016 11:54:30 +0100
Subject: [PATCH] x86/microcode/AMD: Reload proper initrd start address
When we switch to virtual addresses and, especially after
reserve_initrd()->relocate_initrd() have run, we have the updated initrd
address in initrd_start. Use initrd_start then instead of the address
which has been passed to us through boot params. (That still gets used
when we're running the very early routines on the BSP).
Reported-by: Boris Ostrovsky <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/kernel/cpu/microcode/core.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index c4bb2f7169f6..2af69d27da62 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -243,14 +243,12 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa)
# endif
/*
- * Did we relocate the ramdisk?
- *
- * So we possibly relocate the ramdisk *after* applying microcode on the
- * BSP so we rely on use_pa (use physical addresses) - even if it is not
- * absolutely correct - to determine whether we've done the ramdisk
- * relocation already.
+ * Fixup the start address: after reserve_initrd() runs, initrd_start
+ * has the virtual address of the beginning of the initrd. It also
+ * possibly relocates the ramdisk. In either case, initrd_start contains
+ * the updated address so use that instead.
*/
- if (!use_pa && relocated_ramdisk)
+ if (!use_pa && initrd_start)
start = initrd_start;
return find_cpio_data(path, (void *)start, size, NULL);
--
2.11.0
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On 12/20/2016 09:40 AM, Borislav Petkov wrote:
>
> Yes please. Here's a minimal patch for 4.10. Please run it on your setup
> to verify it fixes your issue. It boots fine on my boxes here, FWIW.
>
Without your Sunday's patches but with my AMD-related fixes (cpuid and
eq_id=0)
Tested-by: Boris Ostrovsky <[email protected]>
On Tue, Dec 20, 2016 at 02:26:40PM -0500, Boris Ostrovsky wrote:
> Without your Sunday's patches but with my AMD-related fixes (cpuid and
> eq_id=0)
Can you test tip/x86/urgent with my patch ontop please?
This: http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/urgent
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On 12/20/2016 02:31 PM, Borislav Petkov wrote:
> On Tue, Dec 20, 2016 at 02:26:40PM -0500, Boris Ostrovsky wrote:
>> Without your Sunday's patches but with my AMD-related fixes (cpuid and
>> eq_id=0)
> Can you test tip/x86/urgent with my patch ontop please?
>
> This: http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/urgent
>
> Thanks.
>
Both AMD and Intel tests passed. Bare-metal and various guests.
-boris
On Tue, Dec 20, 2016 at 05:48:21PM -0500, Boris Ostrovsky wrote:
> Both AMD and Intel tests passed. Bare-metal and various guests.
Very cool, thanks a lot for testing!
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Commit-ID: 8877ebdd3f9a3ffc84c4b67562d257c5f553bc49
Gitweb: http://git.kernel.org/tip/8877ebdd3f9a3ffc84c4b67562d257c5f553bc49
Author: Borislav Petkov <[email protected]>
AuthorDate: Tue, 20 Dec 2016 11:54:30 +0100
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 21 Dec 2016 10:50:04 +0100
x86/microcode/AMD: Reload proper initrd start address
When we switch to virtual addresses and, especially after
reserve_initrd()->relocate_initrd() have run, we have the updated initrd
address in initrd_start. Use initrd_start then instead of the address
which has been passed to us through boot params. (That still gets used
when we're running the very early routines on the BSP).
Reported-and-tested-by: Boris Ostrovsky <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/cpu/microcode/core.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index c4bb2f7..2af69d2 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -243,14 +243,12 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa)
# endif
/*
- * Did we relocate the ramdisk?
- *
- * So we possibly relocate the ramdisk *after* applying microcode on the
- * BSP so we rely on use_pa (use physical addresses) - even if it is not
- * absolutely correct - to determine whether we've done the ramdisk
- * relocation already.
+ * Fixup the start address: after reserve_initrd() runs, initrd_start
+ * has the virtual address of the beginning of the initrd. It also
+ * possibly relocates the ramdisk. In either case, initrd_start contains
+ * the updated address so use that instead.
*/
- if (!use_pa && relocated_ramdisk)
+ if (!use_pa && initrd_start)
start = initrd_start;
return find_cpio_data(path, (void *)start, size, NULL);