Patchwork disabling microcode update

login
register
about
Submitter xdrudis
Date 2011-02-26 01:38:49
Message ID <20110226013849.GA5330@ideafix.casa.ct>
Download mbox | patch
Permalink /patch/2704/
State New
Headers show

Comments

xdrudis - 2011-02-26 01:38:49
This is the patch for option B. 

You may not be able to test it without my next patch. At least for me 
selectiong EXPERT in make menuconfig breaks the build. Next patch fixes it.
Alexandru Gagniuc - 2011-02-26 02:01:56
On 02/26/2011 03:38 AM, xdrudis wrote:
> This is the patch for option B. 
> 
> You may not be able to test it without my next patch. At least for me 
> selectiong EXPERT in make menuconfig breaks the build. Next patch fixes it.
> 
> 
I don't like the wording for the help option. It creates the impression
that the entire reason for this option is purely philosophical.
I accepted your argument for this option because you quoted practical
reasons, such as the updated microcode causing problems, and thus I
would prefer a help text focusing on those reasons. That is not to say
to avoid the philosophical issue altogether, but not invoke it as the
main reason.

Other than that, pretty good job :)

Alex
xdrudis - 2011-02-26 20:19:36
On Sat, Feb 26, 2011 at 04:01:56AM +0200, Alex G. wrote:
> On 02/26/2011 03:38 AM, xdrudis wrote:
> > This is the patch for option B. 
> > 
> > You may not be able to test it without my next patch. At least for me 
> > selectiong EXPERT in make menuconfig breaks the build. Next patch fixes it.
> > 
> > 

> I don't like the wording for the help option. 

Stefan Reinauer didn't like it either and I'm not sure he does now.  I
got no suggestions (alternative wordings) so I changed only the
particulars he pointed to.  Feel free to improve it.

[note: this pretty much summarised the other 120 lines, in case
you don't read it all]

> It creates the impression
> that the entire reason for this option is purely philosophical.

Not having the source, documentation and specific expertise 
is a philosophical problem but not only 
a philosophical problem. One of the consequences is that I can
not give any satisfactory practical reasons for updating or not
updating microcode. All I could write would be 

          Select this to apply non-free 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). 

          The microcode patches are selected from mainboard Kconfig
          AMD_UCODE_PATCH_FILE among the
          src/cpu/amd/model_10xxx/mc_*.h files provided by AMD. We
          don't know the issues solved by each microcode patch file,
          the issues created by each file (possibly solved in other
          patches present in or absent from coreboot) or the
          preconditions to apply them to your particular CPU or to the
          set of CPUs the image you're configuring may run on.  We
          know the intent of the patches is to solve issues and that
          we've had systems running well with the patches and issues
          also solved by not applying them.
          
          Unselect to let FAM10 CPUs run with the unpatched microcode
          as shipped from factory.  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.
          
But since Stefan didn't like "IANAL", I bet he won't like this either.
And maybe others here know more than what I just wrote, or it's
published somewhere, it's just that for philosophical reasons, and the
practical reason that it seems to work without updating microcode for
me, I'm not interested in investigating the microcode details, and I
suspect as much of Ivaylo.  Maybe Ivaylo and I picked the wrong mc*.h
file, maybe the right one isn't there, maybe the checks before the
microcode update are somehow wrong, maybe our CPU is newer than the
mc*.h files and it somehow fools the checks, maybe it's just wrong to
have AMD_UCODE_PATCH_FILE as a mainboard option, and a rutime check
should select, for each processor, beetwen all microcode patches
available, so that a coreboot image could run on any CPU revision you
can put on that board and socket. But I can't get past "maybe".

> I accepted your argument for this option because you quoted practical
> reasons, such as the updated microcode causing problems, and thus I
> would prefer a help text focusing on those reasons.

The practical reasons are that not updating microcode is a shortcut
through a few unknowns, and it may work better or worse than updating it
(certainly better for me an Ivaylo, but I don't write the help for us).
Since it has been previously stated that kconfig help shouldn't be qualified
with uncertainity valuations but just express correct facts to the best
of our knowledge, I'm unable to express the practical reasons without
hinting at uncertain facts. 

The only certain facts are that in some cases it works better without
updating and in some (many?) others it works updating. I think too that
in some cases it doesn't work without updating, but I don't have a
concrete case to show (no fool has tried?). I fail to see how saying
this can help.  It could maybe help to specify the details of when it
works updating or when it works not updating, but since I don't have
source, documentation or expertise, I cannot guess what are the
circumstances that cause it, I could only give every complete setup
for each collection of cases, which seems too detailed and verbose.
Should I say ASUS M4A77TD-PRO with Phenom X4 910e or rev RB_C3 in SVI
or DDR3 with AM3 ?
 
> That is not to say
> to avoid the philosophical issue altogether, but not invoke it as the
> main reason.
> 

I think you overestimate the philosophical nature of practical reasons 
such as a simpler license compliance or working with a source backed code
base. Of course they carry philosophical questions of no small importance
and even epistemological questions here noted, but less legal trouble
and less unknowns are practical reasons as well. 

> Other than that, pretty good job :)
> 

