Patchwork Istanbul support

login
register
about
Submitter Bao, Zheng
Date 2010-03-10 03:30:25
Message ID <DD1CC71B621B004FA76856E5129D6B170382671B@sbjgexmb1.amd.com>
Download mbox | patch
Permalink /patch/1033/
State Not Applicable
Headers show

Comments

Bao, Zheng - 2010-03-10 03:30:25
index ac62981..067560d 100644
 }

I assume the line
pci_write_config32(NODE_PCI(nodeid, 0), 0x168, dword);
should be put outside the loop.

Everything seems to be fine. I don't have Istanbul to test. I have read
every changes and they all look good.

Signed-off-by: Zheng Bao <zheng.bao@amd.com>



> -----Original Message-----
> From: coreboot-bounces@coreboot.org
[mailto:coreboot-bounces@coreboot.org]
> On Behalf Of Arne Georg Gleditsch
> Sent: Friday, March 05, 2010 10:41 PM
> To: coreboot@coreboot.org
> Subject: [coreboot] [PATCH] Istanbul support
> 
> Hi,
> 
> The following patch implements Opteron Fam 10 rev D (aka Istanbul)
> support for coreboot.  I have not updated MAX_CPUS for all fam10
> mainboards, but it might make sense to multiply those by 1.5.
> 
> Signed-off-by: Arne Georg Gleditsch <arne.gleditsch@numascale.com>
> 
> --
> 								Arne.
Arne Georg Gleditsch - 2010-03-10 07:27:15
On 10. mars 2010 04:30, Bao, Zheng wrote:
> index ac62981..067560d 100644
> --- a/src/cpu/amd/quadcore/quadcore.c
> +++ b/src/cpu/amd/quadcore/quadcore.c
> @@ -69,11 +72,10 @@ static void real_start_other_core(u32 nodeid, u32
> cores)
>  
>  	if(cores > 1) {
>  		dword = pci_read_config32(NODE_PCI(nodeid, 0), 0x168);
> -		dword |= (1 << 0);	// core2
> -		if(cores > 2) {		// core3
> -			dword |= (1 << 1);
> +		for (i = 0; i < cores - 1; i++) {
> +			dword |= 1 << i;
> +			pci_write_config32(NODE_PCI(nodeid, 0), 0x168,
> dword);
>  		}
> -		pci_write_config32(NODE_PCI(nodeid, 0), 0x168, dword);
>  	}
>  }
> 
> I assume the line
> pci_write_config32(NODE_PCI(nodeid, 0), 0x168, dword);
> should be put outside the loop.

Yes, well, I originally had that as "dword |= (0xf >> (5-cores);
pci_write_config32(..)" or something.  However, after reducing the
loglevel below "debug" and encountering hangs that seemed to be conflict
issues with the core startup, I added the loop to be able to add delays
between startup of the individual cores during debugging.  As it stands,
the write could just as well be outside the loop.

Regarding hangs, I suspect this might be a PCI config race condition
between the cores.  I tried to adjust my configuration to use MMCONFIG
to address this.  It didn't seem to be supported out of the box, and I
didn't have the time to look into it further at the moment.  I still
intend to do so, but it would be nice to know if anyone is using
MMCONFIG with fam10 configurations.  Is it supposed to work?

Patch

--- a/src/cpu/amd/quadcore/quadcore.c
+++ b/src/cpu/amd/quadcore/quadcore.c
@@ -69,11 +72,10 @@  static void real_start_other_core(u32 nodeid, u32
cores)
 
 	if(cores > 1) {
 		dword = pci_read_config32(NODE_PCI(nodeid, 0), 0x168);
-		dword |= (1 << 0);	// core2
-		if(cores > 2) {		// core3
-			dword |= (1 << 1);
+		for (i = 0; i < cores - 1; i++) {
+			dword |= 1 << i;
+			pci_write_config32(NODE_PCI(nodeid, 0), 0x168,
dword);
 		}
-		pci_write_config32(NODE_PCI(nodeid, 0), 0x168, dword);
 	}