[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