Thank you.

I hope Stefan and Peter agree with you, because I don't. For me it looks
uglier each time. 

In fact this is starting to feel like design by committee. I think I'm
doing something without further researching some parts because I'm not
interested, and, although I wouldn't have bothered the list without
practical reasons, I'm more motivated for legal peace of mind and
software freedom than for the practical reasons, so I'm relunctant to
hide it or to attribute it to practical reasons. And you (and possibly
others) are trying to accept a new option because you want to be nice,
sympathise with some general philosophy, and are concerned with
accomodating all use cases but you think it will hardly ever work so
you want it hidden, explained and with warnings. 

In a way we both stand in a quasi contradictory position each and are
trying to reconcile these different positions. If I liked this sort of
bargaining I'd send a resumé to the European Council. I'm all for peer
review (no offense intended calling you my peers) and throwing more
eyeballs at things, and discussing stuff until we're all convinced, but
I've run out of time and motivation, and i don't really find I have
much more to offer.

If anyone wants to take it from here you can refine and commit one
of these patches (option A or B), or come up with something new, 
but it will take someone more aligned with the general sentiment
than myself, to really code something that makes sense to herself and
later can be made acceptable to all. So I'll leave it here.

Thanks to all for your attention and sorry for not being able of more.
Alexandru Gagniuc - 2011-02-26 21:22:17
On 02/26/2011 10:19 PM, xdrudis wrote:
> On Sat, Feb 26, 2011 at 04:01:56AM +0200, Alex G. wrote:
>> On 02/26/2011 03:38 AM, xdrudis wrote:
>>> This is the patch for option B. 
>>>
>>> You may not be able to test it without my next patch. At least for me 
>>> selectiong EXPERT in make menuconfig breaks the build. Next patch fixes it.
>>>
>>>
> 
>> I don't like the wording for the help option. 
> 
> Stefan Reinauer didn't like it either and I'm not sure he does now.  I
> got no suggestions (alternative wordings) so I changed only the
> particulars he pointed to.  Feel free to improve it.
> 
> [note: this pretty much summarised the other 120 lines, in case
> you don't read it all]
> 
>> It creates the impression
>> that the entire reason for this option is purely philosophical.
> 
> Not having the source, documentation and specific expertise 
> is a philosophical problem but not only 
> a philosophical problem. One of the consequences is that I can
> not give any satisfactory practical reasons for updating or not
> updating microcode.
>
I look at the microcode as simply DIP switches used to configure the IRQ
line on the hardware. If the manual (microcode updates) gives me
erroneous information, then I put the switches back to their initial
position (factory microcode). You will disagree and say that, as long as
it can be updated, and source code exists for it, it is software, not
hardware.

> All I could write would be 
> 
>           Select this to apply non-free 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). 
> 
>           The microcode patches are selected from mainboard Kconfig
>           AMD_UCODE_PATCH_FILE among the
>           src/cpu/amd/model_10xxx/mc_*.h files provided by AMD. We
>           don't know the issues solved by each microcode patch file,
>           the issues created by each file (possibly solved in other
>           patches present in or absent from coreboot) or the
>           preconditions to apply them to your particular CPU or to the
>           set of CPUs the image you're configuring may run on.  We
>           know the intent of the patches is to solve issues and that
>           we've had systems running well with the patches and issues
>           also solved by not applying them.
>           
>           Unselect to let FAM10 CPUs run with the unpatched microcode
>           as shipped from factory.  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.
>           
How about something simple, such as

        Unselect this option if you do not wish coreboot to update the
        CPU microcode, or if you experience  issues arising from
        updating the microcode.

        Note that microcode updates are designed to fix issues and bugs
        in the CPU. Also most operating systems will update the
        automatically, so you may still end up running with an updated
        microcode.

	You may, at your option, select to disable microcode updating
        if you believe it to be in violation with your views on software
        freedom.

        If unsure, select y.

This is accurate to the best of my knowleddge.

> But since Stefan didn't like "IANAL", I bet he won't like this either.

IANAL, while a fact, is irrelevant in a help menu. Your profession, or
mine for that reason, has no relevance in the context of deciding
whether or not to disallow microcode updates.

