All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Builtin microcode does nothing..
@ 2016-05-20 10:08 Gabriel C
  2016-05-20 10:23 ` Borislav Petkov
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Gabriel C @ 2016-05-20 10:08 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: LKML

> Does it work when you disable CONFIG_BLK_DEV_INITRD in there?

I can test this whne I'm home since I need to change the config a bit.

> I have a patchset which I'm testing right now which should correct some
> warts in builtin microcode, if you want me to, I can push it out for you
> to test too.

Sure just give me an link or tell me where I can find your testing
patchset and I give it a try.


( I think you forgot to CC me )


Regards,

Gabriel C

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-20 10:08 Builtin microcode does nothing Gabriel C
@ 2016-05-20 10:23 ` Borislav Petkov
  2016-05-20 16:03 ` Gabriel C
  2016-05-21  0:20 ` Gabriel C
  2 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2016-05-20 10:23 UTC (permalink / raw)
  To: Gabriel C; +Cc: LKML

On Fri, May 20, 2016 at 12:08:16PM +0200, Gabriel C wrote:
> ( I think you forgot to CC me )

You're in To:

From: Borislav Petkov <bp@suse.de>
To: Gabriel C <nix.or.die@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>
Subject: Re: Builtin microcode does nothing..
Message-ID: <20160520083634.GA594@pd.tnic>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-20 10:08 Builtin microcode does nothing Gabriel C
  2016-05-20 10:23 ` Borislav Petkov
@ 2016-05-20 16:03 ` Gabriel C
  2016-05-21  0:20 ` Gabriel C
  2 siblings, 0 replies; 22+ messages in thread
From: Gabriel C @ 2016-05-20 16:03 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: LKML

2016-05-20 12:08 GMT+02:00 Gabriel C <nix.or.die@gmail.com>:
>> Does it work when you disable CONFIG_BLK_DEV_INITRD in there?
>
> I can test this whne I'm home since I need to change the config a bit.
>
>> I have a patchset which I'm testing right now which should correct some
>> warts in builtin microcode, if you want me to, I can push it out for you
>> to test too.
>
> Sure just give me an link or tell me where I can find your testing
> patchset and I give it a try.
>
>

Some update on that.

( I still did not test with CONFIG_BLK_DEV_INITRD diabled )

I don't have myself an AMD system but someone tested the same config
but with AMD firmware and this works as expected.

...

[    0.545036] microcode: microcode updated early to new patch_level=0x010000c8

....

So it seems just the Intel part of the code does not work.


Regards,

Gabriel C

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-20 10:08 Builtin microcode does nothing Gabriel C
  2016-05-20 10:23 ` Borislav Petkov
  2016-05-20 16:03 ` Gabriel C
@ 2016-05-21  0:20 ` Gabriel C
  2016-05-21  2:59   ` Gabriel C
  2016-05-21  7:45   ` Borislav Petkov
  2 siblings, 2 replies; 22+ messages in thread
From: Gabriel C @ 2016-05-21  0:20 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: LKML

On 20.05.2016 12:08, Gabriel C wrote:

>> Does it work when you disable CONFIG_BLK_DEV_INITRD in there?
> I can test this when I'm home since I need to change the config a bit.

I got to test an kernel without CONFIG_BLK_DEV_INITRD and this way does 
work.

Regards,

Gabriel C

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-21  0:20 ` Gabriel C
@ 2016-05-21  2:59   ` Gabriel C
  2016-05-21  7:51     ` Borislav Petkov
  2016-05-21  7:45   ` Borislav Petkov
  1 sibling, 1 reply; 22+ messages in thread
From: Gabriel C @ 2016-05-21  2:59 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: LKML

On 21.05.2016 02:20, Gabriel C wrote:

> On 20.05.2016 12:08, Gabriel C wrote:
>
>>> Does it work when you disable CONFIG_BLK_DEV_INITRD in there?
>> I can test this when I'm home since I need to change the config a bit.
>
> I got to test an kernel without CONFIG_BLK_DEV_INITRD and this way 
> does work.
>

While that worked I read the code not sure I get this right but:

scan_microcode() has :

