linux-kernel - [PATCH] arch/x86: Remove second call to topology_update_package

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Oct 2016 10:36:28 -0400
From:   Prarit Bhargava <prarit@...hat.com>
To:     linux-kernel@...r.kernel.org
Cc:     Prarit Bhargava <prarit@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Len Brown <len.brown@...el.com>, Borislav Petkov <bp@...e.de>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Jiri Olsa <jolsa@...hat.com>
Subject: [PATCH] arch/x86: Remove second call to topology_update_package_map()

This was noticed during the investigation of

http://git.kernel.org/tip/2a51fe083eba7f99cbda72f5ef90cdf2f4df882c

A note for reviewers on the cleanup in smp_init_package_map(): The
per_cpu variable x86_bios_cpu_apicid is initialized to BAD_APICID, and the
bitmaps are set to false by default so the code isn't necessary.  The
pr_warn() can also be dropped as generic_processor_info() will do a
pr_warning() in the same situation.

P.

-----8<-----

After commit 1f12e32f4cd5 ("x86/topology: Create logical package id"),
topology_update_package_map() is called twice on a CPU.  The first call is in
generic_processor_info(), which is called on all present cpus (found in either
mptable or ACPI MADT).  The second call is in smp_init_package_map() which
is called later on in the init sequence.

Only a single call is necessary for the kernel to initialize the data
structures properly. This patch drops the later call in smp_init_package_map()
and moves smp_init_package_map() out of smp_store_boot_cpu_info().

Signed-off-by: Prarit Bhargava <prarit@...hat.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: "H. Peter Anvin" <hpa@...or.com>
Cc: x86@...nel.org
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Len Brown <len.brown@...el.com>
Cc: Borislav Petkov <bp@...e.de>
Cc: Tim Chen <tim.c.chen@...ux.intel.com>
Cc: Andi Kleen <ak@...ux.intel.com>
Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc: Jiri Olsa <jolsa@...hat.com>
---
 arch/x86/kernel/smpboot.c |   20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 951f093a96fe..0ff74f8bb5e9 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -271,7 +271,7 @@ int topology_update_package_map(unsigned int apicid, unsigned int cpu)
 	if (!physical_package_map)
 		return 0;
 
-	if (pkg >= max_physical_pkg_id)
+	if (!apic->apic_id_valid(apicid) || pkg >= max_physical_pkg_id)
 		return -EINVAL;
 
 	/* Set the logical package id */
@@ -310,7 +310,7 @@ int topology_phys_to_logical_pkg(unsigned int phys_pkg)
 
 static void __init smp_init_package_map(void)
 {
-	unsigned int ncpus, cpu;
+	unsigned int ncpus;
 	size_t size;
 
 	/*
@@ -342,7 +342,6 @@ static void __init smp_init_package_map(void)
 	}
 
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
-	logical_packages = 0;
 
 	/*
 	 * Possibly larger than what we need as the number of apic ids per
@@ -355,19 +354,6 @@ static void __init smp_init_package_map(void)
 	size = BITS_TO_LONGS(max_physical_pkg_id) * sizeof(unsigned long);
 	physical_package_map = kzalloc(size, GFP_KERNEL);
 
-	for_each_present_cpu(cpu) {
-		unsigned int apicid = apic->cpu_present_to_apicid(cpu);
-
-		if (apicid == BAD_APICID || !apic->apic_id_valid(apicid))
-			continue;
-		if (!topology_update_package_map(apicid, cpu))
-			continue;
-		pr_warn("CPU %u APICId %x disabled\n", cpu, apicid);
-		per_cpu(x86_bios_cpu_apicid, cpu) = BAD_APICID;
-		set_cpu_possible(cpu, false);
-		set_cpu_present(cpu, false);
-	}
-
 	if (logical_packages > __max_logical_packages) {
 		pr_warn("Detected more packages (%u), then computed by BIOS data (%u).\n",
 			logical_packages, __max_logical_packages);
@@ -385,7 +371,6 @@ void __init smp_store_boot_cpu_info(void)
 
 	*c = boot_cpu_data;
 	c->cpu_index = id;
-	smp_init_package_map();
 }
 
 /*
@@ -1285,6 +1270,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	 * Setup boot CPU information
 	 */
 	smp_store_boot_cpu_info(); /* Final full version of the data */
+	smp_init_package_map();
 	cpumask_copy(cpu_callin_mask, cpumask_of(0));
 	mb();
 
-- 
1.7.9.3