Patchwork disabling microcode update

login
register
about
Submitter xdrudis
Date 2011-02-19 20:45:50
Message ID <20110219204550.GD4764@ideafix.casa.ct>
Download mbox | patch
Permalink /patch/2683/
State Rejected
Headers show

Comments

xdrudis - 2011-02-19 20:45:50
On Fri, Feb 18, 2011 at 10:19:31AM -0500, Ward Vandewege wrote:
> Hi Xavi,
> 
> On Wed, Feb 16, 2011 at 02:45:02PM +0100, Xavi Drudis Ferran wrote:
> > Should I send a patch making a Kconfig option to not upgrade microcode for fam10? Is there any interest in that ? 
> 
> Yes, please. I would test and ack that. 
> 
> Thanks,
> Ward.
> 

Here it is. It's limited to fam10. I don't know about other families nor can test.

Sorry about having missed the request from Ivaylo, for a moment I
thought I was the only one interested.

It works in my board (but my board still does not boot, so tests in
functional boards are welcome). 

Thanks.
Stefan Reinauer - 2011-02-21 02:58:01
On 2/19/11 12:45 PM, xdrudis wrote:
> On Fri, Feb 18, 2011 at 10:19:31AM -0500, Ward Vandewege wrote:
>> >  Hi Xavi,
>> >  
>> >  On Wed, Feb 16, 2011 at 02:45:02PM +0100, Xavi Drudis Ferran wrote:
>>> >  >  Should I send a patch making a Kconfig option to not upgrade microcode for fam10? Is there any interest in that ?
What's the particular rationale behind this? When disabling microcode 
updates, why don't you also disable the other erratas?


> +          Select this to apply (propietary?) patches to the cpu
I don't think the word proprietary (nor the question mark) applies very 
well here.

> +          microcode provided by AMD to correct issues in the CPU after
> +          production, and distributed with coreboot (not necessarily
> +          the latest microcode version produced by AMD, but only
> +          applied if newer than the version in your CPU).
> +
> +	  Unselect to let FAM10 CPUs run with factory microcode.  If
Here's some formatting problem. It's also unclear what you mean by 
"factory microcode" imho.

> +          you unselect this, no binary microcode patches will be
> +          included in the image, so it will help you get an image
> +          which you have the entire source code for and may simplify
> +          license compliance (IANAL).
I don't see how this makes license compliance any easier. Also, please 
refrain from using terms like IANAL. If we make claims they should be 
either right to the best of our knowledge or we don't put them in the 
source code, anyways.

BTW, some corporations had legal teams analyze the microcode license and 
it was considered not problematic for inclusion in coreboot in the sense 
of the GPL.

> --- src/cpu/amd/model_10xxx/update_microcode.c	2011-02-19 21:56:44.000000000 +0100
> +++ src/cpu/amd/model_10xxx/update_microcode.c	2011-02-19 22:09:17.000000000 +0100
> @@ -29,6 +29,7 @@
>   #include<cpu/amd/microcode.h>
>   #endif
>
> +#if CONFIG_UPDATE_CPU_MICROCODE
>   static const u8 microcode_updates[] __attribute__ ((aligned(16))) = {
>
Please change the patch to not include update_microcode.c in the 
Makefile for the case that microcode selection is disabled.


Stefan

Patch

Allow the user to disable cpu microcode updating 
(only for AMD model_10xxx cpus) in make menuconfig. 
If disabled the microcode is not included in 
update_microcode.c and therefore the generate image
does not include it. 

Signed-off-by: Xavi Drudis Ferran <xdrudis@tinet.cat>

---

I've abuild tested it with default y and default n,
not sure if abuild build with all default options and 
so both cases are tested or I should do something else. 

--- src/cpu/amd/model_10xxx/Kconfig	2011-02-19 21:56:44.000000000 +0100
+++ src/cpu/amd/model_10xxx/Kconfig	2011-02-19 19:48:20.000000000 +0100
@@ -50,3 +50,22 @@ 
 
 endif
 endif
+
+config UPDATE_CPU_MICROCODE
+	bool "Update cpu microcode"
+	default y
+	depends on CPU_AMD_MODEL_10XXX
+        help
+          Select this to apply (propietary?) patches to the cpu
+          microcode provided by AMD to correct issues in the CPU after
+          production, and distributed with coreboot (not necessarily
+          the latest microcode version produced by AMD, but only
+          applied if newer than the version in your CPU).
+
+	  Unselect to let FAM10 CPUs run with factory microcode.  If
+          you unselect this, no binary microcode patches will be
+          included in the image, so it will help you get an image
+          which you have the entire source code for and may simplify
+          license compliance (IANAL).
+          
+
--- src/cpu/amd/model_10xxx/update_microcode.c	2011-02-19 21:56:44.000000000 +0100
+++ src/cpu/amd/model_10xxx/update_microcode.c	2011-02-19 22:09:17.000000000 +0100
@@ -29,6 +29,7 @@ 
 #include <cpu/amd/microcode.h>
 #endif
 
+#if CONFIG_UPDATE_CPU_MICROCODE
 static const u8 microcode_updates[] __attribute__ ((aligned(16))) = {
 
 #ifdef __PRE_RAM__
@@ -93,9 +94,11 @@ 
 	return new_id;
 
 }
+#endif
 
 void update_microcode(u32 cpu_deviceid)
 {
+#if CONFIG_UPDATE_CPU_MICROCODE
 	u32 equivalent_processor_rev_id;
 
 	/* Update the microcode */
@@ -105,5 +108,6 @@ 
 	} else {
 		printk(BIOS_DEBUG, "microcode: rev id not found. Skipping microcode patch!\n");
 	}
+#endif
 
 }