Re: [PATCH v4 1/3] x86, apic: Don't count the CPU with BP flag fromMP table as booting-up CPU

From: HATAYAMA Daisuke
Date: Tue Nov 12 2013 - 04:59:13 EST


(2013/11/12 9:40), HATAYAMA Daisuke wrote:
(2013/11/12 1:52), Vivek Goyal wrote:
On Mon, Nov 11, 2013 at 11:52:30AM +0900, HATAYAMA Daisuke wrote:

[..]
Looking at my past investigation, kernel/mpparse.c, mm/amdtopology.c and
platform/visws/visws_quirks.c assumes that boot_cpu_physical_apicid
has initial apicid of the BSP, not the current actual booting-up cpu.

These three are called in get_smp_config() below. If either of them is
called actually, boot_cpu_physical_apicid has the apicid different from
the current actual booting-up cpu temporarily. But init_apic_mappings()
soon modifies back the value to the one obtained by read_apic_id().

/*
* Read APIC and some other early information from ACPI tables.
*/
acpi_boot_init();
sfi_init();
x86_dtb_init();

/*
* get boot-time SMP configuration:
*/
if (smp_found_config)
get_smp_config();

prefill_possible_map();

init_cpu_to_node();

init_apic_mappings();

So, thanks to init_apic_mappings(), the patch set would work without the
first patch... This is a careless point in this patch set.


If init_apic_mappings(), is making sure that boot_cpu_physical_apicid is
apic id of booting processor, and you don't need first patch of your
series, then I think atleast re-post your patch series without first
patch.


Yes, I'll repost soon.

And then there can be another series which looks into whether we need
two different variables or not and if we do, then a separate variable
bsp_physical_apicid will track the bsp id as reported by BIOS and
boot_cpu_physical_apicid will track apic id of booting cpu. This might
a very big and slow cleanup. So I think blocking the first patch series
behind it might not make much sense.


Yes, the current handling of boot_cpu_physical_apicid looks strange and
should be cleaned up, but the cleaning up needs reviewing together with
the maintainers for the corresponding part; in particular, it can be
lengthy for the reviewing on amdtopology.c. I leave this as another
work for the time being.


Sorry for my confusion. It's necessary to introduce a new variable to keep
the initial APIC ID for the processor with BSP flag on IA32_APIC_BASE MSR,
which is needed in case of AP is specified by disable_cpu_apicid and using
MP table.

I've posted new v5 patch set a little ago. Could you please review it?

--
Thanks.
HATAYAMA, Daisuke

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/