...
       if (!size) {
         if (!load_builtin_intel_microcode(&cd))
             return UCODE_ERROR;
     } else {
         cd = find_cpio_data(p, (void *)start, size, &offset);
         if (!cd.data)
             return UCODE_ERROR;
     }

...

So when I get this correctly load_builtin_intel_microcode() only works 
on !initrd ?

Should this check not be the other way around ?

something like :

       cd = find_cpio_data(p, (void *)start, size, &offset);
       if (!cd.data) {
            if (!load_builtin_intel_microcode(&cd))
                  return UCODE_ERROR;
       }


Regards,

Gabriel C

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-21  0:20 ` Gabriel C
  2016-05-21  2:59   ` Gabriel C
@ 2016-05-21  7:45   ` Borislav Petkov
  1 sibling, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2016-05-21  7:45 UTC (permalink / raw)
  To: Gabriel C; +Cc: LKML

On Sat, May 21, 2016 at 02:20:56AM +0200, Gabriel C wrote:
> I got to test an kernel without CONFIG_BLK_DEV_INITRD and this way
> does work.

Good, this confirms what I've been debugging here too.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-21  2:59   ` Gabriel C
@ 2016-05-21  7:51     ` Borislav Petkov
  2016-05-25  9:31       ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2016-05-21  7:51 UTC (permalink / raw)
  To: Gabriel C; +Cc: LKML

On Sat, May 21, 2016 at 04:59:15AM +0200, Gabriel C wrote:
> Should this check not be the other way around ?

Actually, I've changed it to this:

        /* try built-in microcode first */
        if (load_builtin_intel_microcode(&cd))
                /*
                 * clear start as we might've gotten an initrd too supplied by
                 * the boot loader, by mistake or simply forgotten there. That's
                 * fine, we ignore it since we've found builtin microcode.
                 */
                start = 0;
        else {
#ifdef CONFIG_BLK_DEV_INITRD
                static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";
                char *p = IS_ENABLED(CONFIG_X86_32) ? (char *)__pa_nodebug(ucode_name)
                                                    : ucode_name;

                cd = find_cpio_data(p, (void *)start, size, NULL);
                if (!cd.data)
#endif
                        return UCODE_ERROR;
        }

so we're trying the built-in microcode first.

Why, you ask?

Well, I'm following the intention here - the user has gone the length
and has explicitly configured builtin microcode into the kernel and
therefore we're looking for it first.

If we don't find it, we fall back to initrd. This way, even if you have
an initrd forgotten in the bootloader, we ignore it.

I'll ping you once I'm done testing here.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-21  7:51     ` Borislav Petkov
@ 2016-05-25  9:31       ` Borislav Petkov
  2016-05-25 13:48         ` Jim Bos
  2016-05-25 21:29         ` Gabriel C
  0 siblings, 2 replies; 22+ messages in thread
From: Borislav Petkov @ 2016-05-25  9:31 UTC (permalink / raw)
  To: Gabriel C, Jim Bos; +Cc: LKML

On Sat, May 21, 2016 at 09:51:18AM +0200, Borislav Petkov wrote:
> I'll ping you once I'm done testing here.

Ok, I've just uploaded a branch, it passes testing here.

http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git, branch tip-microcode

@Jim, I'd appreciate it if you ran it again, if you get a chance, to
confirm everything is still ok.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25  9:31       ` Borislav Petkov
@ 2016-05-25 13:48         ` Jim Bos
  2016-05-25 14:57           ` Borislav Petkov
  2016-05-25 21:29         ` Gabriel C
  1 sibling, 1 reply; 22+ messages in thread
From: Jim Bos @ 2016-05-25 13:48 UTC (permalink / raw)
  To: Borislav Petkov, Gabriel C; +Cc: LKML

On 05/25/2016 11:31 AM, Borislav Petkov wrote:
> On Sat, May 21, 2016 at 09:51:18AM +0200, Borislav Petkov wrote:
>> I'll ping you once I'm done testing here.
> 
> Ok, I've just uploaded a branch, it passes testing here.
> 
> http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git, branch tip-microcode
> 
> @Jim, I'd appreciate it if you ran it again, if you get a chance, to
> confirm everything is still ok.
> 
> Thanks.
> 

Works fine here:

- no initrd.gz (simply all needed drivers builtin the main kernel)
- however, CONFIG_BLK_DEV_INITRD=y to keep option for initrd image
- intel ucode built-in kernel image

It also survives suspend/resume S2RAM cycle.

Thanks,

_
Jim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25 13:48         ` Jim Bos
@ 2016-05-25 14:57           ` Borislav Petkov
  0 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2016-05-25 14:57 UTC (permalink / raw)
  To: Jim Bos; +Cc: Gabriel C, LKML

On Wed, May 25, 2016 at 03:48:51PM +0200, Jim Bos wrote:
> Works fine here:
> 
> - no initrd.gz (simply all needed drivers builtin the main kernel)
> - however, CONFIG_BLK_DEV_INITRD=y to keep option for initrd image
> - intel ucode built-in kernel image
> 
> It also survives suspend/resume S2RAM cycle.

Very nice, thanks for testing again!

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25  9:31       ` Borislav Petkov
  2016-05-25 13:48         ` Jim Bos
@ 2016-05-25 21:29         ` Gabriel C
  2016-05-25 21:38           ` Borislav Petkov
  1 sibling, 1 reply; 22+ messages in thread
From: Gabriel C @ 2016-05-25 21:29 UTC (permalink / raw)
  To: Borislav Petkov, Jim Bos; +Cc: LKML

On 25.05.2016 11:31, Borislav Petkov wrote:

> On Sat, May 21, 2016 at 09:51:18AM +0200, Borislav Petkov wrote:
>> I'll ping you once I'm done testing here.
> Ok, I've just uploaded a branch, it passes testing here.
>
> http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git, branch tip-microcode

  I tested your branch on top 4.6 stable.

  initrd + buitl-in intel ucode won't boot here. It hangs on grub 
'Loading initrd image'..

  I'll build an kernel with your patches on my other box later / 
tomorrow just to be sure..

  I didn't tested without initrd since that worked anyway...

Regards,

Garbiel C

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25 21:29         ` Gabriel C
@ 2016-05-25 21:38           ` Borislav Petkov
  2016-05-25 21:50             ` Gabriel C
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2016-05-25 21:38 UTC (permalink / raw)
  To: Gabriel C; +Cc: Jim Bos, LKML

