Re: [PATCHv4 1/3] powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt
On Tue, Mar 13, 2018 at 10:58 AM, Benjamin Herrenschmidt wrote: > On Mon, 2018-03-12 at 12:43 +0800, Pingfan Liu wrote: >> For kexec -p, the boot cpu can be not the cpu0, this causes the problem >> to alloc paca[]. In theory, there is no requirement to assign cpu's logical >> id as its present seq by device tree. But we have something like >> cpu_first_thread_sibling(), which makes assumption on the mapping inside >> a core. Hence partially changing the mapping, i.e. unbind the mapping of >> core while keep the mapping inside a core. After this patch, boot-cpu >> will always be mapped into the range [0,threads_per_core). > > I'm ok with the idea but not fan of the implementation: > >> Signed-off-by: Pingfan Liu >> --- >> arch/powerpc/include/asm/smp.h | 1 + >> arch/powerpc/kernel/prom.c | 25 ++--- >> arch/powerpc/kernel/setup-common.c | 21 + >> 3 files changed, 36 insertions(+), 11 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h >> index fac963e..1299100 100644 >> --- a/arch/powerpc/include/asm/smp.h >> +++ b/arch/powerpc/include/asm/smp.h >> @@ -30,6 +30,7 @@ >> #include >> >> extern int boot_cpuid; >> +extern int boot_cpuhwid; >> extern int spinning_secondaries; >> >> extern void cpu_die(void); >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c >> index da67606..d0ebb25 100644 >> --- a/arch/powerpc/kernel/prom.c >> +++ b/arch/powerpc/kernel/prom.c >> @@ -315,8 +315,7 @@ static int __init early_init_dt_scan_cpus(unsigned long >> node, >> const __be32 *intserv; >> int i, nthreads; >> int len; >> - int found = -1; >> - int found_thread = 0; >> + bool found = false; >> >> /* We are scanning "cpu" nodes only */ >> if (type == NULL || strcmp(type, "cpu") != 0) >> @@ -341,8 +340,11 @@ static int __init early_init_dt_scan_cpus(unsigned long >> node, >> if (fdt_version(initial_boot_params) >= 2) { >> if (be32_to_cpu(intserv[i]) == >> fdt_boot_cpuid_phys(initial_boot_params)) { >> - found = boot_cpu_count; >> - found_thread = i; >> + /* always map the boot-cpu logical id into the >> + * the range of [0, thread_per_core) >> + */ >> + boot_cpuid = i; >> + found = true; >> } > > Call it boot_thread_id > But I think boot_cpuid has the meaning of global index, while the thread_id has the meaning of index in a core. >> } else { >> /* >> @@ -351,8 +353,10 @@ static int __init early_init_dt_scan_cpus(unsigned long >> node, >>* off secondary threads. >>*/ >> if (of_get_flat_dt_prop(node, >> - "linux,boot-cpu", NULL) != NULL) >> - found = boot_cpu_count; >> + "linux,boot-cpu", NULL) != NULL) { >> + boot_cpuid = i; >> + found = true; >> + } >> } >> #ifdef CONFIG_SMP >> /* logical cpu id is always 0 on UP kernels */ >> @@ -361,13 +365,12 @@ static int __init early_init_dt_scan_cpus(unsigned >> long node, >> } >> >> /* Not the boot CPU */ >> - if (found < 0) >> + if (!found) >> return 0; >> >> - DBG("boot cpu: logical %d physical %d\n", found, >> - be32_to_cpu(intserv[found_thread])); >> - boot_cpuid = found; >> - set_hard_smp_processor_id(found, be32_to_cpu(intserv[found_thread])); >> + boot_cpuhwid = be32_to_cpu(intserv[boot_cpuid]); >> + DBG("boot cpu: logical %d physical %d\n", boot_cpuid, boot_cpuhwid); >> + set_hard_smp_processor_id(boot_cpuid, boot_cpuhwid); >> >> /* >>* PAPR defines "logical" PVR values for cpus that >> diff --git a/arch/powerpc/kernel/setup-common.c >> b/arch/powerpc/kernel/setup-common.c >> index 66f7cc6..1a67344 100644 >> --- a/arch/powerpc/kernel/setup-common.c >> +++ b/arch/powerpc/kernel/setup-common.c >> @@ -86,6 +86,7 @@ struct machdep_calls *machine_id; >> EXPORT_SYMBOL(machine_id); >> >> int boot_cpuid = -1; >> +int boot_cpuhwid = -1; >> EXPORT_SYMBOL_GPL(boot_cpuid); >> >> /* >> @@ -459,11 +460,17 @@ static void __init cpu_init_thread_core_maps(int tpc) >> void __init smp_setup_cpu_maps(void) >> { >> struct device_node *dn; >> + struct device_node *boot_dn = NULL; >> + bool handling_bootdn = true; >> int cpu = 0; >> int nthreads = 1; >> >> DBG("smp_setup_cpu_maps()\n"); >> >> +again: >> + /* E.g. kexec will not boot from the 1st core. So firstly loop to find >> out >> + * the dn of boot-cpu, and map them on
Re: [PATCHv4 1/3] powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt
Hi Pingfan, Thank you for the patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [also build test ERROR on next-20180309] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Pingfan-Liu/powerpc-cpu-partially-unbind-the-mapping-between-cpu-logical-id-and-its-seq-in-dt/20180313-034420 config: powerpc-pq2fads_defconfig (attached as .config) compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=powerpc All errors (new ones prefixed by >>): >> arch/powerpc/kernel/prom.c:79:23: error: 'boot_cpu_count' defined but not >> used [-Werror=unused-variable] static int __initdata boot_cpu_count; ^~ cc1: all warnings being treated as errors vim +/boot_cpu_count +79 arch/powerpc/kernel/prom.c 9b6b563c Paul Mackerras 2005-10-06 71 9b6b563c Paul Mackerras 2005-10-06 72 #ifdef CONFIG_PPC64 28897731 Olof Johansson 2006-04-12 73 int __initdata iommu_is_off; 9b6b563c Paul Mackerras 2005-10-06 74 int __initdata iommu_force_on; cf00a8d1 Paul Mackerras 2005-10-31 75 unsigned long tce_alloc_start, tce_alloc_end; cd3db0c4 Benjamin Herrenschmidt 2010-07-06 76 u64 ppc64_rma_size; 9b6b563c Paul Mackerras 2005-10-06 77 #endif 03bf469a Benjamin Herrenschmidt 2011-05-11 78 static phys_addr_t first_memblock_size; 7ac87abb Matt Evans 2011-05-25 @79 static int __initdata boot_cpu_count; 9b6b563c Paul Mackerras 2005-10-06 80 :: The code at line 79 was first introduced by commit :: 7ac87abb8166b99584149fcfb2efef5773a078e9 powerpc: Fix early boot accounting of CPUs :: TO: Matt Evans :: CC: Benjamin Herrenschmidt --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv4 1/3] powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt
On Mon, 2018-03-12 at 12:43 +0800, Pingfan Liu wrote: > For kexec -p, the boot cpu can be not the cpu0, this causes the problem > to alloc paca[]. In theory, there is no requirement to assign cpu's logical > id as its present seq by device tree. But we have something like > cpu_first_thread_sibling(), which makes assumption on the mapping inside > a core. Hence partially changing the mapping, i.e. unbind the mapping of > core while keep the mapping inside a core. After this patch, boot-cpu > will always be mapped into the range [0,threads_per_core). I'm ok with the idea but not fan of the implementation: > Signed-off-by: Pingfan Liu > --- > arch/powerpc/include/asm/smp.h | 1 + > arch/powerpc/kernel/prom.c | 25 ++--- > arch/powerpc/kernel/setup-common.c | 21 + > 3 files changed, 36 insertions(+), 11 deletions(-) > > diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h > index fac963e..1299100 100644 > --- a/arch/powerpc/include/asm/smp.h > +++ b/arch/powerpc/include/asm/smp.h > @@ -30,6 +30,7 @@ > #include > > extern int boot_cpuid; > +extern int boot_cpuhwid; > extern int spinning_secondaries; > > extern void cpu_die(void); > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index da67606..d0ebb25 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -315,8 +315,7 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, > const __be32 *intserv; > int i, nthreads; > int len; > - int found = -1; > - int found_thread = 0; > + bool found = false; > > /* We are scanning "cpu" nodes only */ > if (type == NULL || strcmp(type, "cpu") != 0) > @@ -341,8 +340,11 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, > if (fdt_version(initial_boot_params) >= 2) { > if (be32_to_cpu(intserv[i]) == > fdt_boot_cpuid_phys(initial_boot_params)) { > - found = boot_cpu_count; > - found_thread = i; > + /* always map the boot-cpu logical id into the > + * the range of [0, thread_per_core) > + */ > + boot_cpuid = i; > + found = true; > } Call it boot_thread_id > } else { > /* > @@ -351,8 +353,10 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, >* off secondary threads. >*/ > if (of_get_flat_dt_prop(node, > - "linux,boot-cpu", NULL) != NULL) > - found = boot_cpu_count; > + "linux,boot-cpu", NULL) != NULL) { > + boot_cpuid = i; > + found = true; > + } > } > #ifdef CONFIG_SMP > /* logical cpu id is always 0 on UP kernels */ > @@ -361,13 +365,12 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, > } > > /* Not the boot CPU */ > - if (found < 0) > + if (!found) > return 0; > > - DBG("boot cpu: logical %d physical %d\n", found, > - be32_to_cpu(intserv[found_thread])); > - boot_cpuid = found; > - set_hard_smp_processor_id(found, be32_to_cpu(intserv[found_thread])); > + boot_cpuhwid = be32_to_cpu(intserv[boot_cpuid]); > + DBG("boot cpu: logical %d physical %d\n", boot_cpuid, boot_cpuhwid); > + set_hard_smp_processor_id(boot_cpuid, boot_cpuhwid); > > /* >* PAPR defines "logical" PVR values for cpus that > diff --git a/arch/powerpc/kernel/setup-common.c > b/arch/powerpc/kernel/setup-common.c > index 66f7cc6..1a67344 100644 > --- a/arch/powerpc/kernel/setup-common.c > +++ b/arch/powerpc/kernel/setup-common.c > @@ -86,6 +86,7 @@ struct machdep_calls *machine_id; > EXPORT_SYMBOL(machine_id); > > int boot_cpuid = -1; > +int boot_cpuhwid = -1; > EXPORT_SYMBOL_GPL(boot_cpuid); > > /* > @@ -459,11 +460,17 @@ static void __init cpu_init_thread_core_maps(int tpc) > void __init smp_setup_cpu_maps(void) > { > struct device_node *dn; > + struct device_node *boot_dn = NULL; > + bool handling_bootdn = true; > int cpu = 0; > int nthreads = 1; > > DBG("smp_setup_cpu_maps()\n"); > > +again: > + /* E.g. kexec will not boot from the 1st core. So firstly loop to find > out > + * the dn of boot-cpu, and map them onto [0, nthreads) > + */ > for_each_node_by_type(dn, "cpu") { > const __be32 *intserv; > __be32 cpu_be; > @@ -488,6 +495,16 @@ void __init smp_setup_cpu_maps(void) > > nthreads = len / sizeof(int); > > + if (handling
[PATCHv4 1/3] powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt
For kexec -p, the boot cpu can be not the cpu0, this causes the problem to alloc paca[]. In theory, there is no requirement to assign cpu's logical id as its present seq by device tree. But we have something like cpu_first_thread_sibling(), which makes assumption on the mapping inside a core. Hence partially changing the mapping, i.e. unbind the mapping of core while keep the mapping inside a core. After this patch, boot-cpu will always be mapped into the range [0,threads_per_core). Signed-off-by: Pingfan Liu --- arch/powerpc/include/asm/smp.h | 1 + arch/powerpc/kernel/prom.c | 25 ++--- arch/powerpc/kernel/setup-common.c | 21 + 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index fac963e..1299100 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -30,6 +30,7 @@ #include extern int boot_cpuid; +extern int boot_cpuhwid; extern int spinning_secondaries; extern void cpu_die(void); diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index da67606..d0ebb25 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -315,8 +315,7 @@ static int __init early_init_dt_scan_cpus(unsigned long node, const __be32 *intserv; int i, nthreads; int len; - int found = -1; - int found_thread = 0; + bool found = false; /* We are scanning "cpu" nodes only */ if (type == NULL || strcmp(type, "cpu") != 0) @@ -341,8 +340,11 @@ static int __init early_init_dt_scan_cpus(unsigned long node, if (fdt_version(initial_boot_params) >= 2) { if (be32_to_cpu(intserv[i]) == fdt_boot_cpuid_phys(initial_boot_params)) { - found = boot_cpu_count; - found_thread = i; + /* always map the boot-cpu logical id into the +* the range of [0, thread_per_core) +*/ + boot_cpuid = i; + found = true; } } else { /* @@ -351,8 +353,10 @@ static int __init early_init_dt_scan_cpus(unsigned long node, * off secondary threads. */ if (of_get_flat_dt_prop(node, - "linux,boot-cpu", NULL) != NULL) - found = boot_cpu_count; + "linux,boot-cpu", NULL) != NULL) { + boot_cpuid = i; + found = true; + } } #ifdef CONFIG_SMP /* logical cpu id is always 0 on UP kernels */ @@ -361,13 +365,12 @@ static int __init early_init_dt_scan_cpus(unsigned long node, } /* Not the boot CPU */ - if (found < 0) + if (!found) return 0; - DBG("boot cpu: logical %d physical %d\n", found, - be32_to_cpu(intserv[found_thread])); - boot_cpuid = found; - set_hard_smp_processor_id(found, be32_to_cpu(intserv[found_thread])); + boot_cpuhwid = be32_to_cpu(intserv[boot_cpuid]); + DBG("boot cpu: logical %d physical %d\n", boot_cpuid, boot_cpuhwid); + set_hard_smp_processor_id(boot_cpuid, boot_cpuhwid); /* * PAPR defines "logical" PVR values for cpus that diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 66f7cc6..1a67344 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -86,6 +86,7 @@ struct machdep_calls *machine_id; EXPORT_SYMBOL(machine_id); int boot_cpuid = -1; +int boot_cpuhwid = -1; EXPORT_SYMBOL_GPL(boot_cpuid); /* @@ -459,11 +460,17 @@ static void __init cpu_init_thread_core_maps(int tpc) void __init smp_setup_cpu_maps(void) { struct device_node *dn; + struct device_node *boot_dn = NULL; + bool handling_bootdn = true; int cpu = 0; int nthreads = 1; DBG("smp_setup_cpu_maps()\n"); +again: + /* E.g. kexec will not boot from the 1st core. So firstly loop to find out +* the dn of boot-cpu, and map them onto [0, nthreads) +*/ for_each_node_by_type(dn, "cpu") { const __be32 *intserv; __be32 cpu_be; @@ -488,6 +495,16 @@ void __init smp_setup_cpu_maps(void) nthreads = len / sizeof(int); + if (handling_bootdn) { + if (boot_cpuid < nthreads && + be32_to_cpu(intserv[boot_cpuid]) == boot_cpuhwid) { + boot_dn = dn; + } + if (boot_dn == NULL) +