Re: [coreboot] [PATCH] SECC Pentium 2/3 users are gonna love this

[Keith Hui]

> Please don't gzip patches, it lowers the chance of someone reviewing
> them to zero ;-)

I figured if we're coding at this level, gzip won't be a hurdle. I
announced it up front, didn't I? :p
> There is 65x microcode in the model_67x directory, and it's not split up
> into single files as for the other CPUs.

The microcodes are static and are not supposed to change, our CPU
drivers are grouped by family, and the microcodes are going to be
included one after another in one place anyway even when made
separate, so why have 5 files for 5 microcodes in the 67x family for
example when one will do? Of course I retained the comments therein
that marks the microcodes and the processors they're intended for.

To put things in perspective, the microcode data we get from Intel has
hundreds of them all in one file.
> + if (cpuid_res.ebx != 0x756e6547 || cpuid_res.edx != 0x49656e69 ||
> cpuid_res.ecx != 0x6c65746e) {
> + printk(BIOS_INFO, "Not 'GenuineIntel' Processor");
> + return 0;
> + }
>
> This is not necessary. When model_6**_init() is executed, it's clear
> already that this is an intel cpu.
> I'm not sure if the check for choosing the CPU driver to execute is on
> these CPUID signatures, or through some other fields. This check is
> carried from v1. I agree we can take a chance and take it out; we'll
> revisit when something breaks. :D
> +/* if (signature & 0x1000) {
> + printk(BIOS_DEBUG,"Overdrive chip no L2 cache configuration\n");
> + return 0;
> + }
> +
> + if (signature < 0x630 || signature >= 0x680) {
> + printk(BIOS_DEBUG,"L2 cache on CPUID %x does not require
> configuration\n", signature);
> + return 0;
> + }*/
>
> I think this code should just be dropped

That may change when (IF) we manage to get coreboot on a socket 8
board on which a Pentium II OverDrive processor sits.
Again this piece is from the original code from v1. I agree the second
signature check is not needed.
> Is it generally possible to move p6_configure_l2_cache to a generic
> place, maybe to cpu/x86/cache ?
>
> +/*--- End L2 init code from corebbot v1 ---*/
>
> I think this kind of comment is not needed.

That's a marker to help preserve my sanity. :-p
> +static inline void strcpy(char *dst, char *src)
> +{
> + while (*src) *dst++ = *src++;
> +}
>
> I know I am the idiot who introduced this in a model_xxx_init.c file,
> but maybe we should move it to lib/strcpy.c ?

Well, 63x and 67x don't even need this; they don't support the
processor brand string that 6bx do. Same with the
fill_processor_name() function. I'll just take them out entirely.
> +static struct cpu_device_id cpu_table[] = {
> + { X86_VENDOR_INTEL, 0x0650 },
> + { X86_VENDOR_INTEL, 0x0651 },
> + { X86_VENDOR_INTEL, 0x0652 },
> + { X86_VENDOR_INTEL, 0x0653 },
> + { X86_VENDOR_INTEL, 0x0671 },
> + { X86_VENDOR_INTEL, 0x0672 },
> + { X86_VENDOR_INTEL, 0x0673 },
> + { 0, 0 },
> +};
>
> Instead of using a common driver, can two drivers use a common set of
> shared functions instead?

It's already partially done. src/cpu/intel/model_67x/l2_cache.c (new
in this patch) is linked in by model_65x driver too.

Next prime suspect: fill_processor_name() in 6bx and a few other CPU
models I'm no expert of.

But 65x and 67x really are so similar that one driver can do both. Now
is our end goal here to spin off model_63x/65x/66x/67x/68x/6ax/6bx all
into their own CPU drivers when most of what's being done is
duplicating code from model_6xx which is what they collectively once
were? What's the rationale behind this need for split? I think a split
should only be done when different initialization sequence are needed,
eg. the special L2 initialization sequence for these SECC P6s.

Same with motherboard models. When P2B-LS support matures I'd probably
use the same directory for P2B-L and P2B-S support. They share the
same circuit board anyway, and a -LS can emulate both by moving two
jumpers.
> +ifeq ($(CONFIG_USE_DCACHE_RAM),y)
> +cpu_incs += $(src)/cpu/intel/car/cache_as_ram.inc
> +endif
>
> This will break all targets except the ones you worked on.

I read this and thought, "I gotta run abuild." So I did. The errors I
saw all seems to be k8/k10 targets and/or ACPI related. Even the
entire P2B family that is in the tree passed abuild with this patch
applied. Oh they haven't been migrated to CAR yet.

Thanks
Keith

