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

[Marc Jones]

Stefan Reinauer wrote:
> Marc Jones  [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 ([e-mail])
>
> Acked-by: Stefan Reinauer
> 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