> And maybe others here know more than what I just wrote, or it's
> published somewhere, it's just that for philosophical reasons, and the
> practical reason that it seems to work without updating microcode for
> me, I'm not interested in investigating the microcode details, and I
> suspect as much of Ivaylo.  Maybe Ivaylo and I picked the wrong mc*.h
> file, maybe the right one isn't there, maybe the checks before the
> microcode update are somehow wrong, maybe our CPU is newer than the
> mc*.h files and it somehow fools the checks, maybe it's just wrong to
> have AMD_UCODE_PATCH_FILE as a mainboard option, and a rutime check
> should select, for each processor, beetwen all microcode patches
> available, so that a coreboot image could run on any CPU revision you
> can put on that board and socket. But I can't get past "maybe".
> 
We want coreboot to be practical. The "maybe" can fill up a text file
larger than the coreboot source. We don't care. If disabling microcode
updates works for you, that is sufficient reason to consider this option

>> That is not to say
>> to avoid the philosophical issue altogether, but not invoke it as the
>> main reason.
>>
> 
> I think you overestimate the philosophical nature of practical reasons 
> such as a simpler license compliance or working with a source backed code
> base. Of course they carry philosophical questions of no small importance
> and even epistemological questions here noted, but less legal trouble
> and less unknowns are practical reasons as well. 
> 
And yet we still do update the microcode for the majority of users, and
we even ship it to you when you download the coreboot source. There is
practicality arising from thought alone, only giving you the choice to
exire from the complications the rest of us subject ourselves to, if any.

>> Other than that, pretty good job :)
>>
> 
> Thank you.
> 
> I hope Stefan and Peter agree with you, because I don't. For me it looks
> uglier each time. 
> 
> In fact this is starting to feel like design by committee. I think I'm
> doing something without further researching some parts because I'm not
> interested, and, although I wouldn't have bothered the list without
> practical reasons, I'm more motivated for legal peace of mind and
> software freedom than for the practical reasons, so I'm relunctant to
> hide it or to attribute it to practical reasons. And you (and possibly
> others) are trying to accept a new option because you want to be nice,
> sympathise with some general philosophy, and are concerned with
> accomodating all use cases but you think it will hardly ever work so
> you want it hidden, explained and with warnings. 
> 
> In a way we both stand in a quasi contradictory position each and are
> trying to reconcile these different positions. If I liked this sort of
> bargaining I'd send a resumé to the European Council. I'm all for peer
> review (no offense intended calling you my peers) and throwing more
> eyeballs at things, and discussing stuff until we're all convinced, but
> I've run out of time and motivation, and i don't really find I have
> much more to offer.
> 
> If anyone wants to take it from here you can refine and commit one
> of these patches (option A or B), or come up with something new, 
> but it will take someone more aligned with the general sentiment
> than myself, to really code something that makes sense to herself and
> later can be made acceptable to all. So I'll leave it here.
> 
> Thanks to all for your attention and sorry for not being able of more. 
>  
It almost an unwritten law ,not just in coreboot, but in anythat ones
first patch will not get accepted without at least a revision. It is a
thoughtless initiation rite, an unconscious demonstration of power and
masculinity from those veteran enough to afford it. It is also a way to
ensure succession over generations: to get you thinking more deeply
about the code you modified, to understand it better, to inescapably see
it in your dreams, so that when you wake up, you will improve it more,
all in an attempt to get YOU to continue contributing. It is an instinct
of survival.

I'm not one of the core developers. I'm just a vigilante patching a
broken society. I'm actually incredibly virgin to the project, only
having joined last month. I get put through the same treatment, and I do
not find offense in it. I am not told that my work is broken, but _why_.
This _why_ is the only way to advance the ranks to the point of ensuring
heredity to the project. Whether I will continue to chase lawless
criminals, invest countless sleepless hours, and abstain from basic
nutrition in the desire to aquire the hardware necessary to do so, is
unbeknownst to me. And I do not care. I make the best of the time I
spend _now_ .

Shortening the server-kneeling spree of characters, you should not quit
because of points of view differing, or the mud getting too deep. You
should only quit if you feel you lost interest, or rather, if you stop
feeling any interest. Whether you are laying down your arms from
exhaustion or your country has more appealing issues at hand, only you
can decide.

Alex
xdrudis - 2011-02-26 22:46:30
On Sat, Feb 26, 2011 at 11:22:17PM +0200, Alex G. wrote:

> I look at the microcode as simply DIP switches used to configure the IRQ
> line on the hardware. If the manual (microcode updates) gives me
> erroneous information, then I put the switches back to their initial
> position (factory microcode). You will disagree and say that, as long as
> it can be updated, and source code exists for it, it is software, not
> hardware.
> 

