[coreboot] New patch to review for coreboot: f23bb55 AMD northbridges: drop node_id and core_id

Kyösti Mälkki (kyosti.malkki@gmail.com) gerrit at coreboot.org
Tue Jul 10 09:50:44 CEST 2012


Kyösti Mälkki (kyosti.malkki at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1200

-gerrit

commit f23bb551593f8f0f004df222e23e53b3caecb5af
Author: Kyösti Mälkki <kyosti.malkki at gmail.com>
Date:   Mon Jul 9 20:38:28 2012 +0300

    AMD northbridges: drop node_id and core_id
    
    Field apic.core_id was never referenced after being set.
    Field apic.node_id has some use but can be derived from apic_id.
    
    Critical part of the patch is, if cpu_topology() resolves to the
    same node_id value as previously.
    
    Change-Id: I976900f187d205aec4f4de2c3175a3704af241bf
    Signed-off-by: Kyösti Mälkki <kyosti.malkki at gmail.com>
---
 src/cpu/amd/dualcore/amd_sibling.c                 |    2 -
 src/include/cpu/amd/amdfam10_sysconf.h             |    1 +
 src/include/cpu/amd/amdk8_sysconf.h                |    1 +
 src/include/device/path.h                          |    2 -
 src/northbridge/amd/agesa/family10/northbridge.c   |    2 -
 src/northbridge/amd/agesa/family14/northbridge.c   |    2 -
 src/northbridge/amd/agesa/family15/northbridge.c   |    2 -
 src/northbridge/amd/agesa/family15tn/northbridge.c |    2 -
 src/northbridge/amd/amdfam10/acpi.c                |   17 ++++++++++---
 src/northbridge/amd/amdfam10/northbridge.c         |   23 ++++++++++++++---
 src/northbridge/amd/amdk8/acpi.c                   |    9 +++++-
 src/northbridge/amd/amdk8/northbridge.c            |   26 +++++++++++++++----
 12 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/src/cpu/amd/dualcore/amd_sibling.c b/src/cpu/amd/dualcore/amd_sibling.c
index af96265..ea5114c 100644
--- a/src/cpu/amd/dualcore/amd_sibling.c
+++ b/src/cpu/amd/dualcore/amd_sibling.c
@@ -187,8 +187,6 @@ void amd_sibling_init(device_t cpu)
 			new->initialized = 0;
 		}
 
