-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

During the suspend/resume programming I came to an issue that first 4KB of
memory must be clear with 0s because otherwise the resources of K8 will be
totally messed up.

It took long to figure it out and here it is:

res = probe_resource(dev, 0x100 + (reg | link));

This is called with dev = NULL and this is no good for probe_resource at all.
The attached patch fixes the potential problems and of course the problem
itself. On one particular place was missing test if the device really exists.
This was copied to fam10 and perhaps the same issue is in v3 (DID NOT check).
The rest of the patch is just very paranoid and do all checkings.

This was tested in SimNOW but I believe it will work on real hw too.

Signed-off-by: Rudolf Marek <[email protected]>

Anyone knows if SimNOW can do breaks on memory ranges?

Because it was quite stupid to get to the bug. When the resources just worked
again I used data breakpoint to see what routine it is. Problem was that I
needed to find the address where it stop to work.

Maybe we really should allow writes to mem only in _RAMBASE - MEMTOPK range (of
course with some exception for table stuff).

- --- src/cpu/amd/car/clear_init_ram.c<-->(revision 4026)
+++ src/cpu/amd/car/clear_init_ram.c<-->(working copy)
@@ -6,7 +6,23 @@
 <----->// gcc 3.4.5 will inline the copy_and_run and clear_init_ram in
post_cache_as_ram
 <----->// will reuse %edi as 0 from clear_memory for copy_and_run part,
actually it is increased already
 <----->// so noline clear_init_ram
- -        clear_memory(0,  ((CONFIG_LB_MEM_TOPK<<10) - DCACHE_RAM_SIZE));
+        clear_memory(_RAMBASE,  ((CONFIG_LB_MEM_TOPK<<10) -_RAMBASE -
DCACHE_RAM_SIZE));
+<----->//512 NOT OK - I previously though this works so I need to test MORE :/
+<----->//256 NOT OK
+<----->//384 NOT OK
+<----->//448 NOT OK
+<----->//480 NOT OK
+<----->//496 NOT OK
+<----->//504 NOT OK
+<----->//508 NOT OK
+<----->// 768 NOT OK
+<----->//896 NOT OK
+<----->//960 OK
+<----->//928 OK
+<----->//912 NOT OK
+<----->//920 NOT OK
+<----->//924 OK
+        clear_memory(0, 924);
.

Oh btw the suspend/resume works, but the code needs to be fixed on some places.
There is a  copy_secondary_start_to_1m_below which just uses first 64KB of mem
for some reason - I dont understand why it cannot be realocable to something
different addr. Please check and tell.

Thanks,
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknKdOAACgkQ3J9wPJqZRNXabQCdFRWA4pYWDuY+VDyFROATejRC
oDcAn02iYJM4fivIzcpJlQ/D9Qu4DFYE
=dtVh
-----END PGP SIGNATURE-----
Index: src/northbridge/amd/amdk8/northbridge.c
===================================================================
--- src/northbridge/amd/amdk8/northbridge.c	(revision 4026)
+++ src/northbridge/amd/amdk8/northbridge.c	(working copy)
@@ -291,11 +291,12 @@
 	unsigned nodeid, link;
 	int result;
 	res = 0;
