[M] Change in coreboot[master]: soc/intel/common: Fix cpu index calculation

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has submitted this change. ( https://review.coreboot.org/c/coreboot/+/70751 )

(

8 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: soc/intel/common: Fix cpu index calculation
......................................................................

soc/intel/common: Fix cpu index calculation

get_cpu_index() helper function returns cpu's index based on it's APIC
id position from the ascending order list of cpus' APIC IDs.
In order to calculate the cpu's index, the helper function needs to
traverse through each cpu node to find their APIC IDs. So, the function
traverse the CPU node list from the cpu whose APIC ID is 0 assuming it
is the first cpu node in the list. This logic works fine where BSP's
APIC ID is 0. But, starting from MTL, APIC ID for BSP need not be 0 as
APIC ID numbering first get assigned for CPU Die Efficient cores, then
Performance cores.
Please refer section# 6.1 of doc#643504 for more details on APIC IDs.

Considering the APIC Id allotment for MTL cores, as existing code
traversing begins from the cpu that has APIC Id#0 which may not be the
first cpu node in the list so index calculation results in wrong value.

The patch addresses above described issue by traversing all the CPU
nodes to calculate the cpu index. Also, prevents inconsistent report
of /sys/devices/system/cpu/cpu*/cpufreq/* and
/sys/devices/system/cpu/cpuXX/acpi_cppc  on each reboot.

TEST=Verified that the get_cpu_index helper function returns the correct
index id for a CPU on Rex.

The coreboot log with code instrumentation, before this patch:

[DEBUG]   my_apic_id:0x10 cpu_index: 0x6
[DEBUG]   my_apic_id:0x11 cpu_index: 0x6
[DEBUG]   my_apic_id:0x42 cpu_index: 0x6
[DEBUG]   my_apic_id:0x21 cpu_index: 0x6
[DEBUG]   my_apic_id:0x40 cpu_index: 0x6
[DEBUG]   my_apic_id:0x31 cpu_index: 0x6
[DEBUG]   my_apic_id:0x39 cpu_index: 0x6
[DEBUG]   my_apic_id:0xa cpu_index: 0x3
[DEBUG]   my_apic_id:0x0 cpu_index: 0x0
[DEBUG]   my_apic_id:0x8 cpu_index: 0x2
[DEBUG]   my_apic_id:0x4 cpu_index: 0x2
[DEBUG]   my_apic_id:0x28 cpu_index: 0x6
[DEBUG]   my_apic_id:0x2 cpu_index: 0x1
[DEBUG]   my_apic_id:0x38 cpu_index: 0x6
[DEBUG]   my_apic_id:0x29 cpu_index: 0x6
[DEBUG]   my_apic_id:0xe cpu_index: 0x5
[DEBUG]   my_apic_id:0x6 cpu_index: 0x2
[DEBUG]   my_apic_id:0x20 cpu_index: 0x6
[DEBUG]   my_apic_id:0x30 cpu_index: 0x6
[DEBUG]   my_apic_id:0x19 cpu_index: 0x6
[DEBUG]   my_apic_id:0xc cpu_index: 0x4
[DEBUG]   my_apic_id:0x18 cpu_index: 0x6

We can see same cpu_index for multiple cores before fix.

After this patch..
[DEBUG]   my_apic_id:0x10 cpu_index: 0x8
[DEBUG]   my_apic_id:019 cpu_index: 0xb
[DEBUG]   my_apic_id:0x11 cpu_index: 0x9
[DEBUG]   my_apic_id:0x18 cpu_index: 0xa
[DEBUG]   my_apic_id:0x40 cpu_index: 0x14
[DEBUG]   my_apic_id:0x30 cpu_index: 0x10
[DEBUG]   my_apic_id:0x42 cpu_index: 0x15
[DEBUG]   my_apic_id:0xc cpu_index: 0x6
[DEBUG]   my_apic_id:0x2 cpu_index: 0x1
[DEBUG]   my_apic_id:0x29 cpu_index: 0xf
[DEBUG]   my_apic_id:0xe cpu_index: 0x7
[DEBUG]   my_apic_id:0x20 cpu_index: 0xc
[DEBUG]   my_apic_id:0x0 cpu_index: 0x0
[DEBUG]   my_apic_id:0x31 cpu_index: 0x11
[DEBUG]   my_apic_id:0x28 cpu_index: 0xe
[DEBUG]   my_apic_id:0x21 cpu_index: 0xd
[DEBUG]   my_apic_id:0xa cpu_index: 0x5
[DEBUG]   my_apic_id:0x38 cpu_index: 0x12
[DEBUG]   my_apic_id:0x8 cpu_index:  0x4
[DEBUG]   my_apic_id:0x4 cpu_index: 0x2
[DEBUG]   my_apic_id:0x39 cpu_index: 0x13

Change-Id: I69e5e6231dd18b43d439340aaed50eb9edeca3b7
Signed-off-by: Sridhar Siricilla 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/70751
Reviewed-by: Arthur Heymans 
Reviewed-by: Kapil Porwal 
Reviewed-by: Sukumar Ghorai 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/common/block/acpi/cpu_hybrid.c
1 file changed, 96 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved
  Sukumar Ghorai: Looks good to me, but someone else must approve
  Kapil Porwal: Looks good to me, approved

diff --git a/src/soc/intel/common/block/acpi/cpu_hybrid.c b/src/soc/intel/common/block/acpi/cpu_hybrid.c
index e5ccf46..8f0da02 100644
--- a/src/soc/intel/common/block/acpi/cpu_hybrid.c
+++ b/src/soc/intel/common/block/acpi/cpu_hybrid.c
@@ -24,16 +24,22 @@
    return get_soc_cpu_type() == CPUID_CORE_TYPE_INTEL_CORE;
 }

+static struct device *get_cpu_bus_first_child(void)
+{
+	struct device *dev = dev_find_path(NULL, DEVICE_PATH_CPU_CLUSTER);
+	assert(dev != NULL);
+	return (dev->link_list)->children;
+}
+
 static u32 get_cpu_index(void)
 {
    u32 cpu_index = 0;
    struct device *dev;
    u32 my_apic_id = lapicid();

-	for (dev = dev_find_lapic(0); dev; dev = dev->next) {
+	for (dev = get_cpu_bus_first_child(); dev; dev = dev->sibling)
    	if (my_apic_id > dev->path.apic.apic_id)
    		cpu_index++;
-	}

return cpu_index;
 }