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>
Reviewed-on: http://review.coreboot.org/1186
Tested-by: build bot (Jenkins)
Reviewed-by: Anton Kochkov <anton.kochkov@gmail.com>
Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

src/northbridge/amd/agesa/family10/northbridge.c

@@ -1311,8 +1311,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;
 
@@ -1355,7 +1354,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)
@@ -1374,6 +1374,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;
@@ -1394,31 +1396,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 */
+			/* 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 (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->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
 	}

src/northbridge/amd/agesa/family14/northbridge.c

@@ -832,7 +832,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. */
@@ -845,18 +844,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) {
+		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");
-		} else {
-			cpu->enabled = 0;
-		}
 	}
 	return max;
 }

src/northbridge/amd/agesa/family15/northbridge.c

@@ -988,8 +988,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;
 
@@ -1045,6 +1044,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);
 
@@ -1052,6 +1052,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;
@@ -1080,28 +1082,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 */
+			/* Update CPU in devicetree. */
+			if (enable_node)
+				cpu = alloc_find_dev(cpu_bus, &cpu_path);
+			else
 				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) {
+			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;

src/northbridge/amd/agesa/family15tn/northbridge.c

@@ -996,8 +996,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;
 
@@ -1053,6 +1052,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);
 
@@ -1060,6 +1060,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;
@@ -1088,28 +1090,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 */
+			/* Update CPU in devicetree. */
+			if (enable_node)
+				cpu = alloc_find_dev(cpu_bus, &cpu_path);
+			else
 				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) {
+			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;

src/northbridge/amd/amdfam10/northbridge.c

@@ -1345,8 +1345,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;
 
@@ -1389,7 +1388,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)
@@ -1406,47 +1406,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 */
+			/* 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 (cdb_dev && cdb_dev->enabled) {
-				if (!cpu) {
-					cpu = alloc_dev(cpu_bus, &cpu_path);
-				}
-				if (cpu) {
-					cpu->enabled = 1;
+#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;
 				}
 			}
-
-			/* 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) {
+#endif
+			cpu->enabled = enable_node;
 			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;
-					}
-				}
-	#endif
 			printk(BIOS_DEBUG, "CPU: %s %s\n",
 				dev_path(cpu), cpu->enabled?"enabled":"disabled");
-			}
 
 		} //j
 	}

src/northbridge/amd/amdk8/northbridge.c

@@ -1239,8 +1239,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));
@@ -1264,7 +1263,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);
@@ -1311,45 +1311,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 */
+			/* 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->enabled = enable_node;
 			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");
-			}
 
 		} //j
 	}