Re: [PATCHv9 2/2] powerpc/setup: Loosen the mapping between cpu logical id and its seq in dt
From: Hari Bathini <hbathini@linux.ibm.com>
Date: 2023-11-27 04:31:32
Also in:
kexec
Subsystem:
linux for powerpc (32-bit and 64-bit), the rest · Maintainers:
Madhavan Srinivasan, Michael Ellerman, Linus Torvalds
Hi Pingfan, Michael, On 17/10/23 4:03 pm, Hari Bathini wrote:
On 17/10/23 7:58 am, Pingfan Liu wrote:quoted
*** Idea *** For kexec -p, the boot cpu can be not the cpu0, this causes the problem of allocating memory for paca_ptrs[]. However, in theory, there is no requirement to assign cpu's logical id as its present sequence in the device tree. But there is something like cpu_first_thread_sibling(), which makes assumption on the mapping inside a core. Hence partially loosening the mapping, i.e. unbind the mapping of core while keep the mapping inside a core. *** Implement *** At this early stage, there are plenty of memory to utilize. Hence, this patch allocates interim memory to link the cpu info on a list, then reorder cpus by changing the list head. As a result, there is a rotate shift between the sequence number in dt and the cpu logical number. *** Result *** After this patch, a boot-cpu's logical id will always be mapped into the range [0,threads_per_core). Besides this, at this phase, all threads in the boot core are forced to be onlined. This restriction will be lifted in a later patch with extra effort. Signed-off-by: Pingfan Liu <redacted> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Nicholas Piggin <npiggin@gmail.com> Cc: Christophe Leroy <redacted> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com> Cc: Wen Xiong <redacted> Cc: Baoquan He <bhe@redhat.com> Cc: Ming Lei <redacted> Cc: Sourabh Jain <redacted> Cc: Hari Bathini <hbathini@linux.ibm.com> Cc: kexec@lists.infradead.org To: linuxppc-dev@lists.ozlabs.orgThanks for working on this, Pingfan. Looks good to me. Acked-by: Hari Bathini <hbathini@linux.ibm.com>
On second thoughts, probably better off with no impact for bootcpu < nr_cpu_ids case and changing only two cores logical numbering otherwise. Something like the below (Please share your thoughts):
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index ec82f5bda908..78a8312aa8c4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c@@ -76,7 +76,9 @@ u64 ppc64_rma_size; unsigned int boot_cpu_node_count __ro_after_init; #endif static phys_addr_t first_memblock_size; +#ifdef CONFIG_SMP static int __initdata boot_cpu_count; +#endif static int __init early_parse_mem(char *p) {
@@ -357,6 +359,25 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
fdt_boot_cpuid_phys(initial_boot_params)) {
found = boot_cpu_count;
found_thread = i;
+ /*
+ * Map boot-cpu logical id into the range
+ * of [0, thread_per_core) if it can't be
+ * accommodated within nr_cpu_ids.
+ */
+ if (i != boot_cpu_count && boot_cpu_count >= nr_cpu_ids) {
+ boot_cpuid = i;
+ DBG("Logical CPU number for boot CPU changed from %d to %d\n",
+ boot_cpu_count, i);
+ } else {
+ boot_cpuid = boot_cpu_count;
+ }
+
+ /* Ensure boot thread is acconted for in nr_cpu_ids */
+ if (boot_cpuid >= nr_cpu_ids) {
+ set_nr_cpu_ids(boot_cpuid + 1);
+ DBG("Adjusted nr_cpu_ids to %u, to include boot CPU.\n",
+ nr_cpu_ids);
+ }
}
#ifdef CONFIG_SMP
/* logical cpu id is always 0 on UP kernels */@@ -368,9 +389,8 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
if (found < 0)
return 0;
- DBG("boot cpu: logical %d physical %d\n", found,
+ DBG("boot cpu: logical %d physical %d\n", boot_cpuid,
be32_to_cpu(intserv[found_thread]));
- boot_cpuid = found;
boot_cpu_hwid = be32_to_cpu(intserv[found_thread]);
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index b7b733474b60..f7179525c774 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c@@ -409,6 +409,12 @@ static void __init cpu_init_thread_core_maps(int tpc) u32 *cpu_to_phys_id = NULL; +struct interrupt_server_node { + bool avail; + int len; + __be32 intserv[]; +}; + /** * setup_cpu_maps - initialize the following cpu maps: * cpu_possible_mask
@@ -429,9 +435,13 @@ u32 *cpu_to_phys_id = NULL; */ void __init smp_setup_cpu_maps(void) { + struct interrupt_server_node *core0_node = NULL, *bt_node = NULL; + int orig_boot_cpu = -1, orig_boot_thread = -1; + bool found_boot_cpu = false; struct device_node *dn; - int cpu = 0; int nthreads = 1; + int cpu = 0; + int j, len; DBG("smp_setup_cpu_maps()\n");
@@ -442,9 +452,9 @@ void __init smp_setup_cpu_maps(void) __func__, nr_cpu_ids * sizeof(u32), __alignof__(u32)); for_each_node_by_type(dn, "cpu") { + bool avail, skip = false; const __be32 *intserv; __be32 cpu_be; - int j, len; DBG(" * %pOF...\n", dn);
@@ -466,29 +476,121 @@ void __init smp_setup_cpu_maps(void) nthreads = len / sizeof(int); - for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) { - bool avail; + avail = of_device_is_available(dn); + if (!avail) + avail = !of_property_match_string(dn, + "enable-method", "spin-table"); + + for (j = 0; (cpu == 0 || !found_boot_cpu) && j < nthreads; j++) { + if (be32_to_cpu(intserv[j]) == boot_cpu_hwid) { + found_boot_cpu = true; + if (cpu == 0) + break; + + /* Original logical CPU number of thread0 in boot core */ + orig_boot_cpu = cpu; + orig_boot_thread = j; + bt_node = memblock_alloc(sizeof(struct interrupt_server_node) + len, + __alignof__(u32)); + if (!bt_node) + panic("%s: Failed to allocate %zu bytes align=0x%zx\n", + __func__, + sizeof(struct interrupt_server_node) + len, + __alignof__(u32)); + bt_node->len = len; + memcpy(bt_node->intserv, intserv, len); + bt_node->avail = avail; + skip = true; + break; + } + } + /* + * Boot CPU not on core0. + * Hold off adding core0 until boot core is found as core0 + * may have to be replaced with boot core if boot core can + * not be accommodated within nr_cpu_ids with its original + * logical CPU numbering. + */ + if (cpu == 0 && !found_boot_cpu) { + core0_node = memblock_alloc(sizeof(struct interrupt_server_node) + len, + __alignof__(u32)); + if (!core0_node) + panic("%s: Failed to allocate %zu bytes align=0x%zx\n", + __func__, + sizeof(struct interrupt_server_node) + len, + __alignof__(u32)); + core0_node->len = len; + memcpy(core0_node->intserv, intserv, len); + core0_node->avail = avail; + skip = true; + } + + if (skip) { + /* Assumes same number of threads for all cores */ + cpu += nthreads; + continue; + } + + for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) { DBG(" thread %d -> cpu %d (hard id %d)\n", j, cpu, be32_to_cpu(intserv[j])); - avail = of_device_is_available(dn); - if (!avail) - avail = !of_property_match_string(dn, - "enable-method", "spin-table"); - set_cpu_present(cpu, avail); set_cpu_possible(cpu, true); cpu_to_phys_id[cpu] = be32_to_cpu(intserv[j]); cpu++; } - if (cpu >= nr_cpu_ids) { + if (found_boot_cpu && cpu >= nr_cpu_ids) { of_node_put(dn); break; } } + /* + * Boot CPU not on core0. + * + * If nr_cpu_ids does not accommodate the original logical CPU
numbering for
+ * boot CPU core, use logical CPU numbers 0 to nthreads for boot CPU core.
+ * Note that boot cpu is already assigned with logical CPU number
somewhere
+ * between 0 to nthreads (depending on the boot thread within the core) in
+ * early_init_dt_scan_cpus() for this case.
+ *
+ * Otherwise, stick with the original logical CPU numbering.
+ */
+ if (bt_node) {
+ int core0_cpu;
+
+ if (orig_boot_cpu + orig_boot_thread >= nr_cpu_ids) {
+ cpu = 0;
+ core0_cpu = orig_boot_cpu;
+ } else {
+ cpu = orig_boot_cpu;
+ core0_cpu = 0;
+ }
+
+ for (j = 0; j < nthreads && core0_cpu < nr_cpu_ids; j++) {
+ DBG(" thread %d -> cpu %d (hard id %d)\n",
+ j, core0_cpu, be32_to_cpu(core0_node->intserv[j]));
+
+ set_cpu_present(core0_cpu, core0_node->avail);
+ set_cpu_possible(core0_cpu, true);
+ cpu_to_phys_id[core0_cpu] = be32_to_cpu(core0_node->intserv[j]);
+ core0_cpu++;
+ }
+
+ for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
+ DBG(" thread %d -> cpu %d (hard id %d)\n",
+ j, cpu, be32_to_cpu(bt_node->intserv[j]));
+
+ set_cpu_present(cpu, bt_node->avail);
+ set_cpu_possible(cpu, true);
+ cpu_to_phys_id[cpu] = be32_to_cpu(bt_node->intserv[j]);
+ cpu++;
+ }
+ }
+
/* If no SMT supported, nthreads is forced to 1 */
if (!cpu_has_feature(CPU_FTR_SMT)) {
DBG(" SMT disabled ! nthreads forced to 1\n");