-	for(nodeid = 0; !res && (nodeid < 8); nodeid++) {
+	for(nodeid = 0; !res && (nodeid < FX_DEVS); nodeid++) {
 		device_t dev;
 		dev = __f0_dev[nodeid];
 		for(link = 0; !res && (link < 3); link++) {
-			res = probe_resource(dev, 0x100 + (reg | link));
+			if (dev)
+				res = probe_resource(dev, 0x100 + (reg | link));
 		}
 	}
 	result = 2;
@@ -760,19 +761,20 @@
 		mem_hole.hole_startk = HW_MEM_HOLE_SIZEK;
 		mem_hole.node_id = -1;
 
-		for (i = 0; i < 8; i++) {
+		for (i = 0; i < FX_DEVS; i++) {
 			uint32_t base;
 			uint32_t hole;
 			base  = f1_read_config32(0x40 + (i << 3));
 			if ((base & ((1<<1)|(1<<0))) != ((1<<1)|(1<<0))) {
 				continue;
 			}
-
-			hole = pci_read_config32(__f1_dev[i], 0xf0);
-			if(hole & 1) { // we find the hole
-				mem_hole.hole_startk = (hole & (0xff<<24)) >> 10;
-				mem_hole.node_id = i; // record the node No with hole
-				break; // only one hole
+			if (__f1_dev[i]) {
+				hole = pci_read_config32(__f1_dev[i], 0xf0);
+				if(hole & 1) { // we find the hole
+					mem_hole.hole_startk = (hole & (0xff<<24)) >> 10;
+					mem_hole.node_id = i; // record the node No with hole
+					break; // only one hole
+				}
 			}
 		}
 
@@ -834,15 +836,15 @@
 	limit = f1_read_config32(0x44 + (i << 3));
 	f1_write_config32(0x44 + (i << 3),limit - (hole_sizek << 2));
 	dev = __f1_dev[i];
-	hoist = pci_read_config32(dev, 0xf0);
-	if(hoist & 1) {
-		pci_write_config32(dev, 0xf0, 0);
+	if (dev) {
+		hoist = pci_read_config32(dev, 0xf0);
+		if(hoist & 1) {
+			pci_write_config32(dev, 0xf0, 0);
+		} else {
+			base = pci_read_config32(dev, 0x40 + (i << 3));
+			f1_write_config32(0x40 + (i << 3),base - (hole_sizek << 2));
+		}
 	}
-	else {
-		base = pci_read_config32(dev, 0x40 + (i << 3));
-		f1_write_config32(0x40 + (i << 3),base - (hole_sizek << 2));
-	}
-
 }
 
 static uint32_t hoist_memory(unsigned long hole_startk, int i)
@@ -878,7 +880,7 @@
 		base |= (4*1024*1024)<<2;
 		f1_write_config32(0x40 + (i<<3), base);
 	}
-	else
+	else if (dev)
 	{
 		hoist = /* hole start address */
 			((hole_startk << 10) & 0xff000000) +
@@ -1020,7 +1022,7 @@
 		#if HW_MEM_HOLE_SIZE_AUTO_INC == 1
 			//We need to double check if the mmio_basek is valid for hole setting, if it is equal to basek, we need to decrease it some
 			uint32_t basek_pri;
-			for (i = 0; i < 8; i++) {
+			for (i = 0; i < FX_DEVS; i++) {
 				uint32_t base;
 				uint32_t basek;
 				base  = f1_read_config32(0x40 + (i << 3));
@@ -1045,7 +1047,7 @@
 #endif
 
 	idx = 0x10;
-	for(i = 0; i < 8; i++) {
+	for(i = 0; i < FX_DEVS; i++) {
 		uint32_t base, limit;
 		unsigned basek, limitk, sizek;
 		base  = f1_read_config32(0x40 + (i << 3));
Index: src/northbridge/amd/amdfam10/northbridge.c
===================================================================
--- src/northbridge/amd/amdfam10/northbridge.c	(revision 4026)
+++ src/northbridge/amd/amdfam10/northbridge.c	(working copy)
@@ -339,7 +339,8 @@
 		device_t dev;
 		dev = __f0_dev[nodeid];
 		for(link = 0; !res && (link < 8); link++) {
-			res = probe_resource(dev, 0x1000 + reg + (link<<16)); // 8 links, 0x1000 man f1,
+			if (dev)
+				res = probe_resource(dev, 0x1000 + reg + (link<<16)); // 8 links, 0x1000 man f1,
 		}
 	}
 	result = 2;

Attachment: fix_null_devs.patch.sig
Description: Binary data

-- 
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to