Re: [coreboot] [PATCH] disabling microcode update
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 = 0x;// Maximum allowed by autoconfiguration #endif -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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
Re: [coreboot] [PATCH] disabling microcode update
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. -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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. -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
The 500MHz and 300MHz are valid HT frequencies Table 59. Link Frequency Bit Field Encoding [FreqExt, Link Frequency] Transmitter Clock Frequency Encoding(MHz) 0_ 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_Vendor-Specific 1_ Reserved 1_00012800 1_00103000 1_00113200 1_0100 to 1_ 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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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
Re: [coreboot] [PATCH] disabling microcode update
xdrudis wrote: This is the patch for option B. Thanks! Make patching cpu microcode optional (for experts). Signed-off-by: Xavi Drudis Ferran xdru...@tinet.cat Acked-by: Peter Stuge pe...@stuge.se Committed as r6385 with some whitespace changes and reworded the help message to try to summarize the discussion. //Peter -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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. 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 xdru...@tinet.cat --- Please apply with -p 1 --- coreboot-r6380/src/cpu/amd/model_10xxx/init_cpus.c 2011-02-25 23:54:12.0 +0100 +++ coreboot-disupducode/src/cpu/amd/model_10xxx/init_cpus.c 2011-02-26 01:46:19.0 +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.0 +0100 +++ coreboot-disupducode/src/cpu/amd/model_10xxx/Kconfig 2011-02-26 00:59:20.0 +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.0 +0100 +++ coreboot-disupducode/src/cpu/amd/model_10xxx/Makefile.inc 2011-02-26 00:04:56.0 +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.0 +0100 +++ coreboot-disupducode/src/mainboard/amd/bimini_fam10/romstage.c 2011-02-26 00:14:15.0 +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.0 +0100 +++ coreboot-disupducode/src/mainboard/amd/mahogany_fam10/romstage.c 2011-02-26 00:15:29.0 +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.0 +0100 +++ coreboot-disupducode/src/mainboard/amd/serengeti_cheetah_fam10/romstage.c 2011-02-26 00:16:25.0 +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
Re: [coreboot] [PATCH] disabling microcode update
This patch tries to fix compilation when you select EXPERT in make menuconfig. If I select Expert mode in make menuconfig I couldn't compile because it complained of 2 missing configuration constants. I hope this is the right solution, but haven't really checked that the related code in src/northbridge/amd/amdht/h3finit.c is correct. Signed-off-by: Xavi Drudis Ferran xdru...@tinet.cat --- --- coreboot-disupducode/src/northbridge/amd/Kconfig 2011-02-26 00:01:01.0 +0100 +++ coreboot-expert/src/northbridge/amd/Kconfig 2011-02-26 03:02:15.0 +0100 @@ -24,8 +24,12 @@ config LIMIT_HT_SPEED_200 bool Limit HT frequency to 200MHz +config LIMIT_HT_SPEED_300 + bool Limit HT frequency to 300MHz config LIMIT_HT_SPEED_400 bool Limit HT frequency to 400MHz +config LIMIT_HT_SPEED_500 + bool Limit HT frequency to 500MHz config LIMIT_HT_SPEED_600 bool Limit HT frequency to 600MHz config LIMIT_HT_SPEED_800 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
On Mon, Feb 21, 2011 at 08:44:35AM -0600, Scott Duplichan wrote: This really isn't relevant, but microcode patch source code certainly exists, as does source code for the main microcode that the patch modifies. A microcode assembler converts the source code into binary form. I think it's relevant. In any case, thank you for the information. Think about what it would take for public microcode patch source code to be useful. First, the processor vendor would have to publish the thousands of lines of source code for the microcode that will be patched. Next, the microcode assembler and related tools would have to be published. At that point, a lot of training is needed to even understand the microcode. Microcode is not written in any standard programming language. The language is completely different for different processor vendors, and even for different processor models in some cases. Anyway, assume that somehow a non-employee became a microcode expert for a particular processor model. Why would the patch need to be modified? The patch corrects a processor erratum. To modify microcode in a way that works around a processor erratum usually requires details of the processor design that are not published. Yes, I know the processor is not open hardware. Yet, the issue does not vanish just because of this. You say it would require an expert. Well, that's what average people think of free software, it takes an expert to modify a program, so who needs software freedom. I won't untangle that fallacy here to avoid preaching to the choir. Sure, there may be more experts apt at modifying emacs than certain microcode source, but so what? Before I knew coreboot I didn't think the expertise necessary to write firmware could be had in a free software project, and now I'm arguing with one of those experts I thought didn't exist. There are several free software projects that work around or reverse engineer secret designs. The fact that it is unlikely or difficult is no excuse to use other people's work against the license they have choosen. It's not me you have to convince or you I have to convince, it's any copyright holder to code linked to this microcode if we want to make a derivative work. When a processor is purchased, its source code is not provided. The microcode patch is a modification of the processor design, so it is not surprising the source code is not supplied. That's a particular point of view, seeing microcode patches as some kind of binary soldering iron of sorts. I don't see it like that. Changing microcode does not change the microprocessor design any more than changing documentation or firmware. The circuits are so that given some microcode perfom some function. They'd do something else with a different microcode but they would still be the same circuits. Otherwise any software would be a circuit metamorphoser. I see microcode as a preloaded propietary program for a VLIW processor whose purpose is to emulate a CISC processor that happens to have more application software available. The x86, amd64 or whatever instruction set works just like an ABI, similar to parrot or java bytecodes allowing people to write once and run anywhere, and makes economic sense. But it does not change the nature of the VLIW processor. The processor design is information, VHDL programs if you like. The only thing that distinguishes the logic that goes to silicon from what goes to software is the possibility to change it without building another device. Therefore, microcode is software for a (nondocumented) machine as soon as it can be modified. This VLIW nondocumented machine could have compilers for any language that generate microcode or interpreters for other instruction sets, and maybe I could have the compiler generate a new opcode to optimize a tight loop or run ARM or PPC binaries in my Phenom. Not that I'm interested, I'd rather recompile free software for amd64. And not that it would be economic to build such a compiler backend for such a small number of CPUs out there. All this is not trying to bash any cpu vendor or imply any wrongdoing by anyone. When I bought my CPU it was advertised as amd64, x86 , MMX , etc. compatible, not as a VLIW processor I could reprogram by microcode patches. And I'm not claiming I have a great business model for paying the engineers and factories to build open hardware of similar power (although if someone does, I'd like to get some advertising). And I'm not saying that I'm surprised to know that sources or documentation for microcode or cpu designs are not published. I'm just surprised to find microcode linked into GPL binaries. My intent is just to explain that those wanting these binary parts left out of their coreboot image are not some kind of luddite zealots, simply people that like to apply uniform criteria to their free software. And this is just too much for a rationale for a 69 lines patch, so I'll leave it here and sorry for the
Re: [coreboot] [PATCH] disabling microcode update
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. If I _really_ wanted to disable microcode updates, I would simply comment out a line of code. 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. + We are firmware developers, not hardware engineers. I'm not voting no on this, just so as long as it is completely invisible to me. Alex -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
On Mon 21/02/11 03:58 , Stefan Reinauer stefan.reina...@coreboot.org wrote: On 2/19/11 12:45 PM, xdrudis wrote: On Fri, Feb 18, 2011 at 10:19:31AM -0500, Ward Vandewege wrote: Hi Xavi, On Wed, Feb 16, 2011 at 02:45:02PM +0100, Xavi Drudis Ferran wrote: Should I send a patch making a Kconfig option to not upgra de microcode for fam10? Is there any interest in that ? What's the particular rationale behind this? Rationale for disabling microcode update: Ivaylo and I have had issues when updating microcode in FAM10 We dont' have source for it (if it exists at all) I've suggested it and got 2 expressions of interest and negative reaction, so I did it. Some mainboard Kconfig specify one header file for microcode and some another. I'm not aware of the criteria for picking one or other (looking at svn log it appears to depend on the CPU revision, but I think I remember there was some revisions which could get more than one header file). If you disable microcode update you may use the same mainboard dir for some more different CPU revisions, assuming you are not affected by any of the bugs the microcode update adresses. Rationale for restricting it to FAM10 Laziness/Lack of time lack of hadrware for testing no specfific request, no specific issues reported on other hardware When disabling microcode updates, why don't you also disable the other erratas? Because we have source for that code, and some documentation.I've fixed code for erratas when it has given me problems. But I cannot fix microcode, only disable update. + Select this to apply (propietary?) patches to t he cpu I don't think the word proprietary (nor the question mark) applies very well here. word : I meant propietary as in non-free. Of course within out legal systems all code is the property of some person or entity, and in that sense it is propietary. But I meant non-free. Would it be better non-free ? Have another suggestion for a word to tell its difference of its distribution terms with the rest of coreboot (or most of the rest) question mark: Laziness. I have only looked at one microcode file. I don't know about the license for the rest. I agree to remove it if you know it's all the same + microcode provided by AMD to correct issues in� �the CPU after+ production, and distributed with coreboot (not n ecessarily+ the latest microcode version produced by AMD, b ut only+ applied if newer than the version in your CPU) .+ + Unselect to let FAM10 CPUs run with factory microcode. If Here's some formatting problem. It's also unclear what you mean by factory microcode imho. formatting: sorry, I think a tab slipped in. I'll try to fix it if we agree to a new text. factory microcode: possibly my poor English. I meant the version of the microcode the CPU came with from factory, that is, the version of microcode that will be used in the CPU if we don't update microcode. + you unselect this, no binary microcode patches will be+ included in the image, so it will help you ge t an image+ which you have the entire source code for and� �may simplify+ license compliance (IANAL). I don't see how this makes license compliance any easier. Also, please refrain from using terms like IANAL. If we make claims they should be either right to the best of our knowledge or we don't put them in the source code, anyways. It is right to the best of my knowledge, but IANAL, so my knowledge may not be enough. I agree to remove IANAL if the list thinks it's enough. License compliance is easier when there are fewer licenses to deal with. update_microcode.c is under GPL. It includes one of a few header files, like mc_patch_01b6.h which has an adhoc license restricting uses (you can't use it for teaching, not explicitly allowing redistribution, etc.). For a layman it is easier not to worry about whether that is linkable with GPL code, whether the forbidden uses are possible, whether some copyright holder will one day sue because their GPL code is in coreboot liked with this... Having to decide about implicit licenses in sign-off, application of source code definition, etc. isn't so simple. It may be safe after all, but it is easier to make sure if you don't inlcude it. License compliance would be simplified by removing microcode even if the microcode was BSD licensed. The fewer licenses the simpler it is. Although probably if it was BSD I wouldn't have bothered sayign it. Those of us who are mere individuals and like to try to keep legal without a legal department may like this extra simplicity. BTW, some corporations had legal teams analyze the microcode license and it was considered not problematic for inclusion in coreboot in the sense of the GPL. Which wouldn't have been needed if everything was GPL, right ? So disabling microcode simplifies license compliance. But I'd be
Re: [coreboot] [PATCH] disabling microcode update
I've suggested it and got 2 expressions of interest and negative reaction, so I did it. Sorry, I meant and no negative reaction. I'd just add that it's just an option, and I've made the default to update microcode, liek it did before, so it has no effect unless you explicity unselect it. -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
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. //Peter -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [PATCH] disabling microcode update
On 2/19/11 12:45 PM, xdrudis wrote:
On Fri, Feb 18, 2011 at 10:19:31AM -0500, Ward Vandewege wrote:
Hi Xavi,
On Wed, Feb 16, 2011 at 02:45:02PM +0100, Xavi Drudis Ferran wrote:
Should I send a patch making a Kconfig option to not upgrade microcode
for fam10? Is there any interest in that ?
What's the particular rationale behind this? When disabling microcode
updates, why don't you also disable the other erratas?
+ Select this to apply (propietary?) patches to the cpu
I don't think the word proprietary (nor the question mark) applies very
well here.
+ microcode provided by AMD to correct issues in the CPU after
+ production, and distributed with coreboot (not necessarily
+ the latest microcode version produced by AMD, but only
+ applied if newer than the version in your CPU).
+
+ Unselect to let FAM10 CPUs run with factory microcode. If
Here's some formatting problem. It's also unclear what you mean by
factory microcode imho.
+ you unselect this, no binary microcode patches will be
+ included in the image, so it will help you get an image
+ which you have the entire source code for and may simplify
+ license compliance (IANAL).
I don't see how this makes license compliance any easier. Also, please
refrain from using terms like IANAL. If we make claims they should be
either right to the best of our knowledge or we don't put them in the
source code, anyways.
BTW, some corporations had legal teams analyze the microcode license and
it was considered not problematic for inclusion in coreboot in the sense
of the GPL.
--- src/cpu/amd/model_10xxx/update_microcode.c 2011-02-19 21:56:44.0
+0100
+++ src/cpu/amd/model_10xxx/update_microcode.c 2011-02-19 22:09:17.0
+0100
@@ -29,6 +29,7 @@
#includecpu/amd/microcode.h
#endif
+#if CONFIG_UPDATE_CPU_MICROCODE
static const u8 microcode_updates[] __attribute__ ((aligned(16))) = {
Please change the patch to not include update_microcode.c in the
Makefile for the case that microcode selection is disabled.
Stefan
--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
[coreboot] [PATCH] disabling microcode update
On Fri, Feb 18, 2011 at 10:19:31AM -0500, Ward Vandewege wrote:
Hi Xavi,
On Wed, Feb 16, 2011 at 02:45:02PM +0100, Xavi Drudis Ferran wrote:
Should I send a patch making a Kconfig option to not upgrade microcode for
fam10? Is there any interest in that ?
Yes, please. I would test and ack that.
Thanks,
Ward.
Here it is. It's limited to fam10. I don't know about other families nor can
test.
Sorry about having missed the request from Ivaylo, for a moment I
thought I was the only one interested.
It works in my board (but my board still does not boot, so tests in
functional boards are welcome).
Thanks. Allow the user to disable cpu microcode updating
(only for AMD model_10xxx cpus) in make menuconfig.
If disabled the microcode is not included in
update_microcode.c and therefore the generate image
does not include it.
Signed-off-by: Xavi Drudis Ferran xdru...@tinet.cat
---
I've abuild tested it with default y and default n,
not sure if abuild build with all default options and
so both cases are tested or I should do something else.
--- src/cpu/amd/model_10xxx/Kconfig 2011-02-19 21:56:44.0 +0100
+++ src/cpu/amd/model_10xxx/Kconfig 2011-02-19 19:48:20.0 +0100
@@ -50,3 +50,22 @@
endif
endif
+
+config UPDATE_CPU_MICROCODE
+ bool Update cpu microcode
+ default y
+ depends on CPU_AMD_MODEL_10XXX
+help
+ Select this to apply (propietary?) patches to the cpu
+ microcode provided by AMD to correct issues in the CPU after
+ production, and distributed with coreboot (not necessarily
+ the latest microcode version produced by AMD, but only
+ applied if newer than the version in your CPU).
+
+ Unselect to let FAM10 CPUs run with factory microcode. If
+ you unselect this, no binary microcode patches will be
+ included in the image, so it will help you get an image
+ which you have the entire source code for and may simplify
+ license compliance (IANAL).
+
+
--- src/cpu/amd/model_10xxx/update_microcode.c 2011-02-19 21:56:44.0 +0100
+++ src/cpu/amd/model_10xxx/update_microcode.c 2011-02-19 22:09:17.0 +0100
@@ -29,6 +29,7 @@
#include cpu/amd/microcode.h
#endif
+#if CONFIG_UPDATE_CPU_MICROCODE
static const u8 microcode_updates[] __attribute__ ((aligned(16))) = {
#ifdef __PRE_RAM__
@@ -93,9 +94,11 @@
return new_id;
}
+#endif
void update_microcode(u32 cpu_deviceid)
{
+#if CONFIG_UPDATE_CPU_MICROCODE
u32 equivalent_processor_rev_id;
/* Update the microcode */
@@ -105,5 +108,6 @@
} else {
printk(BIOS_DEBUG, microcode: rev id not found. Skipping microcode patch!\n);
}
+#endif
}
--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