Let's leave this aside. If it was a picture of something nice instead 
of microcode you would still have legal complications depending on the license.
When you include a work in another you create a derivative work and 
need permission from the copyright holders of both. No one asks you what
the works are (ok, yes, they do, but that's for details).

You can translate that to ethical terms about using the work of others
respecting their conditions if you wish. 

> How about something simple, such as
> 
>         Unselect this option if you do not wish coreboot to update the
>         CPU microcode, or if you experience  issues arising from
>         updating the microcode.
>

Shouldn't we tell something about how to tell an issue arises from 
updating the microcode? I can't. Other than trying to disable it.
 
>         Note that microcode updates are designed to fix issues and bugs
>         in the CPU. Also most operating systems will update the
>         automatically, so you may still end up running with an updated
>         microcode.
>

Good
 
> 	You may, at your option, select to disable microcode updating
>         if you believe it to be in violation with your views on software
>         freedom.
> 
>         If unsure, select y.
> 
> This is accurate to the best of my knowleddge.
> 

I think it misses three facts:

- the issues arising from the microcode updates have been observed
  (are not hypothetical). But this is not giving much information either.

- the license of the microcode patches is not free (how can you believe
it to be in violation of freedom if you're not told this?)

- coreboot does not include all microcode updates ever produced by the
manufacturer (my intention is to be fair if some issue is not due to
something in the microcode patch but to misapplication by coreboot or
lack of fresher updates).

But I don't care. I can accept your text. It's an expert option, shouldn't
need a perfect help.

> IANAL, while a fact, is irrelevant in a help menu. Your profession, or
> mine for that reason, has no relevance in the context of deciding
> whether or not to disallow microcode updates.
>

If one reason is legal complications, and who says it is not a lawyer,
it's relevant. I think the objections where more about : there's no "I"
in a Kconfig file that can be lawyer or not, and uncertainity is to be
avoided because it has an unprofessional look and is perceived as 
complicating usage .

> We want coreboot to be practical. The "maybe" can fill up a text file
> larger than the coreboot source. We don't care. If disabling microcode
> updates works for you, that is sufficient reason to consider this option
>

Certainly. I wasn't pretending to put that text in the help, only explaining
why I couldn't put anything useful.
 
> And yet we still do update the microcode for the majority of users, 

This has ethical consequences but might be made legally simpler by
reading microcode patches from external files.

> and
> we even ship it to you when you download the coreboot source. 

in the source it is more an agregation that a derived work. It's compiling
that makes the derived work. Or maybe I'm confused.

> There is
> practicality arising from thought alone, only giving you the choice to
> exire from the complications the rest of us subject ourselves to, if any.
>

I didn't understand, sorry.
 

> It almost an unwritten law ,not just in coreboot, but in anythat ones
> first patch will not get accepted without at least a revision. It is a

I hope no patch is accepted without a revision, first or
otherwise. This is not exclusive of coreboot, and it is good policy.

> because of points of view differing, or the mud getting too deep. You
> should only quit if you feel you lost interest, or rather, if you stop
> feeling any interest. Whether you are laying down your arms from

Right, I'm not interested in refining this patch further [nor in
power, masculinity, initiation rites, survival by programming firmware
or many other things].
  
It's also nice to quit when you feel you cannot offer anything useful.
We only live so long.
Alexandru Gagniuc - 2011-02-26 23:30:19
On 02/27/2011 12:46 AM, xdrudis wrote:
> On Sat, Feb 26, 2011 at 11:22:17PM +0200, Alex G. wrote:
> 
>> I look at the microcode as simply DIP switches used to configure the IRQ
>> line on the hardware. If the manual (microcode updates) gives me
>> erroneous information, then I put the switches back to their initial
>> position (factory microcode). You will disagree and say that, as long as
>> it can be updated, and source code exists for it, it is software, not
>> hardware.
>>
> 
> Let's leave this aside. If it was a picture of something nice instead 
> of microcode you would still have legal complications depending on the license.
> When you include a work in another you create a derivative work and 
> need permission from the copyright holders of both. No one asks you what
> the works are (ok, yes, they do, but that's for details).
> 
No, I'm not leaving it aside. The microcode you wish to disable (and
IIRC you, Peter, Stepan, and I agree to providing this choice via an
elegant Kconfig option) was provided by AMD, and they have given us
permission to use it in our code, with our license. In fact, they
provided the (coreboot) patches themselves. The developers provided
permission for the microcode to be used with coreboot, by committing it.

> You can translate that to ethical terms about using the work of others
> respecting their conditions if you wish. 
>
And I just detailed how this applies here.

>> How about something simple, such as
>>
>>         Unselect this option if you do not wish coreboot to update the
>>         CPU microcode, or if you experience  issues arising from
>>         updating the microcode.
>>
> 
> Shouldn't we tell something about how to tell an issue arises from 
> updating the microcode? I can't. Other than trying to disable it.
>  
We should, but at the moment, we don't have enough information. It's a
try before you buy issue.

>>         Note that microcode updates are designed to fix issues and bugs
>>         in the CPU. Also most operating systems will update the
>>         automatically, so you may still end up running with an updated
>>         microcode.
>>
> 
> Good
>  
>> 	You may, at your option, select to disable microcode updating
>>         if you believe it to be in violation with your views on software
>>         freedom.
>>
>>         If unsure, select y.
>>
>> This is accurate to the best of my knowleddge.
>>
> 
> I think it misses three facts:
> 
> - the issues arising from the microcode updates have been observed
>   (are not hypothetical). But this is not giving much information either.
> 
I would argue that "if you experience  issues arising from updating the
microcode" implies someone has already hit this, but alright, let's
change it to:


        Unselect this option if you do not wish coreboot to update the
        CPU microcode. Some persons have experienced problems with
        updating the microcode that were solved when the update was
        disabled. If you believe you may be experiencing an issue
        related to updating the microcode, unselect this option. There
        is currently little information available on the effects of
        this.

> - the license of the microcode patches is not free (how can you believe
> it to be in violation of freedom if you're not told this?)
> 
Third paragraph leaves you, the user, to decide this. We are not the
FSF, we cannot decide for others.

> - coreboot does not include all microcode updates ever produced by the
> manufacturer (my intention is to be fair if some issue is not due to
> something in the microcode patch but to misapplication by coreboot or
> lack of fresher updates).
> 
Are you expecting  coreboot to include them all?
> But I don't care. I can accept your text. It's an expert option, shouldn't
> need a perfect help.
Of course you're not. It's an expert option. If you tackle with this
menu, you already know it tries to include the latest microcode
available, not all of them, right?

>> IANAL, while a fact, is irrelevant in a help menu. Your profession, or
>> mine for that reason, has no relevance in the context of deciding
>> whether or not to disallow microcode updates.
>>
> 
> If one reason is legal complications, and who says it is not a lawyer,
> it's relevant. I think the objections where more about : there's no "I"
> in a Kconfig file that can be lawyer or not, and uncertainity is to be
> avoided because it has an unprofessional look and is perceived as 
> complicating usage .
> 
And that is exactly what I meant.

>> We want coreboot to be practical. The "maybe" can fill up a text file
>> larger than the coreboot source. We don't care. If disabling microcode
>> updates works for you, that is sufficient reason to consider this option
>>
> 
> Certainly. I wasn't pretending to put that text in the help, only explaining
> why I couldn't put anything useful.
>  
The text was the only objection I had, and I hoped we were working to
improve it

>> And yet we still do update the microcode for the majority of users, 
> 
> This has ethical consequences but might be made legally simpler by
> reading microcode patches from external files.
> 
Agreed entirely, but as you claim to be lazy and only do it for Fam10
CPUs, so de we, and do not write the infrastructure code to support this.

>> and
>> we even ship it to you when you download the coreboot source. 
> 
> in the source it is more an agregation that a derived work. It's compiling
> that makes the derived work. Or maybe I'm confused.
>
As trunk stands at the moment of this writing, coreboot cannot be
compiled without the microcode, and thus you may very well argue it is a
derived work. Your patch tries to change that, and we agree it is the
better direction.

>> There is
>> practicality arising from thought alone, only giving you the choice to
>> exire from the complications the rest of us subject ourselves to, if any.
>>
> 
> I didn't understand, sorry.
>  
I meant that I, as a user who compiles with the microcode, expose myself
to the legal issues of including bytecode in a GPL work. You can choose
not to do so by disabling the microcode update.

>> It almost an unwritten law ,not just in coreboot, but in anythat ones
>> first patch will not get accepted without at least a revision. It is a
> 
> I hope no patch is accepted without a revision, first or
> otherwise. This is not exclusive of coreboot, and it is good policy.
> 
It is a fact of life, not a policy. We do not artificially, or at least
consciously try to make things hard for newcomers.

>> because of points of view differing, or the mud getting too deep. You
>> should only quit if you feel you lost interest, or rather, if you stop
>> feeling any interest. Whether you are laying down your arms from
> 
> Right, I'm not interested in refining this patch further [nor in
> power, masculinity, initiation rites, survival by programming firmware
> or many other things].
>   
I respect your choice.

> It's also nice to quit when you feel you cannot offer anything useful.
> We only live so long.

You have already given us something of extreme value: the impulse to
start considering this issue, the chance to discuss this. You have given
us an idea, which is worth more than a thousand patches. Ideas can exist
without patches, but not the other way around.

I wish you good luck in whatever you may choose to do next, and I want
you to know that we will still be here (hopefully with an open mind)
should you ever need our help. :)

