Kyösti Mälkki (kyosti.malkki@gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1186
-gerrit
commit 07f63f5bc09b65f36e5b720f2a1b829ec4f6bf00 Author: Kyösti Mälkki kyosti.malkki@gmail.com Date: Fri Jul 6 19:02:56 2012 +0300
AMD northbridges: rewrite CPU allocation
Use of alloc_find_dev() prevents creation of a device duplicates for device_path and is SMP safe.
Reduce scope of variables to make the code more readable and in preparation for refactoring the allocation out of northbridge.c.
Change-Id: I153dc1a5cab4f2eae4ab3a57af02841cb1a261c0 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- src/northbridge/amd/agesa/family10/northbridge.c | 46 ++++++--------- src/northbridge/amd/agesa/family14/northbridge.c | 19 +++--- src/northbridge/amd/agesa/family15/northbridge.c | 41 ++++++-------- src/northbridge/amd/agesa/family15tn/northbridge.c | 41 ++++++-------- src/northbridge/amd/amdfam10/northbridge.c | 60 ++++++++------------ src/northbridge/amd/amdk8/northbridge.c | 53 +++++++----------- 6 files changed, 104 insertions(+), 156 deletions(-)
diff --git a/src/northbridge/amd/agesa/family10/northbridge.c b/src/northbridge/amd/agesa/family10/northbridge.c index 8cc9475..55be491 100644 --- a/src/northbridge/amd/agesa/family10/northbridge.c +++ b/src/northbridge/amd/agesa/family10/northbridge.c @@ -1310,8 +1310,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) /* Find which cpus are present */ cpu_bus = dev->link_list; for (i = 0; i < nodes; i++) { - device_t cdb_dev, cpu; - struct device_path cpu_path; + device_t cdb_dev; unsigned busn, devn; struct bus *pbus;
@@ -1354,7 +1353,8 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
cores_found = 0; // one core cdb_dev = dev_find_slot(busn, PCI_DEVFN(devn, 3)); - if (cdb_dev && cdb_dev->enabled) { + int enable_node = cdb_dev && cdb_dev->enabled; + if (enable_node) { j = pci_read_config32(cdb_dev, 0xe8); cores_found = (j >> 12) & 3; // dev is func 3 if (siblings > 3) @@ -1373,6 +1373,8 @@ static u32 cpu_bus_scan(device_t dev, u32 max) extern CONST OPTIONS_CONFIG_TOPOLOGY ROMDATA TopologyConfiguration; u32 modules = TopologyConfiguration.PlatformNumberOfModules; u32 lapicid_start = 0; + struct device_path cpu_path; + device_t cpu;
/* Build the cpu device path */ cpu_path.type = DEVICE_PATH_APIC; @@ -1393,31 +1395,19 @@ static u32 cpu_bus_scan(device_t dev, u32 max) } cpu_path.apic.apic_id = (lapicid_start * (i/modules + 1)) + ((i % modules) ? (j + (cores_found + 1)) : j);
- /* See if I can find the cpu */ - cpu = find_dev_path(cpu_bus, &cpu_path); - - /* Enable the cpu if I have the processor */ - if (cdb_dev && cdb_dev->enabled) { - if (!cpu) { - cpu = alloc_dev(cpu_bus, &cpu_path); - } - if (cpu) { - cpu->enabled = 1; - } - } - - /* Disable the cpu if I don't have the processor */ - if (cpu && (!cdb_dev || !cdb_dev->enabled)) { - cpu->enabled = 0; - } - - /* Report what I have done */ - if (cpu) { - cpu->path.apic.node_id = i; - cpu->path.apic.core_id = j; - printk(BIOS_DEBUG, "CPU: %s %s\n", - dev_path(cpu), cpu->enabled?"enabled":"disabled"); - } + /* Update CPU in devicetree. */ + if (enable_node) + cpu = alloc_find_dev(cpu_bus, &cpu_path); + else + cpu = find_dev_path(cpu_bus, &cpu_path); + if (!cpu) + continue; + + cpu->enabled = enable_node; + cpu->path.apic.node_id = i; + cpu->path.apic.core_id = j; + printk(BIOS_DEBUG, "CPU: %s %s\n", + dev_path(cpu), cpu->enabled?"enabled":"disabled");
} //j } diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c index a03939c..1d9ef64 100644 --- a/src/northbridge/amd/agesa/family14/northbridge.c +++ b/src/northbridge/amd/agesa/family14/northbridge.c @@ -845,7 +845,6 @@ static void cpu_bus_set_resources(device_t dev) { static u32 cpu_bus_scan(device_t dev, u32 max) { device_t cpu; - struct device_path cpu_path; int apic_id, cores_found;
/* There is only one node for fam14, but there may be multiple cores. */ @@ -858,18 +857,18 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
for (apic_id = 0; apic_id <= cores_found; apic_id++) { + struct device_path cpu_path; + cpu_path.type = DEVICE_PATH_APIC; cpu_path.apic.apic_id = apic_id; cpu = alloc_find_dev(dev->link_list, &cpu_path); - if (cpu) { - cpu->enabled = 1; - cpu->path.apic.node_id = 0; - cpu->path.apic.core_id = apic_id; - printk(BIOS_DEBUG, "CPU: %s %s\n", - dev_path(cpu), cpu->enabled?"enabled":"disabled"); - } else { - cpu->enabled = 0; - } + if (!cpu) + continue; + cpu->enabled = 1; + cpu->path.apic.node_id = 0; + cpu->path.apic.core_id = apic_id; + printk(BIOS_DEBUG, "CPU: %s %s\n", + dev_path(cpu), cpu->enabled?"enabled":"disabled"); } return max; } diff --git a/src/northbridge/amd/agesa/family15/northbridge.c b/src/northbridge/amd/agesa/family15/northbridge.c index d9a153b..3092735 100644 --- a/src/northbridge/amd/agesa/family15/northbridge.c +++ b/src/northbridge/amd/agesa/family15/northbridge.c @@ -1000,8 +1000,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) /* Find which cpus are present */ cpu_bus = dev->link_list; for (i = 0; i < node_nums; i++) { - device_t cdb_dev, cpu; - struct device_path cpu_path; + device_t cdb_dev; unsigned busn, devn; struct bus *pbus;
@@ -1057,6 +1056,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) } else { siblings = 0; //default one core } + int enable_node = cdb_dev && cdb_dev->enabled; printk(BIOS_SPEW, "%s family%xh, core_max=0x%x, core_nums=0x%x, siblings=0x%x\n", dev_path(cdb_dev), 0x0f + family, core_max, core_nums, siblings);
@@ -1064,6 +1064,8 @@ static u32 cpu_bus_scan(device_t dev, u32 max) extern CONST OPTIONS_CONFIG_TOPOLOGY ROMDATA TopologyConfiguration; u32 modules = TopologyConfiguration.PlatformNumberOfModules; u32 lapicid_start = 0; + struct device_path cpu_path; + device_t cpu;
/* Build the cpu device path */ cpu_path.type = DEVICE_PATH_APIC; @@ -1092,28 +1094,19 @@ static u32 cpu_bus_scan(device_t dev, u32 max) printk(BIOS_SPEW, "node 0x%x core 0x%x apicid=0x%x\n", i, j, cpu_path.apic.apic_id);
- /* See if I can find the cpu */ - cpu = find_dev_path(cpu_bus, &cpu_path); - /* Enable the cpu if I have the processor */ - if (cdb_dev && cdb_dev->enabled) { - if (!cpu) { - cpu = alloc_dev(cpu_bus, &cpu_path); - } - if (cpu) { - cpu->enabled = 1; - } - } - /* Disable the cpu if I don't have the processor */ - if (cpu && (!cdb_dev || !cdb_dev->enabled)) { - cpu->enabled = 0; - } - /* Report what I have done */ - if (cpu) { - cpu->path.apic.node_id = i; - cpu->path.apic.core_id = j; - printk(BIOS_DEBUG, "CPU: %s %s\n", - dev_path(cpu), cpu->enabled?"enabled":"disabled"); - } + /* Update CPU in devicetree. */ + if (enable_node) + cpu = alloc_find_dev(cpu_bus, &cpu_path); + else + cpu = find_dev_path(cpu_bus, &cpu_path); + if (!cpu) + continue; + + cpu->enabled = enable_node; + cpu->path.apic.node_id = i; + cpu->path.apic.core_id = j; + printk(BIOS_DEBUG, "CPU: %s %s\n", + dev_path(cpu), cpu->enabled?"enabled":"disabled"); } //j } return max; diff --git a/src/northbridge/amd/agesa/family15tn/northbridge.c b/src/northbridge/amd/agesa/family15tn/northbridge.c index c63890d..36ded65 100644 --- a/src/northbridge/amd/agesa/family15tn/northbridge.c +++ b/src/northbridge/amd/agesa/family15tn/northbridge.c @@ -1007,8 +1007,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) /* Find which cpus are present */ cpu_bus = dev->link_list; for (i = 0; i < node_nums; i++) { - device_t cdb_dev, cpu; - struct device_path cpu_path; + device_t cdb_dev; unsigned busn, devn; struct bus *pbus;
@@ -1064,6 +1063,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) } else { siblings = 0; //default one core } + int enable_node = cdb_dev && cdb_dev->enabled; printk(BIOS_SPEW, "%s family%xh, core_max=0x%x, core_nums=0x%x, siblings=0x%x\n", dev_path(cdb_dev), 0x0f + family, core_max, core_nums, siblings);
@@ -1071,6 +1071,8 @@ static u32 cpu_bus_scan(device_t dev, u32 max) extern CONST OPTIONS_CONFIG_TOPOLOGY ROMDATA TopologyConfiguration; u32 modules = TopologyConfiguration.PlatformNumberOfModules; u32 lapicid_start = 0; + struct device_path cpu_path; + device_t cpu;
/* Build the cpu device path */ cpu_path.type = DEVICE_PATH_APIC; @@ -1099,28 +1101,19 @@ static u32 cpu_bus_scan(device_t dev, u32 max) printk(BIOS_SPEW, "node 0x%x core 0x%x apicid=0x%x\n", i, j, cpu_path.apic.apic_id);
- /* See if I can find the cpu */ - cpu = find_dev_path(cpu_bus, &cpu_path); - /* Enable the cpu if I have the processor */ - if (cdb_dev && cdb_dev->enabled) { - if (!cpu) { - cpu = alloc_dev(cpu_bus, &cpu_path); - } - if (cpu) { - cpu->enabled = 1; - } - } - /* Disable the cpu if I don't have the processor */ - if (cpu && (!cdb_dev || !cdb_dev->enabled)) { - cpu->enabled = 0; - } - /* Report what I have done */ - if (cpu) { - cpu->path.apic.node_id = i; - cpu->path.apic.core_id = j; - printk(BIOS_DEBUG, "CPU: %s %s\n", - dev_path(cpu), cpu->enabled?"enabled":"disabled"); - } + /* Update CPU in devicetree. */ + if (enable_node) + cpu = alloc_find_dev(cpu_bus, &cpu_path); + else + cpu = find_dev_path(cpu_bus, &cpu_path); + if (!cpu) + continue; + + cpu->enabled = enable_node; + cpu->path.apic.node_id = i; + cpu->path.apic.core_id = j; + printk(BIOS_DEBUG, "CPU: %s %s\n", + dev_path(cpu), cpu->enabled?"enabled":"disabled"); } //j } return max; diff --git a/src/northbridge/amd/amdfam10/northbridge.c b/src/northbridge/amd/amdfam10/northbridge.c index aa15fdd..b061acf 100644 --- a/src/northbridge/amd/amdfam10/northbridge.c +++ b/src/northbridge/amd/amdfam10/northbridge.c @@ -1359,8 +1359,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) /* Find which cpus are present */ cpu_bus = dev->link_list; for(i = 0; i < nodes; i++) { - device_t cdb_dev, cpu; - struct device_path cpu_path; + device_t cdb_dev; unsigned busn, devn; struct bus *pbus;
@@ -1403,7 +1402,8 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
cores_found = 0; // one core cdb_dev = dev_find_slot(busn, PCI_DEVFN(devn, 3)); - if (cdb_dev && cdb_dev->enabled) { + int enable_node = cdb_dev && cdb_dev->enabled; + if (enable_node) { j = pci_read_config32(cdb_dev, 0xe8); cores_found = (j >> 12) & 3; // dev is func 3 if (siblings > 3) @@ -1420,47 +1420,33 @@ static u32 cpu_bus_scan(device_t dev, u32 max) }
for (j = 0; j <=jj; j++ ) { + struct device_path cpu_path; + device_t cpu;
/* Build the cpu device path */ cpu_path.type = DEVICE_PATH_APIC; cpu_path.apic.apic_id = i * (nb_cfg_54?(siblings+1):1) + j * (nb_cfg_54?1:64); // ?
- /* See if I can find the cpu */ - cpu = find_dev_path(cpu_bus, &cpu_path); - - /* Enable the cpu if I have the processor */ - if (cdb_dev && cdb_dev->enabled) { - if (!cpu) { - cpu = alloc_dev(cpu_bus, &cpu_path); - } - if (cpu) { - cpu->enabled = 1; - } - } - - /* Disable the cpu if I don't have the processor */ - if (cpu && (!cdb_dev || !cdb_dev->enabled)) { - cpu->enabled = 0; - } - - /* Report what I have done */ - if (cpu) { - cpu->path.apic.node_id = i; - cpu->path.apic.core_id = j; - #if CONFIG_ENABLE_APIC_EXT_ID && (CONFIG_APIC_ID_OFFSET>0) - if(sysconf.enabled_apic_ext_id) { - if(sysconf.lift_bsp_apicid) { - cpu->path.apic.apic_id += sysconf.apicid_offset; - } else - { - if (cpu->path.apic.apic_id != 0) - cpu->path.apic.apic_id += sysconf.apicid_offset; - } + /* Update CPU in devicetree. */ + if (enable_node) + cpu = alloc_find_dev(cpu_bus, &cpu_path); + else + cpu = find_dev_path(cpu_bus, &cpu_path); + if (!cpu) + continue; + +#if CONFIG_ENABLE_APIC_EXT_ID && (CONFIG_APIC_ID_OFFSET>0) + if(sysconf.enabled_apic_ext_id) { + if (cpu->path.apic.apic_id != 0 || sysconf.lift_bsp_apicid) { + cpu->path.apic.apic_id += sysconf.apicid_offset; } - #endif - printk(BIOS_DEBUG, "CPU: %s %s\n", - dev_path(cpu), cpu->enabled?"enabled":"disabled"); } +#endif + cpu->enabled = enable_node; + cpu->path.apic.node_id = i; + cpu->path.apic.core_id = j; + printk(BIOS_DEBUG, "CPU: %s %s\n", + dev_path(cpu), cpu->enabled?"enabled":"disabled");
} //j } diff --git a/src/northbridge/amd/amdk8/northbridge.c b/src/northbridge/amd/amdk8/northbridge.c index bec02f0..a7538a6 100644 --- a/src/northbridge/amd/amdk8/northbridge.c +++ b/src/northbridge/amd/amdk8/northbridge.c @@ -1250,8 +1250,7 @@ static u32 cpu_bus_scan(device_t dev, u32 max) /* Find which cpus are present */ cpu_bus = dev->link_list; for(i = 0; i < sysconf.nodes; i++) { - device_t cpu_dev, cpu; - struct device_path cpu_path; + device_t cpu_dev;
/* Find the cpu's pci device */ cpu_dev = dev_find_slot(0, PCI_DEVFN(0x18 + i, 3)); @@ -1275,7 +1274,8 @@ static u32 cpu_bus_scan(device_t dev, u32 max) }
e0_later_single_core = 0; - if (cpu_dev && cpu_dev->enabled) { + int enable_node = cpu_dev && cpu_dev->enabled; + if (enable_node) { j = pci_read_config32(cpu_dev, 0xe8); j = (j >> 12) & 3; // dev is func 3 printk(BIOS_DEBUG, " %s siblings=%d\n", dev_path(cpu_dev), j); @@ -1322,45 +1322,32 @@ static u32 cpu_bus_scan(device_t dev, u32 max) #endif
for (j = 0; j <=jj; j++ ) { + struct device_path cpu_path; + device_t cpu;
/* Build the cpu device path */ cpu_path.type = DEVICE_PATH_APIC; cpu_path.apic.apic_id = i * (nb_cfg_54?(siblings+1):1) + j * (nb_cfg_54?1:8);
- /* See if I can find the cpu */ - cpu = find_dev_path(cpu_bus, &cpu_path); + /* Update CPU in devicetree. */ + if (enable_node) + cpu = alloc_find_dev(cpu_bus, &cpu_path); + else + cpu = find_dev_path(cpu_bus, &cpu_path); + if (!cpu) + continue;
- /* Enable the cpu if I have the processor */ - if (cpu_dev && cpu_dev->enabled) { - if (!cpu) { - cpu = alloc_dev(cpu_bus, &cpu_path); - } - if (cpu) { - cpu->enabled = 1; + if(sysconf.enabled_apic_ext_id) { + if (cpu->path.apic.apic_id != 0 || sysconf.lift_bsp_apicid) { + cpu->path.apic.apic_id += sysconf.apicid_offset; } }
- /* Disable the cpu if I don't have the processor */ - if (cpu && (!cpu_dev || !cpu_dev->enabled)) { - cpu->enabled = 0; - } - - /* Report what I have done */ - if (cpu) { - cpu->path.apic.node_id = i; - cpu->path.apic.core_id = j; - if(sysconf.enabled_apic_ext_id) { - if(sysconf.lift_bsp_apicid) { - cpu->path.apic.apic_id += sysconf.apicid_offset; - } else - { - if (cpu->path.apic.apic_id != 0) - cpu->path.apic.apic_id += sysconf.apicid_offset; - } - } - printk(BIOS_DEBUG, "CPU: %s %s\n", - dev_path(cpu), cpu->enabled?"enabled":"disabled"); - } + cpu->enabled = enable_node; + cpu->path.apic.node_id = i; + cpu->path.apic.core_id = j; + printk(BIOS_DEBUG, "CPU: %s %s\n", + dev_path(cpu), cpu->enabled?"enabled":"disabled");
} //j }
-
Kyösti Mälkki