[PATCH] SECC Pentium 2/3 users are gonna love this

[Jouni Mettälä]

Hi

Parts of original patch are already in coreboot. This version made cache
work in my board now. It might need work so it doesn't break others. Here is
part of serial capture. Rest is attached

Initializing CPU #0
CPU: vendor Intel device 673
CPU: family 06, model 07, stepping 03
microcode_info: sig = 0x00000673 pf=0x00000001 rev = 0x00000000
microcode updated to revision: 0000000e from revision 00000000
Configuring L2 cache... rdmsr(IA32_PLATFORM_ID) = 0, 11020000
L2 Cache latency is 8
Sending 0 to set_l2_register4
L2 ECC Checking is enabled
L2 Physical Address Range is 4096M
Maximum cache mask is 20000
L2 Cache Mask is 4000
read_l2(2) = 8
write_l2(2) = 8
L2 Cache size is 512K
L2 Cache lines initialized

Signed-off-by: Jouni Mettälä <[e-mail]>

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Fri, Jan 07, 2011 at 10:45:40PM +0200, Jouni Mettälä wrote:
> Hi
>   Parts of original patch are already in coreboot. This version made cache
>   work in my board now. It might need work so it doesn't break others. Here
>   is part of serial capture. Rest is attached

Got the following on compile:

    CC         southbridge/intel/i82371eb/reset.driver.o
    CC         northbridge/intel/i440bx/northbridge.driver.o
    CC         cpu/intel/model_6xx/model_6xx_init.driver.o
    CC         cpu/intel/model_65x/model_65x_init.driver.o
make: *** No rule to make target `src/cpu/intel/model_67x/l2_cache.c', needed by `build/cpu/intel/model_67x/l2_cache.driver.o'.  Stop.

[PATCH] SECC Pentium 2/3 users are gonna love this

[Idwer Vollering]

2011/1/8 Roger <[e-mail]>
> On Fri, Jan 07, 2011 at 10:45:40PM +0200, Jouni Mettälä wrote:
> > Hi
> >   Parts of original patch are already in coreboot. This version made
> > cache
> >   work in my board now. It might need work so it doesn't break others.
> > Here
> >   is part of serial capture. Rest is attached
>
> Got the following on compile:
>
> CC         southbridge/intel/i82371eb/reset.driver.o
>    CC         northbridge/intel/i440bx/northbridge.driver.o
>    CC         cpu/intel/model_6xx/model_6xx_init.driver.o
>    CC         cpu/intel/model_65x/model_65x_init.driver.o
> make: *** No rule to make target `src/cpu/intel/model_67x/l2_cache.c',
> needed by `build/cpu/intel/model_67x/l2_cache.driver.o'.  Stop.

Jouni: is l2_cache.c a new file ? If it is: have you forgot to ad it (svn
add l2_cache.c) ?
>

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Sat, Jan 08, 2011 at 03:22:15AM +0100, Idwer Vollering wrote:
> 2011/1/8 Roger <[1][e-mail]>
>
> On Fri, Jan 07, 2011 at 10:45:40PM +0200, Jouni Mettälä wrote:
>     >   Hi
>     >   Parts of original patch are already in coreboot. This version made
>     cache
>     >   work in my board now. It might need work so it doesn't break others.
>     Here
>     >   is part of serial capture. Rest is attached
>
> Got the following on compile:
>
>    CC         southbridge/intel/i82371eb/reset.driver.o
>        CC         northbridge/intel/i440bx/northbridge.driver.o
>        CC         cpu/intel/model_6xx/model_6xx_init.driver.o
>        CC         cpu/intel/model_65x/model_65x_init.driver.o
>     make: *** No rule to make target `src/cpu/intel/model_67x/l2_cache.c',
>     needed by `build/cpu/intel/model_67x/l2_cache.driver.o'.  Stop.
>
> Jouni: is l2_cache.c a new file ? If it is: have you forgot to ad it (svn
>   add l2_cache.c) ?

find src/coreboot -iname "l2_cache.c" returns null here. :-/

-- 
Roger
http://rogerx.freeshell.org/

[PATCH] SECC Pentium 2/3 users are gonna love this

[Jouni Mettälä]

Here is updated patch with l2_cache.c added.

Signed-off-by: Jouni Mettälä <[e-mail]>

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Sat, Jan 08, 2011 at 12:25:36PM +0000, Jouni Mettälä wrote:
> Here is updated patch with l2_cache.c added.
>   Signed-off-by: Jouni Mettälä <[1][e-mail]>

CC         mainboard/tyan/s1846/crt0.s
    CC         mainboard/tyan/s1846/crt0.romstage.o