All the best,
Alex
Alexandru Gagniuc - 2011-02-26 23:30:46
On 02/27/2011 01:30 AM, Peter Stuge wrote:
> xdrudis wrote:
>> This is the patch for option B. 
> 
> Thanks!
> 
> 
>> Make patching cpu microcode optional (for experts).
>>
>> Signed-off-by: Xavi Drudis Ferran <xdrudis@tinet.cat>
> 
> Acked-by: Peter Stuge <peter@stuge.se>
> 
> Committed as r6385 with some whitespace changes and reworded the help
> message to try to summarize the discussion.
> 
Thanks. I like the wording very much.

Alex
Peter Stuge - 2011-02-26 23:30:57
xdrudis wrote:
> This is the patch for option B. 

Thanks!


> Make patching cpu microcode optional (for experts).
> 
> Signed-off-by: Xavi Drudis Ferran <xdrudis@tinet.cat>

Acked-by: Peter Stuge <peter@stuge.se>

Committed as r6385 with some whitespace changes and reworded the help
message to try to summarize the discussion.


//Peter
xdrudis - 2011-02-27 09:57:46
On Sun, Feb 27, 2011 at 01:30:19AM +0200, Alex G. wrote:
> On 02/27/2011 12:46 AM, xdrudis wrote:
> > On Sat, Feb 26, 2011 at 11:22:17PM +0200, Alex G. wrote:
>>>  You will disagree and say that, as long as
>>> it can be updated, and source code exists for it, it is software, not
>>> hardware.
> > 
> > Let's leave this aside. If it was a picture of something nice instead 
> > of microcode you would still have legal complications depending on the license.
> > When you include a work in another you create a derivative work and 
> > need permission from the copyright holders of both. No one asks you what
> > the works are (ok, yes, they do, but that's for details).
> > 
> No, I'm not leaving it aside. 

Yes, you are. Please read yourself. Your argument revolves on licenses not
the nature of the copyrighted work. I left aside whether it's software 
or hardware.

> The microcode you wish to disable (and
> IIRC you, Peter, Stepan, and I agree to providing this choice via an
> elegant Kconfig option) was provided by AMD, and they have given us
> permission to use it in our code, with our license. In fact, they
> provided the (coreboot) patches themselves. The developers provided
> permission for the microcode to be used with coreboot, by committing it.
> 

The issue is not the rights of the microcode authors. The issue is the
rights of author A of code C that licensed her code under GPL and
contributor T incorporated to coreboot. A never gave permission to
link C with the microcode (both because it does not have source, and
even if it had because its license does not grant the 4 freedoms).
It's A who can sue, not AMD. (A==T is possible but the sign-off might
be interpreted differently).