On Wed, May 25, 2016 at 11:29:03PM +0200, Gabriel C wrote:
>  initrd + buitl-in intel ucode won't boot here. It hangs on grub 'Loading
> initrd image'..

That's with my patches? Can I have the .config please so that I can
reproduce?

With initrd you mean, you supply an initrd with the micrcode with grub
*and* have the same microcode builtin into the kernel, correct?

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25 21:38           ` Borislav Petkov
@ 2016-05-25 21:50             ` Gabriel C
  2016-05-25 23:36               ` Gabriel C
  0 siblings, 1 reply; 22+ messages in thread
From: Gabriel C @ 2016-05-25 21:50 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Jim Bos, LKML



On 25.05.2016 23:38, Borislav Petkov wrote:
> On Wed, May 25, 2016 at 11:29:03PM +0200, Gabriel C wrote:
>>   initrd + buitl-in intel ucode won't boot here. It hangs on grub 'Loading
>> initrd image'..
> That's with my patches? Can I have the .config please so that I can
> reproduce?
>
> With initrd you mean, you supply an initrd with the micrcode with grub
> *and* have the same microcode builtin into the kernel, correct?

  No is a initrd without microcode with grub just built-in one.

  config is same like in my first email.

  http://crazy.dnshome.de/~crazy/lkml/config/config

I build an kernel on my oder box in a bit and let you know .. this 
server I tested on is sometimes strange.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25 21:50             ` Gabriel C
@ 2016-05-25 23:36               ` Gabriel C
  2016-05-26 10:03                 ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Gabriel C @ 2016-05-25 23:36 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Jim Bos, LKML

On 25.05.2016 23:50, Gabriel C wrote:

>
> I build an kernel on my oder box in a bit and let you know .. this 
> server I tested on is sometimes strange
  Hangs on the second box too also..

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-25 23:36               ` Gabriel C
@ 2016-05-26 10:03                 ` Borislav Petkov
  2016-05-26 11:52                   ` Gabriel C
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2016-05-26 10:03 UTC (permalink / raw)
  To: Gabriel C; +Cc: Jim Bos, LKML