-                new->path.apic.node_id = cpu->path.apic.node_id;
-                new->path.apic.core_id = i;
 
 #if 1
 		printk(BIOS_DEBUG, "CPU: %u has sibling %u\n",
diff --git a/src/include/cpu/amd/amdfam10_sysconf.h b/src/include/cpu/amd/amdfam10_sysconf.h
index 519dde6..fb973a25 100644
--- a/src/include/cpu/amd/amdfam10_sysconf.h
+++ b/src/include/cpu/amd/amdfam10_sysconf.h
@@ -72,4 +72,5 @@ extern struct amdfam10_sysconf_t sysconf;
 
 void get_sblk_pci1234(void);
 void get_bus_conf(void);
+void cpu_topology(u32 apic_id, u16 *node_id, u16 *core_id);
 #endif
diff --git a/src/include/cpu/amd/amdk8_sysconf.h b/src/include/cpu/amd/amdk8_sysconf.h
index 3ae35fd..a10ae89 100644
--- a/src/include/cpu/amd/amdk8_sysconf.h
+++ b/src/include/cpu/amd/amdk8_sysconf.h
@@ -27,4 +27,5 @@ extern struct amdk8_sysconf_t sysconf;
 
 void get_sblk_pci1234(void);
 void get_bus_conf(void);
+void cpu_topology(u32 apic_id, u16 *node_id, u16 *core_id);
 #endif
diff --git a/src/include/device/path.h b/src/include/device/path.h
index 018fb93..fb1ecbb 100644
--- a/src/include/device/path.h
+++ b/src/include/device/path.h
@@ -38,8 +38,6 @@ struct i2c_path
 struct apic_path
 {
 	unsigned apic_id;
-	unsigned node_id;
-	unsigned core_id;
 	unsigned index;
 };
 
diff --git a/src/northbridge/amd/agesa/family10/northbridge.c b/src/northbridge/amd/agesa/family10/northbridge.c
index 3088f54..8d4c334 100644
--- a/src/northbridge/amd/agesa/family10/northbridge.c
+++ b/src/northbridge/amd/agesa/family10/northbridge.c
@@ -1430,8 +1430,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 
 			/* 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");
 			}
diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c
index f7c3b8e..7f6b6f7 100644
--- a/src/northbridge/amd/agesa/family14/northbridge.c
+++ b/src/northbridge/amd/agesa/family14/northbridge.c
@@ -835,8 +835,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 		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 {
diff --git a/src/northbridge/amd/agesa/family15/northbridge.c b/src/northbridge/amd/agesa/family15/northbridge.c
index 5b22707..5951154 100644
--- a/src/northbridge/amd/agesa/family15/northbridge.c
+++ b/src/northbridge/amd/agesa/family15/northbridge.c
@@ -1083,8 +1083,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 			}
 			/* 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");
 			}
diff --git a/src/northbridge/amd/agesa/family15tn/northbridge.c b/src/northbridge/amd/agesa/family15tn/northbridge.c
index d967793..bd63b67 100644
--- a/src/northbridge/amd/agesa/family15tn/northbridge.c
+++ b/src/northbridge/amd/agesa/family15tn/northbridge.c
@@ -1067,8 +1067,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 			}
 			/* 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");
 			}
diff --git a/src/northbridge/amd/amdfam10/acpi.c b/src/northbridge/amd/amdfam10/acpi.c
index 87c2d8c..b878a59 100644
--- a/src/northbridge/amd/amdfam10/acpi.c
+++ b/src/northbridge/amd/amdfam10/acpi.c
@@ -53,6 +53,8 @@ unsigned long acpi_create_srat_lapics(unsigned long current)
 	int cpu_index = 0;
 
 	for(cpu = all_devices; cpu; cpu = cpu->next) {
+		u16 node_id, core_id;
+
 		if ((cpu->path.type != DEVICE_PATH_APIC) ||
 		   (cpu->bus->dev->path.type != DEVICE_PATH_APIC_CLUSTER)) {
 			continue;
@@ -60,8 +62,11 @@ unsigned long acpi_create_srat_lapics(unsigned long current)
 		if (!cpu->enabled) {
 			continue;
 		}
-		printk(BIOS_DEBUG, "SRAT: lapic cpu_index=%02x, node_id=%02x, apic_id=%02x\n", cpu_index, cpu->path.apic.node_id, cpu->path.apic.apic_id);
-		current += acpi_create_srat_lapic((acpi_srat_lapic_t *)current, cpu->path.apic.node_id, cpu->path.apic.apic_id);
+
+		cpu_topology(cpu->path.apic.apic_id, &node_id, &core_id);
+
+		printk(BIOS_DEBUG, "SRAT: lapic cpu_index=%02x, node_id=%02x, apic_id=%02x\n", cpu_index, node_id, cpu->path.apic.apic_id);
+		current += acpi_create_srat_lapic((acpi_srat_lapic_t *)current, node_id, cpu->path.apic.apic_id);
 		cpu_index++;
 	}
 	return current;
@@ -353,6 +358,8 @@ unsigned long acpi_add_ssdt_pstates(acpi_rsdp_t *rsdp, unsigned long current)
 	}
 
 	for(cpu = all_devices; cpu; cpu = cpu->next) {
+		u16 node_id, core_id;
+
 		if ((cpu->path.type != DEVICE_PATH_APIC) ||
 		   (cpu->bus->dev->path.type != DEVICE_PATH_APIC_CLUSTER)) {
 			continue;
@@ -360,14 +367,16 @@ unsigned long acpi_add_ssdt_pstates(acpi_rsdp_t *rsdp, unsigned long current)
 		if (!cpu->enabled) {
 			 continue;
 		}
-		printk(BIOS_DEBUG, "ACPI: pstate cpu_index=%02x, node_id=%02x, core_id=%02x\n", cpu_index, cpu->path.apic.node_id, cpu->path.apic.core_id);
+
+		cpu_topology(cpu->path.apic.apic_id, &node_id, &core_id);
+		printk(BIOS_DEBUG, "ACPI: pstate cpu_index=%02x, node_id=%02x\n", cpu_index, node_id);
 
 		current	  = ALIGN(current, 16);
 		ssdt = (acpi_header_t *)current;
 		memcpy(ssdt, AmlCode_sspr, sizeof(acpi_header_t));
 		current += ssdt->length;
 		memcpy(ssdt, AmlCode_sspr, ssdt->length);
-		update_sspr((void*)ssdt,cpu->path.apic.node_id, cpu_index);
+		update_sspr((void*)ssdt, node_id, cpu_index);
 		/* recalculate checksum */
 		ssdt->checksum = 0;
 		ssdt->checksum = acpi_checksum((unsigned char *)ssdt,ssdt->length);
diff --git a/src/northbridge/amd/amdfam10/northbridge.c b/src/northbridge/amd/amdfam10/northbridge.c
index 3b20684..e5ba5ea 100644
--- a/src/northbridge/amd/amdfam10/northbridge.c
+++ b/src/northbridge/amd/amdfam10/northbridge.c
@@ -1241,6 +1241,9 @@ static void add_more_links(device_t dev, unsigned total_links)
 	last->next = NULL;
 }
 
