On Tue 22/02/11 23:58 , "Alex G." mr.nuke.me@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.
On 02/23/2011 03:51 PM, Xavi Drudis Ferran wrote:
Pompous ?
Yes. This is an option for experienced users, and people too smart for they own sake (pozitive connotation), that value their freedom more than practicality. They will go to an extra effort to ensure that. Therefore, considering the above, an option in menuconfig (gtkconfig, xconfig, etch) is pompous.
If that is too visible for you I don't know how to build an option that different users may enable or disable.
Well, how about I simply need to add a select DISABLE_MICROCODE_UPDATE in my board's Kconfig file?
If you mean visible in the source code:
It will have to appear in the source code for it to work. I'd have to be unreasonable to say "make it invisible in the source code".
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 :
When I decide to dedicate my time to helping someone on IRC who has problems getting coreboot to run, I don't want them to keep me on hold, and then say "I also tried with this 'disable microcode update' option, and it still didn't work". It has happened before with other options, completely irrelevant to their issue.
"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.
That's what I would do _myself_. That isn't to say I don't agree with other options. And yet, I doubt adding a Kconfig line will be much of a hassle.
If I had know how polemic it all would become I've probably hadn't sent the "pompous" patch at all.
Please take no offense in seeing your patch as "pompous". It is not. It is the option in menuconfig that is pompous. The "polemic" is good. If we take the time to digest it now, we will have a precedent to refer to next time it comes up.
But I won't follow along this route, because we simply don't agree in enough categories to agree in this.
If you had asked me about disabling microcode updates a week ago, I would probably have laughed and not even have thought of it. I respect your point of view, and that is why I am _thinking_ about this, and of a way to incorporate it without much noise to both of us (though you could argue that noise has already been generated).
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.
Yes, I will see it if I look for it explicitly. There's nothing I can do about that, and to reject it for this reason would be nothing but malice from my side. Is the 'adding a line in Kconfig' option hassle free enough for you? I just don't see a way to make it obscure enough it menuconfig, but I won't object if you do find one.
Alex
Alex G. wrote:
Is the 'adding a line in Kconfig' option hassle free enough for you? I just don't see a way to make it obscure enough it menuconfig, but I won't object if you do find one.
Don't we have an experts option that will enable further options?
//Peter
On 02/24/2011 12:26 PM, Peter Stuge wrote:
Alex G. wrote:
Is the 'adding a line in Kconfig' option hassle free enough for you? I just don't see a way to make it obscure enough it menuconfig, but I won't object if you do find one.
Don't we have an experts option that will enable further options?
If it's hidden under expert options, I can work with that :) .
Alex
Hi Xavi,
On Wed, Feb 23, 2011 at 02:51:46PM +0100, Xavi Drudis Ferran wrote:
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).
This is the option that Stepan and Peter prefer, so this seems like the best way forward? I'd test and ack this patch.
Thanks! Ward.
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.
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
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.
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
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.
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
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.
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
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
This patch tries to fix compilation when you select EXPERT in make menuconfig.
On 02/26/2011 03:39 AM, xdrudis wrote:
This patch tries to fix compilation when you select EXPERT in make menuconfig.
HT Frequencies are multiples of 200MHz AFAIK, so there are no 300MHz and 500MHz. I'm not sure why the build breaks, and why this fixes it, but I don't think this is the right solution. Someone wiser should comment on this.
Alex
On Sat, Feb 26, 2011 at 03:53:43AM +0200, Alex G. wrote:
On 02/26/2011 03:39 AM, xdrudis wrote:
This patch tries to fix compilation when you select EXPERT in make menuconfig.
HT Frequencies are multiples of 200MHz AFAIK, so there are no 300MHz and 500MHz. I'm not sure why the build breaks, and why this fixes it, but I don't think this is the right solution. Someone wiser should comment on this.
Oh! You may well be right. All others are multiples of 200 MHz .
Then we should maybe remove 2 #elif in the following code. But I wonder whether the break in the progression of values indicates that all values from there on should be changed too.
I have no idea what this does, other than the Kconfig help:
choice prompt "HyperTransport frequency" default LIMIT_HT_SPEED_AUTO help This option sets the maximum permissible HyperTransport link frequency.
Use of this option will only limit the autodetected HT frequency. It will not (and cannot) increase the frequency beyond the autodetected limits.
This is primarily used to work around poorly designed or laid out HT traces on certain motherboards.
The code where it is used is in src/northbridge/amd/amdht/h3finit.c:1330
#if CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_200 cbPCBFreqLimit = 0x0001; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_300 cbPCBFreqLimit = 0x0003; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_400 cbPCBFreqLimit = 0x0007; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_500 cbPCBFreqLimit = 0x000F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_600 cbPCBFreqLimit = 0x001F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_800 cbPCBFreqLimit = 0x003F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1000 cbPCBFreqLimit = 0x007F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1200 cbPCBFreqLimit = 0x00FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1400 cbPCBFreqLimit = 0x01FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1600 cbPCBFreqLimit = 0x03FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1800 cbPCBFreqLimit = 0x07FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2000 cbPCBFreqLimit = 0x0FFF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2200 cbPCBFreqLimit = 0x1FFF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2400 cbPCBFreqLimit = 0x3FFF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2600 cbPCBFreqLimit = 0x7FFF; #else cbPCBFreqLimit = 0xFFFF; // Maximum allowed by autoconfiguration #endif
xdrudis wrote:
HT Frequencies are multiples of 200MHz AFAIK, so there are no 300MHz and 500MHz.
..
Oh! You may well be right. All others are multiples of 200 MHz .
Then we should maybe remove 2 #elif in the following code. But I wonder whether the break in the progression of values indicates that all values from there on should be changed too.
A safer change might be:
#elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_300 #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_500
#elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_300) && CONFIG_LIMIT_HT_SPEED_300 #elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_500) && CONFIG_LIMIT_HT_SPEED_500
I would ack that if it builds.
//Peter
On Sat, Feb 26, 2011 at 07:17:46PM +0100, Peter Stuge wrote:
xdrudis wrote:
HT Frequencies are multiples of 200MHz AFAIK, so there are no 300MHz and 500MHz.
..
Oh! You may well be right. All others are multiples of 200 MHz .
Then we should maybe remove 2 #elif in the following code. But I wonder whether the break in the progression of values indicates that all values from there on should be changed too.
A safer change might be:
#elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_300 #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_500
#elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_300) && CONFIG_LIMIT_HT_SPEED_300 #elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_500) && CONFIG_LIMIT_HT_SPEED_500
I would ack that if it builds.
I'd rather wait to see if someone who understands the code knows whether 300 and 500 make sense (then we define them as in my patch) or they don't (then we eliminate the #elif and maybe modify some values around there). What you propose just defers the real question to Kconfig. Besides, without my patch, I don't think CONFIG_LIMIT_HT_SPEED_300 and CONFIG_LIMIT_HT_SPEED_500 are defined anyware, so you're solution is equivalent to eliminaing the #elif.
The 500MHz and 300MHz are valid HT frequencies
Table 59. Link Frequency Bit Field Encoding [FreqExt, Link Frequency] Transmitter Clock Frequency Encoding (MHz) 0_0000 200 (default) 0_0001 300 0_0010 400 0_0011 500 0_0100 600 0_0101 800 0_0110 1000 0_0111 1200* 0_1000 1400* 0_1001 1600* 0_1010 1800 0_1011 2000 0_1100 2200 0_1101 2400 0_1110 2600 0_1111 Vendor-Specific 1_0000 Reserved 1_0001 2800 1_0010 3000 1_0011 3200 1_0100 to 1_1111 Reserved * These frequencies were defined in revision 2.00 of the specification but are redefined electrically in revision 3.00. The presence of a Gen3 capability block indicates that a device implements the Gen3 electrical behavior for these frequencies. Gen1 and Gen3 devices are not interoperable at these frequencies.
Check http://www.hypertransport.org/docs/twgdocs/HTC20051222-0046-0035.pdf
For details
Thanks, Rudolf
xdrudis wrote:
This patch tries to fix compilation when you select EXPERT in make menuconfig.
..
Signed-off-by: Xavi Drudis Ferran xdrudis@tinet.cat
Acked-by: Peter Stuge peter@stuge.se
r6386
-
Alex G. -
Peter Stuge -
Rudolf Marek -
Ward Vandewege -
Xavi Drudis Ferran -
xdrudis