[coreboot] [PATCH] disabling microcode update

[Xavi Drudis Ferran]

On Tue 22/02/11 23:58 , "Alex G." <mr.nuke.me at gmail.com> wrote:

> On 02/22/2011 02:47 AM, Peter Stuge wrote:
> > Xavi Drudis Ferran wrote:
> >> Does everyone prefer to have it not include update_microcode.c and
> >> change romstage.c in those boards that call update_microcode(...) ?
> > 
> > At least I like this better. It makes it clear what effect this
> > option has for mainboard code.
> > 
> If this option ever existed, I want it to stay out of my way entirely. I
> don't even want to know it exists without explicitly searching for it. I
> don't want to ever be in the situation where I find out that my board
> froze because the microcode update was disabled, and I don't want
> newcomers to waste their time asking about that pompous sounding
>
> "Disable Microcode update" option in menuconfig.
>

Pompous ? 

If you look at the patch the option name is 
+config UPDATE_CPU_MICROCODE
+	bool "Update cpu microcode"
+	default y

and it's "y" by default, so you won't get your microcode update disabled unless 
all these happen: 
- you have a FAM 10 cpu
- you make menuconfig (or similar)
- you go to the chipset menu and under title CPU unselect the preselected option "Update cpu microcode"

If that is too visible for you I don't know how to build an option that different users 
may enable or disable. 

If you mean visible in the source code: 

Option A:
the patch I sent changes only one .c file and one Kconfig file. The function update_microcode()
would still exist and it would do the update or do nothing depending on CONFIG_UPDATE_CPU_MICROCODE.
No other files would need touching.
This is what my software engineer intuition recommends, centralise changes in one place, change 
the behaviour of a procedure depending on configuration options, don't change every single place
that procedure is called. 

But people have considered this to be too little visible, or too little  invasive for its heretic nature, 
or I don't quite understand what, and they've asked 

Option B: 

change src/cpu/amd/model_10xxx/Makefile.inc 
so that update_microcode.c does not get compiled in. when CONFIG_UPDATE_CPU_MICROCODE is false
since in that case the function update_microcode(...) would be undefined, we would have to also 
change all files with a call to it, by placing an #if around the call. Those files are 
src/cpu/amd/model_10xxx/init-cpus.c
src/mainboard/*/*/romstage.c
(and by */* I mean all or some of the fam 10 boards, not all boards).

And I can even imagine another 

Option C: 

change update_microcode.c so that when CONFIG_AMD_UCODE_PATCH_FILE is defined
to be an empty string or 0 or something, then mc_*.h  does not get 
included into update_microcode.c, the update_microcode function does
not find microcode to update and it ends up doing nothing. 

I don't think this is desirable because then this would be a mainboard
option, not a user option, and you might need different mainboard dirs 
for different CPU revisions or just for different user choices.   So I won't 
send a patch for this. But someone might.

If I understand you I think you would prefer option A, so that the change is not
so visible in source code, but maybe you want C because you don't want to 
see it in make menuconfig at all . I'd appreciate some more detail.



About newcomers complaining because they've disabled microcode update, 
it's not somethign that would happen without user action, and we already 
have newcomers (Ivaylo and I) complaining because the microcode update 
it giving us trouble. We can add a note in Kconfig help like :

"If you unselect this option and have some problem you want to ask 
about in the coreboot mailing list (or elsewhere) please report clearly
you disable microcode update, since some developers strongly advise against this."


> If I _really_ wanted to disable microcode updates, I would simply
> comment out a line of code.
> 

That's what I did ! But I found out there were more people than just myself
wanting this option, so I tried to code a patch that would change nothing 
for those who wanted the old behaviour but would allow to disable microcode
update and keep testing new svn revisions, sending patches, etc. without 
the hurdle of a local modification, for those of us that need it.
 
If I had know how polemic it all would become I've probably hadn't sent
the "pompous" patch at all.

> As far as the licensing discusion gooes, I have thought of this
> intensely. I don't see microcode as violating my freedom. It is
> hardware. modifying microcode modifies the hardware. This is something
> that software cannot do.
> 

Violating licenses is not the same as violating freedom, and freedom from 
lawsuits is also something one mike like. Also being kind to people you 
take code from and considering they might consider hardware as the part
of an information system that you cannot change (excluding people) and  
software as the part that you can change, even if you think they're wrong 
and microcode is hardware, has some ethical value. 

But I won't follow along this route, because we simply don't agree in enough 
categories to agree in this.

> + We are firmware developers, not hardware engineers.
> 

May I remind that the patch does not modifiy the microcode creatively ? 
It just leaves as it was the microcode that the "hardware" engineers 
installed in the factory ? Some people have found that version of the 
"hardware" to work better than the "modified hardware" after microcode
update. 

> I'm not voting no on this, just so as long as it is completely invisible
> to me.
> 

Please specify which of Option A, B, or C do you prefer, or propose something
else, if you don't vote no. I can't code a completely invisible patch to a free software
project that evolves in the open like coreboot.  
Whatever I do you will see it if you look at it.

Thank you.