Re: [tip:x86/apic] x86/acpi: Introduce persistent storage for cpuid <-> apicid mapping

From: Thomas Gleixner
Date: Wed Oct 05 2016 - 10:06:41 EST


On Mon, 3 Oct 2016, Yinghai Lu wrote:
> init_cpu_node become:
> [ 55.477160] init_cpu_to_node:
> [ 55.483280] cpu 0 -> apicid 0x0 -> node 0
> [ 55.491558] cpu 1 -> apicid 0xff -> node 1
> [ 55.500017] cpu 2 -> apicid 0x2 -> node 0
> [ 55.508296] cpu 3 -> apicid 0x4 -> node 0
> [ 55.516575] cpu 4 -> apicid 0x6 -> node 0
> ...
>
> looks like problem is
>
> acpi_parse_lapic==>acpi_register_lapic==>__generic_processor_info==>allocate_logical_cpuid
>
> it will take lapic_id[0xff] take cpu index 1.
>
> Then will have not /dev/cpu/1/msr, that will make the MLC not happy.

That's pretty irrelevant whether that MLC thing - whatever it is - is
unhappy or not. If it cannot deal with something missing, it's surely not
the kernels problem. Unplug the cpu and the file will be missing as well.

> -static int acpi_register_lapic(int id, u32 acpiid, u8 enabled)
> +static int acpi_register_lapic(int id, u32 acpiid, u8 enabled, int disabled_id)
> {
> unsigned int ver = 0;
> int cpu;

This is whitespace damaged. The patch does not apply. Can you finaly after
doing 10 years of kernel development start being more careful?

> @@ -176,6 +177,11 @@ static int acpi_register_lapic(int id, u
> return -EINVAL;
> }
>
> + if (!enabled && (id == disabled_id)) {
> + ++disabled_cpus;
> + return -EINVAL;
> + }

Why would you need that disabled_id thing at all? The proper fix is to let
the apic driver detect the issue and this boils down to a 5 lines
change. Does the patch below fix the issue for you?

Thanks,

tglx

8<----------------
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2076,6 +2076,11 @@ int __generic_processor_info(int apicid,
bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
phys_cpu_present_map);

+ if (!apic->apic_id_valid(apicid)) {
+ disabled_cpus++;
+ return -EINVAL;
+ }
+
/*
* boot_cpu_physical_apicid is designed to have the apicid
* returned by read_apic_id(), i.e, the apicid of the