+static unsigned int siblings = 0;
+static unsigned int nb_cfg_54 = 0;
+
 static u32 cpu_bus_scan(device_t dev, u32 max)
 {
 	struct bus *cpu_bus;
@@ -1250,8 +1253,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 #endif
 	int i,j;
 	int nodes;
-	unsigned nb_cfg_54;
-	unsigned siblings;
 	int cores_found;
 	int disable_siblings;
 	unsigned ApicIdCoreIdSize;
@@ -1423,8 +1424,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 
 			/* 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) {
@@ -1445,6 +1444,22 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 	return max;
 }
 
+void cpu_topology(u32 apic_id, u16 *node_id, u16 *core_id)
+{
+	// i = node_id, j = core_id
+	// cpu_path.apic.apic_id = i * (nb_cfg_54?(siblings+1):1) + j * (nb_cfg_54?1:64); // ?
+
+	if (nb_cfg_54) {
+		// cpu_path.apic.apic_id = node_id * (siblings+1) + core_id
+		*node_id = apic_id / (siblings+1);
+		*core_id = apic_id - (*node_id) * (siblings+1);
+	} else {
+		// cpu_path.apic.apic_id = node_id + core_id * 64;
+		*node_id = apic_id % 64;
+		*core_id = apic_id / 64;
+	}
+}
+
 static void cpu_bus_init(device_t dev)
 {
 	initialize_cpus(dev->link_list);
diff --git a/src/northbridge/amd/amdk8/acpi.c b/src/northbridge/amd/amdk8/acpi.c
index ba04da3..927b648 100644
--- a/src/northbridge/amd/amdk8/acpi.c
+++ b/src/northbridge/amd/amdk8/acpi.c
@@ -60,6 +60,8 @@ unsigned long acpi_create_srat_lapics(unsigned long current)
 	int cpu_index = 0;
 
 	for(cpu = all_devices; cpu; cpu = cpu->next) {
+		u16 node_id, core_id;
+
 		if ((cpu->path.type != DEVICE_PATH_APIC) ||
 		    (cpu->bus->dev->path.type != DEVICE_PATH_APIC_CLUSTER)) {
 			continue;
@@ -67,8 +69,11 @@ unsigned long acpi_create_srat_lapics(unsigned long current)
 		if (!cpu->enabled) {
 			continue;
 		}
-		printk(BIOS_DEBUG, "SRAT: lapic cpu_index=%02x, node_id=%02x, apic_id=%02x\n", cpu_index, cpu->path.apic.node_id, cpu->path.apic.apic_id);
-		current += acpi_create_srat_lapic((acpi_srat_lapic_t *)current, cpu->path.apic.node_id, cpu->path.apic.apic_id);
+
+		cpu_topology(cpu->path.apic.apic_id, &node_id, &core_id);
+
+		printk(BIOS_DEBUG, "SRAT: lapic cpu_index=%02x, node_id=%02x, apic_id=%02x\n", cpu_index, node_id, cpu->path.apic.apic_id);
+		current += acpi_create_srat_lapic((acpi_srat_lapic_t *)current, node_id, cpu->path.apic.apic_id);
 		cpu_index++;
 	}
 	return current;
diff --git a/src/northbridge/amd/amdk8/northbridge.c b/src/northbridge/amd/amdk8/northbridge.c
index cf80389..fba0e04 100644
--- a/src/northbridge/amd/amdk8/northbridge.c
+++ b/src/northbridge/amd/amdk8/northbridge.c
@@ -1158,21 +1158,20 @@ static void add_more_links(device_t dev, unsigned total_links)
 	last->next = NULL;
 }
 
+static unsigned int siblings = 0;
+static unsigned int nb_cfg_54 = 0;
+
 static u32 cpu_bus_scan(device_t dev, u32 max)
 {
 	struct bus *cpu_bus;
 	device_t dev_mc;
 	int bsp_apicid;
 	int i,j;
-	unsigned nb_cfg_54;
-	unsigned siblings;
 	int e0_later_single_core;
 	int disable_siblings;
 
-	nb_cfg_54 = 0;
 	sysconf.enabled_apic_ext_id = 0;
 	sysconf.lift_bsp_apicid = 0;
-	siblings = 0;
 
 	/* Find the bootstrap processors apicid */
 	bsp_apicid = lapicid();
@@ -1309,8 +1308,6 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 
 			/* 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;
@@ -1329,6 +1326,23 @@ static u32 cpu_bus_scan(device_t dev, u32 max)
 	return max;
 }
 
+void cpu_topology(u32 apic_id, u16 *node_id, u16 *core_id)
+{
+	// i = node_id, j = core_id
+	// cpu_path.apic.apic_id = i * (nb_cfg_54?(siblings+1):1) + j * (nb_cfg_54?1:8);
+
+	if (nb_cfg_54) {
+		// apic_id = node_id * (siblings+1) + core_id
+		*node_id = apic_id / (siblings+1);
+		*core_id = apic_id - (*node_id) * (siblings+1);
+
+	} else {
+		// apic_id = node_id + core_id * 8;
+		*node_id = apic_id % 8;
+		*core_id = apic_id / 8;
+	}
+}
+
 static void cpu_bus_init(device_t dev)
 {
 #if CONFIG_WAIT_BEFORE_CPUS_INIT




More information about the coreboot mailing list