[coreboot] K8 RAMinit problematic code

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


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));
+#error Broken here as well.
 		memcpy(sysinfo, sysinfox, DCACHE_RAM_GLOBAL_VAR_SIZE);
 		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.



More information about the coreboot mailing list