make: *** No rule to make target `/home/roger/src/coreboot-tyan/coreboot/build/config.h', needed by `build/lib/uart8250.romstage.o'.  Stop.

-- 
Roger
http://rogerx.freeshell.org/

[PATCH] SECC Pentium 2/3 users are gonna love this

[Myles Watson]

On Sat, Jan 8, 2011 at 5:25 AM, Jouni Mettälä <[e-mail]> wrote:
> Here is updated patch with l2_cache.c added.

I think it's missing l2_cache.h now.

Thanks,
Myles

[PATCH] SECC Pentium 2/3 users are gonna love this

[Jouni Mettälä]

Updated patch with l2_cache.h added

Signed-off-by: Jouni Mettälä <[e-mail]>

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Sat, Jan 08, 2011 at 08:37:53PM +0000, Jouni Mettälä wrote:
> Updated patch with l2_cache.h added
>   Signed-off-by: Jouni Mettälä <[1][e-mail]>
>
> References
>
> Visible links
>   1. mailto:[e-mail]

OK. V3 compiled file.  My previous uart missing file looks to be my fault for
not properly utilizing svn update.

I have only compiled without errors and have not tested for execution
problems, yet.

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

I've applied the patch here and have been testing on a Tyan 1832DL using the
Tyan 1846 mainboard and am not seeing any of the patch's printk output within
the logs here.

I have grepped for L2 & rdmsr.

I can see where one of the patch files links in SLOT1 cpu with
the 67x cpu.  Can also see where the compile output also compiles model_67x
code along with two or three other cpu models.

[PATCH] SECC Pentium 2/3 users are gonna love this

[Keith Hui]

Hi all,

Here is the new L2 cache patch. Sign-off in the patch itself. Still
very juicy and tasty at 25k. :D

Also done is including cpu/intel/model_68x again in slot_1. Otherwise
it will die with a Coppermine P3 installed.

My boot log on P2B-LS and a Katmai 600MHz attached.

I have optimized it some more, and added more information and
meaningful constants as I cross checked the code with Intel's
documentation. Some debugging messages are different too. Give this a
good workout.

Cheers
Keith

ps. Copying people who have sent me reports. :)

On Fri, Jan 7, 2011 at 3:45 PM, Jouni Mettälä <[e-mail]> wrote:
> Hi
> Parts of original patch are already in coreboot. This version made cache
> work in my board now. It might need work so it doesn't break others. Here is
> part of serial capture. Rest is attached
> Initializing CPU #0
> CPU: vendor Intel device 673
> CPU: family 06, model 07, stepping 03
> microcode_info: sig = 0x00000673 pf=0x00000001 rev = 0x00000000
> microcode updated to revision: 0000000e from revision 00000000
> Configuring L2 cache... rdmsr(IA32_PLATFORM_ID) = 0, 11020000
> L2 Cache latency is 8
> Sending 0 to set_l2_register4
> L2 ECC Checking is enabled
> L2 Physical Address Range is 4096M
> Maximum cache mask is 20000
> L2 Cache Mask is 4000
> read_l2(2) = 8
> write_l2(2) = 8
> L2 Cache size is 512K
> L2 Cache lines initialized
> Signed-off-by: Jouni Mettälä <[e-mail]>

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Tue, Jan 11, 2011 at 11:17:17PM -0500, Keith Hui wrote:
> Hi all,
>
> Here is the new L2 cache patch. Sign-off in the patch itself. Still
> very juicy and tasty at 25k. :D
>
> Also done is including cpu/intel/model_68x again in slot_1. Otherwise
> it will die with a Coppermine P3 installed.

ug. I'm jealous.  What's the catch with Coppermine (for my CPU)?

[PATCH] SECC Pentium 2/3 users are gonna love this

[Marc Bertens]

Hi i have a question about this L2 cache, can it also be used for the P3
socket PGA370.

My nokia Ip530 has that type of CPU and as far as i know L2 cache is
disabled 

Regards,
Marc


-----Original Message-----
From: Keith Hui <[e-mail]>
To: [e-mail]
Cc: Jouni Mettälä <[e-mail]>, Idwer Vollering
<[e-mail]>, Roger <[e-mail]>
Subject: Re: [coreboot] [PATCH] SECC Pentium 2/3 users are gonna love
this
Date: Tue, 11 Jan 2011 23:17:17 -0500


Hi all,

Here is the new L2 cache patch. Sign-off in the patch itself. Still
very juicy and tasty at 25k. :D

Also done is including cpu/intel/model_68x again in slot_1. Otherwise
it will die with a Coppermine P3 installed.

