On Tue 22/02/11 23:58 , "Alex G." <mr.nuke.me(a)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.