Again, IANAL, people report lawyers have ok'ed this, implicit licenses
in sign-off are complicated, A may not be aware or willing to sue, 
risk mitigation is a personal choice , etc.,etc.,etc. I'm not trying
to scare people off. Just clarifying the issue, then each one may judge.

Thanks Peter, Rudolf and all for the review and commit. 
Good summary of the discussion in the help, Peter.

Patch

Make patching cpu microcode optional (for experts).
It's been requested to not link update_microcode.c in that case, 
and therefore we have to comment all uses. 

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

---

Please apply with -p 1 

--- coreboot-r6380/src/cpu/amd/model_10xxx/init_cpus.c	2011-02-25 23:54:12.000000000 +0100
+++ coreboot-disupducode/src/cpu/amd/model_10xxx/init_cpus.c	2011-02-26 01:46:19.000000000 +0100
@@ -325,7 +325,9 @@ 
 		 * This happens after HTinit.
 		 * The BSP runs this code in it's own path.
 		 */
+#if CONFIG_UPDATE_CPU_MICROCODE == 1
 		update_microcode(cpuid_eax(1));
+#endif
 		cpuSetAMDMSR();
 
 #if CONFIG_SET_FIDVID
diff -ru coreboot-r6380/src/cpu/amd/model_10xxx/Kconfig coreboot-disupducode/src/cpu/amd/model_10xxx/Kconfig
--- coreboot-r6380/src/cpu/amd/model_10xxx/Kconfig	2011-02-25 23:54:12.000000000 +0100
+++ coreboot-disupducode/src/cpu/amd/model_10xxx/Kconfig	2011-02-26 00:59:20.000000000 +0100
@@ -50,3 +50,26 @@ 
 
 endif
 endif