My boot log on P2B-LS and a Katmai 600MHz attached.

I have optimized it some more, and added more information and
meaningful constants as I cross checked the code with Intel's
documentation. Some debugging messages are different too. Give this a
good workout.

Cheers
Keith

ps. Copying people who have sent me reports. :)

On Fri, Jan 7, 2011 at 3:45 PM, Jouni Mettälä <[e-mail]> wrote:
> Hi
> Parts of original patch are already in coreboot. This version made cache
> work in my board now. It might need work so it doesn't break others. Here is
> part of serial capture. Rest is attached
> Initializing CPU #0
> CPU: vendor Intel device 673
> CPU: family 06, model 07, stepping 03
> microcode_info: sig = 0x00000673 pf=0x00000001 rev = 0x00000000
> microcode updated to revision: 0000000e from revision 00000000
> Configuring L2 cache... rdmsr(IA32_PLATFORM_ID) = 0, 11020000
> L2 Cache latency is 8
> Sending 0 to set_l2_register4
> L2 ECC Checking is enabled
> L2 Physical Address Range is 4096M
> Maximum cache mask is 20000
> L2 Cache Mask is 4000
> read_l2(2) = 8
> write_l2(2) = 8
> L2 Cache size is 512K
> L2 Cache lines initialized
> Signed-off-by: Jouni Mettälä <[e-mail]>
> -- 
> coreboot mailing list: [e-mail]
> http://www.coreboot.org/mailman/listinfo/coreboot

[PATCH] SECC Pentium 2/3 users are gonna love this

[Keith Hui]

The L2 cache on a Coppermine doesn't need any special enabling
sequence. I just put a 1GHz Coppermine into my board and it boots fine
showing the full 256k cache. This patch doesn't even apply to them
anyway.

Cheers
Keith

On Wed, Jan 12, 2011 at 6:04 PM, Marc Bertens <[e-mail]> wrote:
> Hi i have a question about this L2 cache, can it also be used for the P3
> socket PGA370.
>
> My nokia Ip530 has that type of CPU and as far as i know L2 cache is
> disabled
>
> Regards,
> Marc
>
> -----Original Message-----
> From: Keith Hui <[e-mail]>
> To: [e-mail]
> Cc: Jouni Mettälä <[e-mail]>, Idwer Vollering <[e-mail]>,
> Roger <[e-mail]>
> Subject: Re: [coreboot] [PATCH] SECC Pentium 2/3 users are gonna love this
> Date: Tue, 11 Jan 2011 23:17:17 -0500
>
> Hi all,
>
> Here is the new L2 cache patch. Sign-off in the patch itself. Still
> very juicy and tasty at 25k. :D
>
> Also done is including cpu/intel/model_68x again in slot_1. Otherwise
> it will die with a Coppermine P3 installed.
>
> My boot log on P2B-LS and a Katmai 600MHz attached.
>
> I have optimized it some more, and added more information and
> meaningful constants as I cross checked the code with Intel's
> documentation. Some debugging messages are different too. Give this a
> good workout.
>
> Cheers
> Keith
>
> ps. Copying people who have sent me reports. :)
>
> On Fri, Jan 7, 2011 at 3:45 PM, Jouni Mettälä <[e-mail]> wrote:
> > Hi
> > Parts of original patch are already in coreboot. This version made cache
> > work in my board now. It might need work so it doesn't break others. Here
> > is
> > part of serial capture. Rest is attached
> > Initializing CPU #0
> > CPU: vendor Intel device 673
> > CPU: family 06, model 07, stepping 03
> > microcode_info: sig = 0x00000673 pf=0x00000001 rev = 0x00000000
> > microcode updated to revision: 0000000e from revision 00000000
> > Configuring L2 cache... rdmsr(IA32_PLATFORM_ID) = 0, 11020000
> > L2 Cache latency is 8
> > Sending 0 to set_l2_register4
> > L2 ECC Checking is enabled
> > L2 Physical Address Range is 4096M
> > Maximum cache mask is 20000
> > L2 Cache Mask is 4000
> > read_l2(2) = 8
> > write_l2(2) = 8
> > L2 Cache size is 512K
> > L2 Cache lines initialized
> > Signed-off-by: Jouni Mettälä <[e-mail]

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Wed, Jan 12, 2011 at 10:55:37PM -0500, Keith Hui wrote:
> The L2 cache on a Coppermine doesn't need any special enabling
> sequence. I just put a 1GHz Coppermine into my board and it boots fine
> showing the full 256k cache. This patch doesn't even apply to them
> anyway.

