[coreboot] K8 RAMinit problematic code

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Sep 1 17:12:17 CEST 2008


Hi,

this patch is not to be applied, but an annotation from my tree.

Index: northbridge/amd/k8/dqs.c
===================================================================
--- northbridge/amd/k8/dqs.c	(Revision 853)
+++ northbridge/amd/k8/dqs.c	(Arbeitskopie)
@@ -2001,6 +2001,7 @@
 static inline void train_ram_on_node(unsigned nodeid, unsigned coreid, struct sys_info *sysinfo, unsigned retcall)
 {
 	if(coreid) return; // only do it on core0
+#error This is broken beyond repair. We need to use the generic global variable infrastructure, especially if we relocate the global variables without telling anybody.
 	struct sys_info *sysinfox = ((CONFIG_LB_MEM_TOPK<<10) - DCACHE_RAM_GLOBAL_VAR_SIZE);
 	wait_till_sysinfo_in_ram(); // use pci to get it
 
@@ -2012,6 +2013,7 @@
 		sysinfo->mem_trained[nodeid] = sysinfox->mem_trained[nodeid];
 		memcpy(&sysinfo->ctrl[nodeid], &sysinfox->ctrl[nodeid], sizeof(struct mem_controller));
 	#else
+#error Broken here as well.
 		memcpy(sysinfo, sysinfox, DCACHE_RAM_GLOBAL_VAR_SIZE);
 	#endif
 		set_top_mem_ap(sysinfo->tom_k, sysinfo->tom2_k); // keep the ap's tom consistent with bsp's



Even if the stuff above is fixed, we absolutely have to finish the
concept of accessing global variables by adding locking and/or spelling
out the access rules, especially for the CAR stage.

Oh, and can we please mark struct sys_info as const where possible and
also not pass it around as parameter. Especially if we perform stack
relocation, this can lead to really nasty problems.
Thanks.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list