Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1544620 > unrolled thread

Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by virtual address

Started byBorislav Petkov <bp@alien8.de>
First post2016-12-19 16:40 +0100
Last post2016-12-21 11:00 +0100
Articles 14 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-19 16:40 +0100
    Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-19 17:50 +0100
      Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-19 19:10 +0100
        Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-19 19:20 +0100
          Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-19 19:50 +0100
            Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-20 00:40 +0100
              Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-20 02:30 +0100
                Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-20 02:50 +0100
                  Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-20 15:50 +0100
                    Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-20 20:30 +0100
                      Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-20 20:40 +0100
                        Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-20 23:50 +0100
                          Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by  virtual address Borislav Petkov <bp@alien8.de> - 2016-12-21 00:00 +0100
                    [tip:x86/urgent] x86/microcode/AMD: Reload proper initrd start  address tip-bot for Borislav Petkov <tipbot@zytor.com> - 2016-12-21 11:00 +0100

#1544620 — Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by virtual address

FromBorislav Petkov <bp@alien8.de>
Date2016-12-19 16:40 +0100
SubjectRe: [PATCH] x86/microcode: Adjust ramdisk address when accessing by virtual address
Message-ID<sQ8cG-1hT-25@gated-at.bofh.it>
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.

[toc] | [next] | [standalone]

#1544665

FromBorislav Petkov <bp@alien8.de>
Date2016-12-19 17:50 +0100
Message-ID<sQ9ip-1WS-5@gated-at.bofh.it>
In reply to#1544620
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.

[toc] | [prev] | [next] | [standalone]

#1544722

FromBorislav Petkov <bp@alien8.de>
Date2016-12-19 19:10 +0100
Message-ID<sQaxP-2TJ-11@gated-at.bofh.it>
In reply to#1544665
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.

[toc] | [prev] | [next] | [standalone]

#1544724

FromBoris Ostrovsky <boris.ostrovsky@oracle.com>
Date2016-12-19 19:20 +0100
Message-ID<sQaHv-2WP-9@gated-at.bofh.it>
In reply to#1544722
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

[toc] | [prev] | [next] | [standalone]

#1544738

FromBorislav Petkov <bp@alien8.de>
Date2016-12-19 19:50 +0100
Message-ID<sQbax-36y-11@gated-at.bofh.it>
In reply to#1544724
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.

[toc] | [prev] | [next] | [standalone]

#1544824

FromBorislav Petkov <bp@alien8.de>
Date2016-12-20 00:40 +0100
Message-ID<sQfHb-605-21@gated-at.bofh.it>
In reply to#1544738
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.

[toc] | [prev] | [next] | [standalone]

#1544850

FromBoris Ostrovsky <boris.ostrovsky@oracle.com>
Date2016-12-20 02:30 +0100
Message-ID<sQhpD-7c7-3@gated-at.bofh.it>
In reply to#1544824

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

[toc] | [prev] | [next] | [standalone]

#1544858

FromBoris Ostrovsky <boris.ostrovsky@oracle.com>
Date2016-12-20 02:50 +0100
Message-ID<sQhJ0-7iB-13@gated-at.bofh.it>
In reply to#1544850

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

[toc] | [prev] | [next] | [standalone]

#1545175

FromBorislav Petkov <bp@alien8.de>
Date2016-12-20 15:50 +0100
Message-ID<sQtTP-6Qf-13@gated-at.bofh.it>
In reply to#1544858
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 <bp@suse.de>
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 <boris.ostrovsky@oracle.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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.

[toc] | [prev] | [next] | [standalone]

#1545430

FromBoris Ostrovsky <boris.ostrovsky@oracle.com>
Date2016-12-20 20:30 +0100
Message-ID<sQygU-1i9-77@gated-at.bofh.it>
In reply to#1545175
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 <boris.ostrovsky@oracle.com>

[toc] | [prev] | [next] | [standalone]

#1545432

FromBorislav Petkov <bp@alien8.de>
Date2016-12-20 20:40 +0100
Message-ID<sQyqu-1lw-17@gated-at.bofh.it>
In reply to#1545430
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.

[toc] | [prev] | [next] | [standalone]

#1545520

FromBoris Ostrovsky <boris.ostrovsky@oracle.com>
Date2016-12-20 23:50 +0100
Message-ID<sQBol-3lH-7@gated-at.bofh.it>
In reply to#1545432
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

[toc] | [prev] | [next] | [standalone]

#1545528

FromBorislav Petkov <bp@alien8.de>
Date2016-12-21 00:00 +0100
Message-ID<sQBy7-3pf-15@gated-at.bofh.it>
In reply to#1545520
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.

[toc] | [prev] | [next] | [standalone]

#1545687 — [tip:x86/urgent] x86/microcode/AMD: Reload proper initrd start address

Fromtip-bot for Borislav Petkov <tipbot@zytor.com>
Date2016-12-21 11:00 +0100
Subject[tip:x86/urgent] x86/microcode/AMD: Reload proper initrd start address
Message-ID<sQLQJ-1IF-5@gated-at.bofh.it>
In reply to#1545175
Commit-ID:  8877ebdd3f9a3ffc84c4b67562d257c5f553bc49
Gitweb:     http://git.kernel.org/tip/8877ebdd3f9a3ffc84c4b67562d257c5f553bc49
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Tue, 20 Dec 2016 11:54:30 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <boris.ostrovsky@oracle.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/20161220144012.lc4cwrg6dphqbyqu@pd.tnic
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 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);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web