+
+config UPDATE_CPU_MICROCODE
+       bool
+       default y
+
+config UPDATE_CPU_MICROCODE
+        bool "Update cpu microcode"
+        default y
+        depends on EXPERT && CPU_AMD_MODEL_10XXX
+        help
+          Select this to apply non-free 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 the unpatched microcode
+          as shipped from factory.  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.
+          
+
diff -ru coreboot-r6380/src/cpu/amd/model_10xxx/Makefile.inc coreboot-disupducode/src/cpu/amd/model_10xxx/Makefile.inc
--- coreboot-r6380/src/cpu/amd/model_10xxx/Makefile.inc	2011-02-25 23:54:12.000000000 +0100
+++ coreboot-disupducode/src/cpu/amd/model_10xxx/Makefile.inc	2011-02-26 00:04:56.000000000 +0100
@@ -1,5 +1,4 @@ 
-# no conditionals here. If you include this file from a socket, then you get all the binaries.
 driver-y += model_10xxx_init.c
-ramstage-y += update_microcode.c
+ramstage-$(CONFIG_UPDATE_CPU_MICROCODE) += update_microcode.c
 ramstage-y += apic_timer.c
 ramstage-y += processor_name.c
diff -ru coreboot-r6380/src/mainboard/amd/bimini_fam10/romstage.c coreboot-disupducode/src/mainboard/amd/bimini_fam10/romstage.c
--- coreboot-r6380/src/mainboard/amd/bimini_fam10/romstage.c	2011-02-25 23:54:27.000000000 +0100
+++ coreboot-disupducode/src/mainboard/amd/bimini_fam10/romstage.c	2011-02-26 00:14:15.000000000 +0100
@@ -66,7 +66,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -132,7 +136,10 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/amd/mahogany_fam10/romstage.c coreboot-disupducode/src/mainboard/amd/mahogany_fam10/romstage.c
--- coreboot-r6380/src/mainboard/amd/mahogany_fam10/romstage.c	2011-02-25 23:54:27.000000000 +0100
+++ coreboot-disupducode/src/mainboard/amd/mahogany_fam10/romstage.c	2011-02-26 00:15:29.000000000 +0100
@@ -66,7 +66,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include "southbridge/amd/sb700/early_setup.c"
@@ -125,7 +129,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/amd/serengeti_cheetah_fam10/romstage.c coreboot-disupducode/src/mainboard/amd/serengeti_cheetah_fam10/romstage.c
--- coreboot-r6380/src/mainboard/amd/serengeti_cheetah_fam10/romstage.c	2011-02-25 23:54:27.000000000 +0100
+++ coreboot-disupducode/src/mainboard/amd/serengeti_cheetah_fam10/romstage.c	2011-02-26 00:16:25.000000000 +0100
@@ -87,7 +87,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -227,7 +231,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/amd/tilapia_fam10/romstage.c coreboot-disupducode/src/mainboard/amd/tilapia_fam10/romstage.c
--- coreboot-r6380/src/mainboard/amd/tilapia_fam10/romstage.c	2011-02-25 23:54:27.000000000 +0100
+++ coreboot-disupducode/src/mainboard/amd/tilapia_fam10/romstage.c	2011-02-26 00:14:55.000000000 +0100
@@ -65,7 +65,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -124,7 +128,10 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/asus/m4a785-m/romstage.c coreboot-disupducode/src/mainboard/asus/m4a785-m/romstage.c
--- coreboot-r6380/src/mainboard/asus/m4a785-m/romstage.c	2011-02-25 23:54:23.000000000 +0100
+++ coreboot-disupducode/src/mainboard/asus/m4a785-m/romstage.c	2011-02-26 00:10:18.000000000 +0100
@@ -65,7 +65,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -125,7 +129,10 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/asus/m4a78-em/romstage.c coreboot-disupducode/src/mainboard/asus/m4a78-em/romstage.c
--- coreboot-r6380/src/mainboard/asus/m4a78-em/romstage.c	2011-02-25 23:54:24.000000000 +0100
+++ coreboot-disupducode/src/mainboard/asus/m4a78-em/romstage.c	2011-02-26 00:11:40.000000000 +0100
@@ -65,7 +65,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -125,7 +129,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/gigabyte/ma785gmt/romstage.c coreboot-disupducode/src/mainboard/gigabyte/ma785gmt/romstage.c
--- coreboot-r6380/src/mainboard/gigabyte/ma785gmt/romstage.c	2011-02-25 23:54:23.000000000 +0100
+++ coreboot-disupducode/src/mainboard/gigabyte/ma785gmt/romstage.c	2011-02-26 00:19:20.000000000 +0100
@@ -61,7 +61,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -121,7 +125,10 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/gigabyte/ma78gm/romstage.c coreboot-disupducode/src/mainboard/gigabyte/ma78gm/romstage.c
--- coreboot-r6380/src/mainboard/gigabyte/ma78gm/romstage.c	2011-02-25 23:54:23.000000000 +0100
+++ coreboot-disupducode/src/mainboard/gigabyte/ma78gm/romstage.c	2011-02-26 00:18:33.000000000 +0100
@@ -65,7 +65,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -123,7 +127,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/hp/dl165_g6_fam10/romstage.c coreboot-disupducode/src/mainboard/hp/dl165_g6_fam10/romstage.c
--- coreboot-r6380/src/mainboard/hp/dl165_g6_fam10/romstage.c	2011-02-25 23:54:25.000000000 +0100
+++ coreboot-disupducode/src/mainboard/hp/dl165_g6_fam10/romstage.c	2011-02-26 00:09:06.000000000 +0100
@@ -82,7 +82,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -136,7 +140,10 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/iei/kino-780am2-fam10/romstage.c coreboot-disupducode/src/mainboard/iei/kino-780am2-fam10/romstage.c
--- coreboot-r6380/src/mainboard/iei/kino-780am2-fam10/romstage.c	2011-02-25 23:54:26.000000000 +0100
+++ coreboot-disupducode/src/mainboard/iei/kino-780am2-fam10/romstage.c	2011-02-26 00:19:45.000000000 +0100
@@ -67,7 +67,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -126,7 +130,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/jetway/pa78vm5/romstage.c coreboot-disupducode/src/mainboard/jetway/pa78vm5/romstage.c
--- coreboot-r6380/src/mainboard/jetway/pa78vm5/romstage.c	2011-02-25 23:54:26.000000000 +0100
+++ coreboot-disupducode/src/mainboard/jetway/pa78vm5/romstage.c	2011-02-26 00:09:34.000000000 +0100
@@ -72,7 +72,11 @@ 
 #include "cpu/amd/quadcore/quadcore.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 #include <spd.h>
