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

[Stefan Reinauer]

* 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/microcode/microcode.c
> ===================================================================
> --- coreboot-v2.orig/src/cpu/amd/microcode/microcode.c	2008-04-16 10:20:04.000000000 -0600
> +++ coreboot-v2/src/cpu/amd/microcode/microcode.c	2008-04-16 10:27:43.000000000 -0600
> @@ -61,8 +61,8 @@
>  {
>  
>  	if (m->processor_rev_id != equivalent_processor_rev_id) {
> -		printk_debug("microcode: rev id does not match this patch.\n");
> -		printk_debug("microcode: Not updated! Fix microcode_updates[] \n");
> +		printk_err("microcode: rev id (%x) does not match this patch.\n", m->processor_rev_id);
> +		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?

> 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
> + enclosed microcode  only for development  and deployment of
> + firmware,  BIOS,  or  operating  system code  for  computer
> + systems   that  contain  AMD   processors.   You   are  not
> + authorized  to use  the  enclosed microcode  for any  other
> + purpose.
> +
> + THE  MICROCODE IS PROVIDED  "AS IS"  WITHOUT ANY  EXPRESS OR
> + IMPLIED WARRANTY  OF ANY KIND, INCLUDING BUT  NOT LIMITED TO
> + WARRANTIES    OF    MERCHANTABILITY,   NON-    INFRINGEMENT,
> + TITLE,FITNESS  FOR  ANY  PARTICULAR PURPOSE,  OR  WARRANTIES
> + ARISING FROM CONDUCT, COURSE  OF DEALING, OR USAGE OF TRADE.
> + AMD does not assume  any responsibility for any errors which
> + may  appear   in  this   microcode  or  any   other  related
> + information provided  to you by  AMD, or result from  use of
> + this microcode.   AMD is not obligated  to furnish, support,
> + or  make   any  further  information,   software,  technical
> + information, know-how, or show-how available related to this
> + microcode.
> +
> + The  microcode is provided  with "RESTRICTED  RIGHTS."  Use,
> + duplication, or disclosure by the U.S. Government is subject
> + to  the  restrictions as  set  forth  in  FAR 52.227-14  and
> + DFAR252.227-7013,  et seq.,  or its  successor.  Use  of the
> + microcode    by    the    U.S.     Government    constitutes
> + acknowledgement  of   AMD's  proprietary  rights   in  them.
> + ============================================================
> +*/

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?


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

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866