[coreboot] [patch][v2][2/4] Family 10
Marc Jones
Marc.Jones at amd.com
Tue Apr 22 20:05:25 CEST 2008
Stefan Reinauer wrote:
> * Marc Jones <Marc.Jones at amd.com> [080416 19:19]:
>> Update the FAM10 microcode to current versions. In addition, AP microcode is
>> now updated in early initialization to support errata settings that require it.
>>
>>
>> Signed-off-by: Marc Jones (marc.jones at amd.com)
>
> Acked-by: Stefan Reinauer <stepan at coresystems.de>
>
> See a few comments below. They're mostly future ideas.
>
> Stefan
>
>> + printk_err("microcode: Not updated! Fix microcode_updates[] \n");
>> return 0;
>
> Is this really an error condition? Or rather a warning? How serious is
> it?
>
I think it should always be reported which is why I made it an _err. It
will continue to boot but you could have any number of instabilities due
to errata (publicly documented or not).
>> Index: coreboot-v2/src/cpu/amd/model_10xxx/mc_patch_01000065.h
>> ===================================================================
>> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
>> +++ coreboot-v2/src/cpu/amd/model_10xxx/mc_patch_01000065.h 2008-04-16 09:49:40.000000000 -0600
>> @@ -0,0 +1,164 @@
>> +/*
>> + ============================================================
>> + (c) Advanced Micro Devices, Inc., 2004-2008
>> +
>> + The enclosed microcode is intended to be used with AMD
>> + Microprocessors. You may copy, view and install the
>
> I know this is an old license and we have been accepting this before.
> We are fine with linking this into our code, are we?
>
> Would it make sense for v3 to pack this into a lar? and load it
> externally?
>
>
That would depend on how we incorporate AGESA in v3, but it is a good
idea since this is a binary blob.
>> @@ -455,6 +450,7 @@
>> { X86_VENDOR_AMD, 0x100f21 },
>> { X86_VENDOR_AMD, 0x100f2A },
>> { X86_VENDOR_AMD, 0x100f22 },
>> + { X86_VENDOR_AMD, 0x100f23 },
>> { 0, 0 },
>> };
>
> I want to suggest here that the list of CPUs is changed to only use
> family and model but not stepping. AMD CPUs are not broken enough that
> we need different cpu init code per stepping. So it is enough to write
>
> { X86_VENDOR_AMD, 0x100f20 },
>
> and it will match 1, a, 2, 3, ....
>
> I am not a fan of artificially restricting the number of machines that
> are correctly initialized just because a developer didnt have it in
> her/his hands yet.
>
I have mixed feelings. I think it is important to have all the stepping
errata in place. This is important for stability and performance. I
agree that AMD aren't broken enough that things will catch on fire
without the errata but I would prefer coreboot have the same (or better)
quality as the IBVs.
Marc
--
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors
More information about the coreboot
mailing list