@@ -131,7 +135,10 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/msi/ms9652_fam10/romstage.c coreboot-disupducode/src/mainboard/msi/ms9652_fam10/romstage.c
--- coreboot-r6380/src/mainboard/msi/ms9652_fam10/romstage.c	2011-02-25 23:54:23.000000000 +0100
+++ coreboot-disupducode/src/mainboard/msi/ms9652_fam10/romstage.c	2011-02-26 00:13:49.000000000 +0100
@@ -76,7 +76,11 @@ 
 #include "southbridge/nvidia/mcp55/early_setup_car.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -153,7 +157,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/supermicro/h8dmr_fam10/romstage.c coreboot-disupducode/src/mainboard/supermicro/h8dmr_fam10/romstage.c
--- coreboot-r6380/src/mainboard/supermicro/h8dmr_fam10/romstage.c	2011-02-25 23:54:25.000000000 +0100
+++ coreboot-disupducode/src/mainboard/supermicro/h8dmr_fam10/romstage.c	2011-02-26 00:18:07.000000000 +0100
@@ -68,7 +68,11 @@ 
 #include "southbridge/nvidia/mcp55/early_setup_car.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -145,7 +149,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/supermicro/h8qme_fam10/romstage.c coreboot-disupducode/src/mainboard/supermicro/h8qme_fam10/romstage.c
--- coreboot-r6380/src/mainboard/supermicro/h8qme_fam10/romstage.c	2011-02-25 23:54:25.000000000 +0100
+++ coreboot-disupducode/src/mainboard/supermicro/h8qme_fam10/romstage.c	2011-02-26 00:17:34.000000000 +0100
@@ -74,7 +74,11 @@ 
 #include "southbridge/nvidia/mcp55/early_setup_car.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -197,7 +201,11 @@ 
  /* Setup sysinfo defaults */
  set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
  update_microcode(val);
+#endif
+
  post_code(0x33);
 
  cpuSetAMDMSR();
diff -ru coreboot-r6380/src/mainboard/tyan/s2912_fam10/romstage.c coreboot-disupducode/src/mainboard/tyan/s2912_fam10/romstage.c
--- coreboot-r6380/src/mainboard/tyan/s2912_fam10/romstage.c	2011-02-25 23:54:28.000000000 +0100
+++ coreboot-disupducode/src/mainboard/tyan/s2912_fam10/romstage.c	2011-02-26 00:13:09.000000000 +0100
@@ -77,7 +77,11 @@ 
 #include "southbridge/nvidia/mcp55/early_setup_car.c"
 #include "cpu/amd/car/post_cache_as_ram.c"
 #include "cpu/amd/microcode/microcode.c"
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 #include "cpu/amd/model_10xxx/update_microcode.c"
+#endif
+
 #include "cpu/amd/model_10xxx/init_cpus.c"
 #include "northbridge/amd/amdfam10/early_ht.c"
 
@@ -153,7 +157,11 @@ 
 	/* Setup sysinfo defaults */
 	set_sysinfo_in_ram(0);
 
+
+#if CONFIG_UPDATE_CPU_MICROCODE==1
 	update_microcode(val);
+#endif
+
 	post_code(0x33);
 
 	cpuSetAMDMSR();