[coreboot] [patch][v2][2/4] Family 10

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Apr 22 22:06:42 CEST 2008


On 22.04.2008 20:05, Marc Jones wrote:
> 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
>   
>>> 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.
>   

As the resident "everything in LAR" guy I'm strongly in favour of having
microcode updates as blobs in a LAR. Two points need to be made sure,
though:
- If we compress the update, a decompression routine must not be able to
trigger data-corruption bugs.
- We have to know whether microcode updates can be deferred to a point
after RAMinit.


>>> @@ -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.
>   

Agreed. Can we handle this like IP routing tables?
Store familiy/model/stepping together with number of significant bits
from the left, then pick the one which matches and has most significant
digits.

That even allows for append-only tables with overrides.

We have to make sure that the table is stored in a separate LAR blob to
avoid having to update stage2 for microcode updates and more supported
processors.

Regards,
Carl-Daniel




More information about the coreboot mailing list