Detect all possible combinations of mismatch right in the CPUID evaluation
code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/common.c          |   15 ++-------------
 arch/x86/kernel/cpu/topology_common.c |   10 ++++++++++
 2 files changed, 12 insertions(+), 13 deletions(-)

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1676,22 +1676,11 @@ static void generic_identify(struct cpui
 #endif
 }
 
-/*
- * Validate that ACPI/mptables have the same information about the
- * effective APIC id and update the package map.
- */
-static void validate_apic_and_package_id(struct cpuinfo_x86 *c)
+static void update_package_map(struct cpuinfo_x86 *c)
 {
 #ifdef CONFIG_SMP
 	unsigned int cpu = smp_processor_id();
-	u32 apicid;
 
-	apicid = apic->cpu_present_to_apicid(cpu);
-
-	if (apicid != c->topo.apicid) {
-		pr_err(FW_BUG "CPU%u: APIC id mismatch. Firmware: %x APIC: %x\n",
-		       cpu, apicid, c->topo.initial_apicid);
-	}
 	BUG_ON(topology_update_package_map(c->topo.pkg_id, cpu));
 	BUG_ON(topology_update_die_map(c->topo.die_id, cpu));
 #else
@@ -1876,7 +1865,7 @@ void identify_secondary_cpu(struct cpuin
 #ifdef CONFIG_X86_32
 	enable_sep_cpu();
 #endif
-	validate_apic_and_package_id(c);
+	update_package_map(c);
 	x86_spec_ctrl_setup_ap();
 	update_srbds_msr();
 
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -176,6 +176,16 @@ void cpu_parse_topology(struct cpuinfo_x
 
 	parse_topology(&tscan, false);
 
+	if (c->topo.initial_apicid != c->topo.apicid) {
+		pr_err(FW_BUG "CPU%4u: APIC ID mismatch. CPUID: 0x%04x APIC: 0x%04x\n",
+		       cpu, c->topo.initial_apicid, c->topo.apicid);
+	}
+
+	if (c->topo.apicid != cpuid_to_apicid[cpu]) {
+		pr_err(FW_BUG "CPU%4u: APIC ID mismatch. Firmware: 0x%04x APIC: 0x%04x\n",
+		       cpu, cpuid_to_apicid[cpu], c->topo.apicid);
+	}
+
 	for (dom = TOPO_SMT_DOMAIN; dom < TOPO_MAX_DOMAIN; dom++) {
 		if (tscan.dom_shifts[dom] == x86_topo_system.dom_shifts[dom])
 			continue;

On 8/7/2023 6:52 AM, T
>   
> --- a/arch/x86/kernel/cpu/topology_common.c
> +++ b/arch/x86/kernel/cpu/topology_common.c
> @@ -176,6 +176,16 @@ void cpu_parse_topology(struct cpuinfo_x
>   
>   	parse_topology(&tscan, false);
>   
> +	if (c->topo.initial_apicid != c->topo.apicid) {
> +		pr_err(FW_BUG "CPU%4u: APIC ID mismatch. CPUID: 0x%04x APIC: 0x%04x\n",
> +		       cpu, c->topo.initial_apicid, c->topo.apicid);
> +	}
> +
> +	if (c->topo.apicid != cpuid_to_apicid[cpu]) {
> +		pr_err(FW_BUG "CPU%4u: APIC ID mismatch. Firmware: 0x%04x APIC: 0x%04x\n",
> +		       cpu, cpuid_to_apicid[cpu], c->topo.apicid);
> +	}
> +

while these messages are basically the same as current ones they are short one key thing for the user
... which one of the two will be used. Yes one can look up in the source code where the message comes from
and reverse engineer that... or we can just add this to these pr_err() messages


like

pr_err(FW_BUG "CPU%4u: APIC ID mismatch. CPUID: 0x%04x APIC: 0x%04x. APIC value will be used.\n",
		       cpu, c->topo.initial_apicid, c->topo.apicid);


>   	for (dom = TOPO_SMT_DOMAIN; dom < TOPO_MAX_DOMAIN; dom++) {
>   		if (tscan.dom_shifts[dom] == x86_topo_system.dom_shifts[dom])
>   			continue;
>

On Mon, Aug 07 2023 at 07:28, Arjan van de Ven wrote:

> On 8/7/2023 6:52 AM, T
>>   
>> --- a/arch/x86/kernel/cpu/topology_common.c
>> +++ b/arch/x86/kernel/cpu/topology_common.c
>> @@ -176,6 +176,16 @@ void cpu_parse_topology(struct cpuinfo_x
>>   
>>   	parse_topology(&tscan, false);
>>   
>> +	if (c->topo.initial_apicid != c->topo.apicid) {
>> +		pr_err(FW_BUG "CPU%4u: APIC ID mismatch. CPUID: 0x%04x APIC: 0x%04x\n",
>> +		       cpu, c->topo.initial_apicid, c->topo.apicid);
>> +	}
>> +
>> +	if (c->topo.apicid != cpuid_to_apicid[cpu]) {
>> +		pr_err(FW_BUG "CPU%4u: APIC ID mismatch. Firmware: 0x%04x APIC: 0x%04x\n",
>> +		       cpu, cpuid_to_apicid[cpu], c->topo.apicid);
>> +	}
>> +
>
> while these messages are basically the same as current ones they are short one key thing for the user
> ... which one of the two will be used. Yes one can look up in the source code where the message comes from
> and reverse engineer that... or we can just add this to these pr_err() messages
>
>
> like
>
> pr_err(FW_BUG "CPU%4u: APIC ID mismatch. CPUID: 0x%04x APIC: 0x%04x. APIC value will be used.\n",
> 		       cpu, c->topo.initial_apicid, c->topo.apicid);

Good point.