Patchwork model_65x CPU support

login
register
about
Submitter Keith Hui
Date 2010-10-17 00:48:15
Message ID <AANLkTimr4cWO3K3f4JOW3Xx0eRDrczCXPTzd+PVxQYLW@mail.gmail.com>
Download mbox | patch
Permalink /patch/2126/
State Accepted
Commit r5960
Headers show

Comments

Keith Hui - 2010-10-17 00:48:15
>Keith Hui wrote:
>> Attached patch moves support for Deschutes Slot 1 CPUs (model_65x)
>> into its own home.

>Then I guess the patch should also delete some lines in existing
files?

Yes. I knew I must be missing something. (Oops) See attached patch.

Signed-off-by: Keith Hui <buurin@gmail.com>

>> This is similar to the Katmai patch I submitted before, but without
>> microcode this time. With 17 of them they will make the patch too
>> large. I think I'll defer them for later or someone else*, hehe. :D
>>
>> abuild-tested.
>
>It doesn't look like the new code is used anywhere? Should it be?

It should be used from cpu/intel/slot_1.

>> * also because I like to keep all microcodes for the same family
>> within one file instead of one microcode per file.

>We want microcode updates to be as close as possible to upstream
How upstream are we talking about? :P These updates come from Intel as
one big .h file. ;-)

>format. Eventually I think we want to put them verbatim into cbfs.
This I agree wholeheartedly. Did I ever mention that Award also stuff
their microcode updates this way into the firmware image? And hey, I
might as well take a crack at this!

But a bit of scheduling headache then awaits. How much CPU init do we
need to do before we start drawing things in from the cbfs? Is there
anything that cannot be done until microcode updates are loaded? Is
cpu_dev_ops.init() an appropriate place to access cbfs? If not, from
where?

Cheers
Keith
Removes model_65x CPUIDs from model_6xx code. 
They now have their own home at cpu/intel/model_65x.

Signed-off-by: Keith Hui <buurin@gmail.com>
Corey Osgood - 2010-10-17 02:34:58
On Sat, Oct 16, 2010 at 5:48 PM, Keith Hui <buurin@gmail.com> wrote:
> But a bit of scheduling headache then awaits. How much CPU init do we
> need to do before we start drawing things in from the cbfs? Is there
> anything that cannot be done until microcode updates are loaded? Is
> cpu_dev_ops.init() an appropriate place to access cbfs? If not, from
> where?

Microcode updates are, from my understanding, typically not extremely
important. IMO, including them in coreboot should be a Kconfig option,
because it's probably faster to let just linux load them (but other
OS's may not have that ability).

-Corey
Patrick Georgi - 2010-10-17 07:28:14
Am 17.10.2010 04:34, schrieb Corey Osgood:
> Microcode updates are, from my understanding, typically not extremely
> important. IMO, including them in coreboot should be a Kconfig option,
> because it's probably faster to let just linux load them (but other
> OS's may not have that ability).
We're just lucky that none of the CPUs requires a microcode update for
proper CAR operation yet (for example).
That would be something that microcode updates in linux couldn't manage.

Generally these updates should happen as early as possible.


Patrick
Stefan Reinauer - 2010-10-17 07:40:05
On 16.10.2010, at 19:34, Corey Osgood <corey.osgood@gmail.com> wrote:

> On Sat, Oct 16, 2010 at 5:48 PM, Keith Hui <buurin@gmail.com> wrote:
>> But a bit of scheduling headache then awaits. How much CPU init do we
>> need to do before we start drawing things in from the cbfs? Is there
>> anything that cannot be done until microcode updates are loaded? Is
>> cpu_dev_ops.init() an appropriate place to access cbfs? If not, from
>> where?
> 
> Microcode updates are, from my understanding, typically not extremely
> important. IMO, including them in coreboot should be a Kconfig option,
> because it's probably faster to let just linux load them (but other
> OS's may not have that ability).
> 

Rumors say that on i7 you'd want to load microcode before enabling CAR. I don't think it's mandatory but in case it is we have a wonderful cbfs walker written in assembler by Patrick. 

Right now we load microcode in stage 2 which is long after ram has been initialized.

Stefan 



> -Corey
> 
> -- 
> coreboot mailing list: coreboot@coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>
Stefan Reinauer - 2010-10-17 08:07:30
On 10/16/10 5:48 PM, Keith Hui wrote:
>> We want microcode updates to be as close as possible to upstream
> How upstream are we talking about? :P These updates come from Intel as
> one big .h file. ;-)

I investigated the possibility to merge the intel cpu drivers in that
regard and looked at pulling in the complete intel microcode update file
in. However, it's far too big in size for that and there's really no
gain in pulling in a lot of blob that never gets used. If you look at
other BIOSes you will find out that they carefully select which
microcode to include, and they do that for a good reason, as do we.

> But a bit of scheduling headache then awaits. How much CPU init do we
> need to do before we start drawing things in from the cbfs? 
Right now we don't do any.


> Is there
> anything that cannot be done until microcode updates are loaded? Is
> cpu_dev_ops.init() an appropriate place to access cbfs? If not, from
> where?
That's where we load microcode right now, and I suggest not changing
that unless there is a reason to do so.

Stefan
Uwe Hermann - 2010-10-17 22:06:29
On Sat, Oct 16, 2010 at 08:48:15PM -0400, Keith Hui wrote:
> >Keith Hui wrote:
> >> Attached patch moves support for Deschutes Slot 1 CPUs (model_65x)
> >> into its own home.
> 
> >Then I guess the patch should also delete some lines in existing
> files?
> 
> Yes. I knew I must be missing something. (Oops) See attached patch.
> 
> Signed-off-by: Keith Hui <buurin@gmail.com>

Thanks, r5960.

 
Uwe.

Patch

Index: src/cpu/intel/model_6xx/model_6xx_init.c
===================================================================
--- src/cpu/intel/model_6xx/model_6xx_init.c	(revision 5953)
+++ src/cpu/intel/model_6xx/model_6xx_init.c	(working copy)
@@ -84,11 +84,6 @@ 
 	{ X86_VENDOR_INTEL, 0x0633 }, /* PII, C0 */
 	{ X86_VENDOR_INTEL, 0x0634 }, /* PII, C1 */
 
-	{ X86_VENDOR_INTEL, 0x0650 }, /* PII/Celeron, dA0/mdA0/A0 */
-	{ X86_VENDOR_INTEL, 0x0651 }, /* PII/Celeron, dA1/A1 */
-	{ X86_VENDOR_INTEL, 0x0652 }, /* PII/Celeron/Xeon, dB0/mdB0/B0 */
-	{ X86_VENDOR_INTEL, 0x0653 }, /* PII/Xeon, dB1/B1 */
-
 	{ X86_VENDOR_INTEL, 0x0660 }, /* Celeron, A0 */
 	{ X86_VENDOR_INTEL, 0x0665 }, /* Celeron, B0 */
 	{ X86_VENDOR_INTEL, 0x066a }, /* PII, mdxA0/dmmA0 + others */