FYI: Have 450P3 and 2x750P3's here and none of my coreboot logs state anything
about L2 being activated.  From what you're saying, the L2 cache is entirely
automatically activated on Coppermines.

Cheers.

-- 
Roger
http://rogerx.freeshell.org/

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Tue, Jan 11, 2011 at 11:17:17PM -0500, Keith Hui wrote:
> Hi all,
>
> Here is the new L2 cache patch. Sign-off in the patch itself. Still
> very juicy and tasty at 25k. :D
>
> Also done is including cpu/intel/model_68x again in slot_1. Otherwise
> it will die with a Coppermine P3 installed.
>
> My boot log on P2B-LS and a Katmai 600MHz attached.
>
> I have optimized it some more, and added more information and
> meaningful constants as I cross checked the code with Intel's
> documentation. Some debugging messages are different too. Give this a
> good workout.
>
> Cheers
> Keith

I applied this patch as well here, and compiles fine.

NOTE: Although I tested execution and it doesn't appear to break anything, I
have the Coppermine CPU's and not these specific CPU's.

[PATCH] SECC Pentium 2/3 users are gonna love this

[Roger]

On Tue, Jan 11, 2011 at 11:17:17PM -0500, Keith Hui wrote:
> Hi all,
>
> Here is the new L2 cache patch. Sign-off in the patch itself. Still
> very juicy and tasty at 25k. :D
>
> Also done is including cpu/intel/model_68x again in slot_1. Otherwise
> it will die with a Coppermine P3 installed.
>
> My boot log on P2B-LS and a Katmai 600MHz attached.
>
> I have optimized it some more, and added more information and
> meaningful constants as I cross checked the code with Intel's
> documentation. Some debugging messages are different too. Give this a
> good workout.

Acked-by: Roger Zauner <[e-mail]>

Wondering if people are still waiting for acked-by. ... acked it, code compiles
for my "Host bridge: Intel Corporation 440BX/ZX/DX - 82443BX/ZX/DX Host
bridge", but have Coppermine CPU's so the code is ignored.

[PATCH] SECC Pentium 2/3 users are gonna love this

[Keith Hui]

On Fri, Jan 14, 2011 at 3:47 AM, Roger <[e-mail]> wrote:
> On Tue, Jan 11, 2011 at 11:17:17PM -0500, Keith Hui wrote:
> > Hi all,
> >
> > Here is the new L2 cache patch. Sign-off in the patch itself. Still
> > very juicy and tasty at 25k. :D
> >
> > Also done is including cpu/intel/model_68x again in slot_1. Otherwise
> > it will die with a Coppermine P3 installed.
> >
> > My boot log on P2B-LS and a Katmai 600MHz attached.
> >
> > I have optimized it some more, and added more information and
> > meaningful constants as I cross checked the code with Intel's
> > documentation. Some debugging messages are different too. Give this a
> > good workout.
>
> Acked-by: Roger Zauner <[e-mail]>
>
> Wondering if people are still waiting for acked-by. ... acked it, code compiles
> for my "Host bridge: Intel Corporation 440BX/ZX/DX - 82443BX/ZX/DX Host
> bridge", but have Coppermine CPU's so the code is ignored.

Also wondering the status of this.

[PATCH] SECC Pentium 2/3 users are gonna love this

[Keith Hui]

ping3?

On Mon, Jan 31, 2011 at 12:53 PM, Keith Hui <[e-mail]> wrote:
> On Fri, Jan 14, 2011 at 3:47 AM, Roger <[e-mail]> wrote:
> > On Tue, Jan 11, 2011 at 11:17:17PM -0500, Keith Hui wrote:
> > > Hi all,
> > >
> > > Here is the new L2 cache patch. Sign-off in the patch itself. Still
> > > very juicy and tasty at 25k. :D
> > >
> > > Also done is including cpu/intel/model_68x again in slot_1. Otherwise
> > > it will die with a Coppermine P3 installed.
> > >
> > > My boot log on P2B-LS and a Katmai 600MHz attached.
> > >
> > > I have optimized it some more, and added more information and
> > > meaningful constants as I cross checked the code with Intel's
> > > documentation. Some debugging messages are different too. Give this a
> > > good workout.
> >
> > Acked-by: Roger Zauner <[e-mail]>
> >
> > Wondering if people are still waiting for acked-by. ... acked it, code compiles
> > for my "Host bridge: Intel Corporation 440BX/ZX/DX - 82443BX/ZX/DX Host
> > bridge", but have Coppermine CPU's so the code is ignored.
>
> Also wondering the status of this.