On Thu, May 26, 2016 at 01:36:51AM +0200, Gabriel C wrote:
>  Hangs on the second box too also..

Ok, please try the diff below ontop to see if it fixes your issue.

Looks like I'd need to rewrite the figuring out where the microcode data
is. Currently, it is ugly and error prone.

Thanks.

---
diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 01c2d14ec05f..4bee5bdbaf2c 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -143,8 +143,13 @@ static inline bool
 get_builtin_firmware(struct cpio_data *cd, const char *name)	{ return false; }
 #endif
 
+static bool initrd_valid;
+
 static inline unsigned long get_initrd_start(void)
 {
+	if (!initrd_valid)
+		return 0;
+
 #ifdef CONFIG_BLK_DEV_INITRD
 	return initrd_start;
 #else
@@ -154,6 +159,9 @@ static inline unsigned long get_initrd_start(void)
 
 static inline unsigned long get_initrd_start_addr(void)
 {
+	if (!initrd_valid)
+		return 0;
+
 #ifdef CONFIG_BLK_DEV_INITRD
 #ifdef CONFIG_X86_32
 	unsigned long *initrd_start_p = (unsigned long *)__pa_nodebug(&initrd_start);
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 43f7caff4749..7ed06c397e2b 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -55,6 +55,8 @@ static struct mc_saved_data {
 	struct microcode_intel **mc_saved;
 } mc_saved_data;
 
+static bool initrd_valid;
+
 /* Go through saved patches and find the one suitable for the current CPU. */
 static enum ucode_state
 find_microcode_patch(struct microcode_intel **saved,
@@ -533,12 +535,10 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
 
 static __init enum ucode_state
 scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
-	       unsigned long *start, unsigned long size,
+	       unsigned long start, unsigned long size,
 	       struct ucode_cpu_info *uci)
 {
-	struct cpio_data cd;
-	cd.data = NULL;
-	cd.size = 0;
+	struct cpio_data cd = { NULL, 0, "" };
 
 	/* try built-in microcode first */
 	if (load_builtin_intel_microcode(&cd))
@@ -547,21 +547,23 @@ scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
 		 * the boot loader, by mistake or simply forgotten there. That's
 		 * fine, we ignore it since we've found builtin microcode.
 		 */
-		*start = 0;
+		initrd_valid = false;
 	else {
 #ifdef CONFIG_BLK_DEV_INITRD
 		static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";
 		char *p = IS_ENABLED(CONFIG_X86_32) ? (char *)__pa_nodebug(ucode_name)
 						    : ucode_name;
 
-		cd = find_cpio_data(p, (void *)*start, size, NULL);
-		if (!cd.data)
+		cd = find_cpio_data(p, (void *)start, size, NULL);
+		if (cd.data)
+			initrd_valid = true;
+		else
 #endif
 			return UCODE_ERROR;
 	}
 
-	return get_matching_model_microcode(*start, cd.data, cd.size,
-					    mcs, mc_ptrs, uci);
+	return get_matching_model_microcode(initrd_valid ? start : 0,
+					    cd.data, cd.size, mcs, mc_ptrs, uci);
 }
 
 /*
@@ -703,20 +705,16 @@ static void __init
 _load_ucode_intel_bsp(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
 		      unsigned long start, unsigned long size)
 {
-	unsigned long _start = start;
 	struct ucode_cpu_info uci;
 	enum ucode_state ret;
 
 	collect_cpu_info_early(&uci);
 
-	ret = scan_microcode(mcs, mc_ptrs, &_start, size, &uci);
+	ret = scan_microcode(mcs, mc_ptrs, start, size, &uci);
 	if (ret != UCODE_OK)
 		return;
 
-	/* Pass updated starting address of blobs to the next routine.  */
-	start = _start;
-
-	ret = load_microcode(mcs, mc_ptrs, start, &uci);
+	ret = load_microcode(mcs, mc_ptrs, initrd_valid ? start : 0, &uci);
 	if (ret != UCODE_OK)
 		return;
 


-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-26 10:03                 ` Borislav Petkov
@ 2016-05-26 11:52                   ` Gabriel C
  2016-05-26 12:46                     ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Gabriel C @ 2016-05-26 11:52 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Jim Bos, LKML


On 26.05.2016 12:03, Borislav Petkov wrote:
> On Thu, May 26, 2016 at 01:36:51AM +0200, Gabriel C wrote:
>>   Hangs on the second box too also..
> Ok, please try the diff below ontop to see if it fixes your issue.
>
> Looks like I'd need to rewrite the figuring out where the microcode data
> is. Currently, it is ugly and error prone.

  With this patch ontop your your tip brach all is fine.
  Tested on both server and both are up and running.


>
> ---
> diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
> index 01c2d14ec05f..4bee5bdbaf2c 100644
> --- a/arch/x86/include/asm/microcode.h
> +++ b/arch/x86/include/asm/microcode.h
> @@ -143,8 +143,13 @@ static inline bool
>   get_builtin_firmware(struct cpio_data *cd, const char *name)	{ return false; }
>   #endif
>   
> +static bool initrd_valid;
> +
>   static inline unsigned long get_initrd_start(void)
>   {
> +	if (!initrd_valid)
> +		return 0;
> +
>   #ifdef CONFIG_BLK_DEV_INITRD
>   	return initrd_start;
>   #else
> @@ -154,6 +159,9 @@ static inline unsigned long get_initrd_start(void)
>   
>   static inline unsigned long get_initrd_start_addr(void)
>   {
> +	if (!initrd_valid)
> +		return 0;
> +
>   #ifdef CONFIG_BLK_DEV_INITRD
>   #ifdef CONFIG_X86_32
>   	unsigned long *initrd_start_p = (unsigned long *)__pa_nodebug(&initrd_start);
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 43f7caff4749..7ed06c397e2b 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -55,6 +55,8 @@ static struct mc_saved_data {
>   	struct microcode_intel **mc_saved;
>   } mc_saved_data;
>   
> +static bool initrd_valid;
> +
>   /* Go through saved patches and find the one suitable for the current CPU. */
>   static enum ucode_state
>   find_microcode_patch(struct microcode_intel **saved,
> @@ -533,12 +535,10 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>   
>   static __init enum ucode_state
>   scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
> -	       unsigned long *start, unsigned long size,
> +	       unsigned long start, unsigned long size,
>   	       struct ucode_cpu_info *uci)
>   {
> -	struct cpio_data cd;
> -	cd.data = NULL;
> -	cd.size = 0;
> +	struct cpio_data cd = { NULL, 0, "" };
>   
>   	/* try built-in microcode first */
>   	if (load_builtin_intel_microcode(&cd))
> @@ -547,21 +547,23 @@ scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
>   		 * the boot loader, by mistake or simply forgotten there. That's
>   		 * fine, we ignore it since we've found builtin microcode.
>   		 */
> -		*start = 0;
> +		initrd_valid = false;
>   	else {
>   #ifdef CONFIG_BLK_DEV_INITRD
>   		static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";
>   		char *p = IS_ENABLED(CONFIG_X86_32) ? (char *)__pa_nodebug(ucode_name)
>   						    : ucode_name;
>   
> -		cd = find_cpio_data(p, (void *)*start, size, NULL);
> -		if (!cd.data)
> +		cd = find_cpio_data(p, (void *)start, size, NULL);
> +		if (cd.data)
> +			initrd_valid = true;
> +		else
>   #endif
>   			return UCODE_ERROR;
>   	}
>   
> -	return get_matching_model_microcode(*start, cd.data, cd.size,
> -					    mcs, mc_ptrs, uci);
> +	return get_matching_model_microcode(initrd_valid ? start : 0,
> +					    cd.data, cd.size, mcs, mc_ptrs, uci);
>   }
>   
>   /*
> @@ -703,20 +705,16 @@ static void __init
>   _load_ucode_intel_bsp(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
>   		      unsigned long start, unsigned long size)
>   {
> -	unsigned long _start = start;
>   	struct ucode_cpu_info uci;
>   	enum ucode_state ret;
>   
>   	collect_cpu_info_early(&uci);
>   
> -	ret = scan_microcode(mcs, mc_ptrs, &_start, size, &uci);
> +	ret = scan_microcode(mcs, mc_ptrs, start, size, &uci);
>   	if (ret != UCODE_OK)
>   		return;
>   
> -	/* Pass updated starting address of blobs to the next routine.  */
> -	start = _start;
> -
> -	ret = load_microcode(mcs, mc_ptrs, start, &uci);
> +	ret = load_microcode(mcs, mc_ptrs, initrd_valid ? start : 0, &uci);
>   	if (ret != UCODE_OK)
>   		return;
>   
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-26 11:52                   ` Gabriel C
@ 2016-05-26 12:46                     ` Borislav Petkov
  2016-06-03  8:02                       ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2016-05-26 12:46 UTC (permalink / raw)
  To: Gabriel C; +Cc: Jim Bos, LKML

On Thu, May 26, 2016 at 01:52:22PM +0200, Gabriel C wrote:
>  With this patch ontop your your tip brach all is fine.
>  Tested on both server and both are up and running.

Thanks.

In the meantime, I've simplified the code even more and testing here
with your .config looks good. I'll prepare another patchset next week
and post it here in case you guys feel bored and want to test it too.

:-)

Thanks again!

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-05-26 12:46                     ` Borislav Petkov
@ 2016-06-03  8:02                       ` Borislav Petkov
  2016-06-04  0:37                         ` Gabriel C
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2016-06-03  8:02 UTC (permalink / raw)
  To: Gabriel C, Jim Bos; +Cc: LKML

On Thu, May 26, 2016 at 02:46:06PM +0200, Borislav Petkov wrote:
> In the meantime, I've simplified the code even more and testing here
> with your .config looks good. I'll prepare another patchset next week
> and post it here in case you guys feel bored and want to test it too.

Ok, here's a new version, it passes testing here but we've heard that
before :-\

http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git/log/?h=tip-microcode

If you guys could give it a quick run again, it'll be much appreciated.

Thanks!

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-06-03  8:02                       ` Borislav Petkov
@ 2016-06-04  0:37                         ` Gabriel C
  2016-06-04  6:35                           ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Gabriel C @ 2016-06-04  0:37 UTC (permalink / raw)
  To: Borislav Petkov, Jim Bos; +Cc: LKML

On 03.06.2016 10:02, Borislav Petkov wrote:

> On Thu, May 26, 2016 at 02:46:06PM +0200, Borislav Petkov wrote:
>> In the meantime, I've simplified the code even more and testing here
>> with your .config looks good. I'll prepare another patchset next week
>> and post it here in case you guys feel bored and want to test it too.
> Ok, here's a new version, it passes testing here but we've heard that
> before :-\
>
> http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git/log/?h=tip-microcode
>
> If you guys could give it a quick run again, it'll be much appreciated.

  I tested your patches on both boxes on top 4.7-rc1
  and they work just fine for me.

  The test was done as before:

  CONFIG_FIRMWARE_IN_KERNEL=y
  CONFIG_EXTRA_FIRMWARE="intel-ucode/...." ( to match my CPU )
  CONFIG_BLK_DEV_INITRD=y

  Kernel booted with initrd *without* grub microcode.

  If you wish any other tests please let me know.

  Regards

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
  2016-06-04  0:37                         ` Gabriel C
@ 2016-06-04  6:35                           ` Borislav Petkov
  0 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2016-06-04  6:35 UTC (permalink / raw)
  To: Gabriel C; +Cc: Jim Bos, LKML

On Sat, Jun 04, 2016 at 02:37:11AM +0200, Gabriel C wrote:
>  I tested your patches on both boxes on top 4.7-rc1
>  and they work just fine for me.
> 
>  The test was done as before:
> 
>  CONFIG_FIRMWARE_IN_KERNEL=y
>  CONFIG_EXTRA_FIRMWARE="intel-ucode/...." ( to match my CPU )
>  CONFIG_BLK_DEV_INITRD=y
> 
>  Kernel booted with initrd *without* grub microcode.

Good.

>  If you wish any other tests please let me know.

Sure, will do.

But thanks for reporting and testing, much appreciated!

I'll add your Tested-by: tag.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Builtin microcode does nothing..
       [not found] <CAEJqkgjFWak5fUc_zR4v-BDgAnjU0NtRfUDDPaEVGjjbEZoqUQ@mail.gmail.com>
@ 2016-05-20  8:36 ` Borislav Petkov
  0 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2016-05-20  8:36 UTC (permalink / raw)
  To: Gabriel C; +Cc: LKML, Ingo Molnar, Thomas Gleixner

On Fri, May 20, 2016 at 02:34:34AM +0200, Gabriel C wrote:
> With this kernel the early microcode loading does nothing nor the 'old
> interface'.

Does it work when you disable CONFIG_BLK_DEV_INITRD in there?

I have a patchset which I'm testing right now which should correct some
warts in builtin microcode, if you want me to, I can push it out for you
to test too.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Builtin microcode does nothing
@ 2016-05-20  0:55 Gabriel C
  0 siblings, 0 replies; 22+ messages in thread
From: Gabriel C @ 2016-05-20  0:55 UTC (permalink / raw)
  To: LKML; +Cc: Borislav Petkov, Ingo Molnar, Thomas Gleixner

Hi guys ,

I tested kernel 4.6 with the built in microcode method , following
Documentation/x86/early-microcode.txt.

The kernel config[1] has :


CONFIG_FIRMWARE_IN_KERNEL=y
CONFIG_EXTRA_FIRMWARE="intel-ucode/06-1a-05"
CONFIG_EXTRA_FIRMWARE_DIR="/lib/firmware"


also

CONFIG_MICROCODE=y
CONFIG_MICROCODE_INTEL=y
CONFIG_MICROCODE_AMD=y
CONFIG_MICROCODE_OLD_INTERFACE=y

I also use a basic initrd generated with dracut.

With this kernel the early microcode loading does nothing nor the 'old
interface'.

Interesting is the following :

doing a ' echo 1 >/sys/devices/system/cpu/microcode/reload ' the kernel
finds the buitin firmware and updates the microcode.

...

[  713.273956] platform microcode: firmware: using built-in firmware
intel-ucode/06-1a-05
[  713.274024] microcode: CPU0 sig=0x106a5, pf=0x1, revision=0x15
[  713.274596] microcode: CPU0 updated to revision 0x19, date = 2013-06-21

...

Also the method with the 'combined initrd' works just fine.

[1] http://crazy.dnshome.de/~crazy/lkml/config/config

Please CC me since I'm not subscribed to the list.


Best Regards,

Gabriel C

( sorry but gmail messed up so I resend )

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-06-04  6:35 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-20 10:08 Builtin microcode does nothing Gabriel C
2016-05-20 10:23 ` Borislav Petkov
2016-05-20 16:03 ` Gabriel C
2016-05-21  0:20 ` Gabriel C
2016-05-21  2:59   ` Gabriel C
2016-05-21  7:51     ` Borislav Petkov
2016-05-25  9:31       ` Borislav Petkov
2016-05-25 13:48         ` Jim Bos
2016-05-25 14:57           ` Borislav Petkov
2016-05-25 21:29         ` Gabriel C
2016-05-25 21:38           ` Borislav Petkov
2016-05-25 21:50             ` Gabriel C
2016-05-25 23:36               ` Gabriel C
2016-05-26 10:03                 ` Borislav Petkov
2016-05-26 11:52                   ` Gabriel C
2016-05-26 12:46                     ` Borislav Petkov
2016-06-03  8:02                       ` Borislav Petkov
2016-06-04  0:37                         ` Gabriel C
2016-06-04  6:35                           ` Borislav Petkov
2016-05-21  7:45   ` Borislav Petkov
     [not found] <CAEJqkgjFWak5fUc_zR4v-BDgAnjU0NtRfUDDPaEVGjjbEZoqUQ@mail.gmail.com>
2016-05-20  8:36 ` Borislav Petkov
  -- strict thread matches above, loose matches on Subject: below --
2016-05-20  0:55 Gabriel C

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.