Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On Fri, Dec 16, 2011 at 3:29 PM, Scott Wood scottw...@freescale.com wrote: On 12/15/2011 09:35 PM, Benjamin Herrenschmidt wrote: On Fri, 2011-12-16 at 03:29 +, McClintock Matthew-B29882 wrote: On Thu, Dec 15, 2011 at 9:12 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2011-11-28 at 22:24 -0600, Matthew McClintock wrote: boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. Where does the ordering matter currently? The kernel won't boot if the order of the cpu nodes in the device tree does not match the reg property. This can be fixed by using init_thread_info.cpu instead of a separate variable - which seems to be correct since we don't actually need a separate boot_cpuid variable at all for powerpc. The correct initialization occurs in this scenario in early_init_dt_scan_cpus(). (boot_cpuid maps to init_thread_info.cpu via a define and could be just renamed everywhere in arch/powerpc/ ) So what about head_fsl_booke.S comparing boot_cpuid to -1 ? That seems to be broken now in at least 2 ways, boot_cpuid doesn't exist anymore and you don't initialize it to -1 either... This is 4/5 which is also waiting for your review. http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-October/093474.html Ah missed that. This is FSL specific, I'd need Kumar and/or Scott's ack for that one. It would be nice if we could eliminate all usage of the boot cpu dtb field -- it's easy to forget to set it, especially if you're not making an AMP config. The default -1 means this patch would break booting with such a tree. I can add a check here to see if the boot cpu in the device tree is -1 to assume this is the boot cpu in addition to the boot cpu matching the PIR. This seems like the best approach to me, keep what I have in these two patches and add this additional check to keep all device tree's working properly. If we don't want to record the PIR of the first CPU to enter as the boot CPU (is the concern implementations where the CPU node's reg is not the same as what's in PIR?), This is something that has been fixed by using init_thread_info.cpu above, we don't need to change the current method of booting we are doing. I was just making some additional fixes Ben requested while I was looking at the same code fix some kexec issues with device tree ordering. Basically (I think) all we need to fix the device tree order is the following: -extern int boot_cpuid; +#define boot_cpuid (init_thread_info.cpu) And all the other stuff could remain the same. That is we check some variable and see if it's -1, if it is we are the boot cpu and we change that variable to something else and the other cpus that boot will know they are secondary cpus. how about just having a variable that gets set before releasing secondaries? If you're in the boot entry code and that variable is set, you're a secondary. Or, use a distinct release address for secondaries rather than __early_start. The former method is the way things are working now, and the latter is another possible solution which would require some more though/work for me than the current changes. -M ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On 12/20/2011 12:44 PM, McClintock Matthew-B29882 wrote: On Fri, Dec 16, 2011 at 3:29 PM, Scott Wood scottw...@freescale.com wrote: It would be nice if we could eliminate all usage of the boot cpu dtb field -- it's easy to forget to set it, especially if you're not making an AMP config. The default -1 means this patch would break booting with such a tree. I can add a check here to see if the boot cpu in the device tree is -1 to assume this is the boot cpu in addition to the boot cpu matching the PIR. Won't that break on secondaries with a -1 dtb? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On 12/15/2011 09:35 PM, Benjamin Herrenschmidt wrote: On Fri, 2011-12-16 at 03:29 +, McClintock Matthew-B29882 wrote: On Thu, Dec 15, 2011 at 9:12 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2011-11-28 at 22:24 -0600, Matthew McClintock wrote: boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. Where does the ordering matter currently? So what about head_fsl_booke.S comparing boot_cpuid to -1 ? That seems to be broken now in at least 2 ways, boot_cpuid doesn't exist anymore and you don't initialize it to -1 either... This is 4/5 which is also waiting for your review. http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-October/093474.html Ah missed that. This is FSL specific, I'd need Kumar and/or Scott's ack for that one. It would be nice if we could eliminate all usage of the boot cpu dtb field -- it's easy to forget to set it, especially if you're not making an AMP config. The default -1 means this patch would break booting with such a tree. If we don't want to record the PIR of the first CPU to enter as the boot CPU (is the concern implementations where the CPU node's reg is not the same as what's in PIR?), how about just having a variable that gets set before releasing secondaries? If you're in the boot entry code and that variable is set, you're a secondary. Or, use a distinct release address for secondaries rather than __early_start. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On Fri, 2011-12-16 at 15:29 -0600, Scott Wood wrote: It would be nice if we could eliminate all usage of the boot cpu dtb field -- it's easy to forget to set it, especially if you're not making an AMP config. The default -1 means this patch would break booting with such a tree. If we don't want to record the PIR of the first CPU to enter as the boot CPU (is the concern implementations where the CPU node's reg is not the same as what's in PIR?), how about just having a variable that gets set before releasing secondaries? If you're in the boot entry code and that variable is set, you're a secondary. Or, use a distinct release address for secondaries rather than __early_start. Of course you can only do that on processors that have a reliable PIR :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On Mon, 2011-11-28 at 22:24 -0600, Matthew McClintock wrote: boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. So what about head_fsl_booke.S comparing boot_cpuid to -1 ? That seems to be broken now in at least 2 ways, boot_cpuid doesn't exist anymore and you don't initialize it to -1 either... Cheers, Ben. Signed-off-by: Matthew McClintock m...@freescale.com --- v2: Fix compile issue for peries Remove '-1' initial value arch/powerpc/include/asm/smp.h |2 +- arch/powerpc/kernel/setup_32.c |5 +++-- arch/powerpc/kernel/setup_64.c |1 - arch/powerpc/sysdev/xics/xics-common.c |1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index adba970..f26c554 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -29,7 +29,7 @@ #endif #include asm/percpu.h -extern int boot_cpuid; +#define boot_cpuid (init_thread_info.cpu) extern int spinning_secondaries; extern void cpu_die(void); diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c index ac76108..8d4df4c 100644 --- a/arch/powerpc/kernel/setup_32.c +++ b/arch/powerpc/kernel/setup_32.c @@ -46,10 +46,11 @@ extern void bootx_init(unsigned long r4, unsigned long phys); -int boot_cpuid = -1; -EXPORT_SYMBOL_GPL(boot_cpuid); +/* we need a place to store phys cpu for non-SMP case */ +#ifndef CONFIG_SMP int boot_cpuid_phys; EXPORT_SYMBOL_GPL(boot_cpuid_phys); +#endif int smp_hw_index[NR_CPUS]; diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index fb9bb46..6d0f00f 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -73,7 +73,6 @@ #define DBG(fmt...) #endif -int boot_cpuid = 0; int __initdata spinning_secondaries; u64 ppc64_pft_size; diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c index d72eda6..8998b7a 100644 --- a/arch/powerpc/sysdev/xics/xics-common.c +++ b/arch/powerpc/sysdev/xics/xics-common.c @@ -20,6 +20,7 @@ #include linux/of.h #include linux/slab.h #include linux/spinlock.h +#include linux/sched.h #include asm/prom.h #include asm/io.h ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On Thu, Dec 15, 2011 at 9:12 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2011-11-28 at 22:24 -0600, Matthew McClintock wrote: boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. So what about head_fsl_booke.S comparing boot_cpuid to -1 ? That seems to be broken now in at least 2 ways, boot_cpuid doesn't exist anymore and you don't initialize it to -1 either... This is 4/5 which is also waiting for your review. http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-October/093474.html -M ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On Fri, 2011-12-16 at 03:29 +, McClintock Matthew-B29882 wrote: On Thu, Dec 15, 2011 at 9:12 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2011-11-28 at 22:24 -0600, Matthew McClintock wrote: boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. So what about head_fsl_booke.S comparing boot_cpuid to -1 ? That seems to be broken now in at least 2 ways, boot_cpuid doesn't exist anymore and you don't initialize it to -1 either... This is 4/5 which is also waiting for your review. http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-October/093474.html Ah missed that. This is FSL specific, I'd need Kumar and/or Scott's ack for that one. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
On Thu, Dec 15, 2011 at 9:35 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: This is 4/5 which is also waiting for your review. http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-October/093474.html Ah missed that. This is FSL specific, I'd need Kumar and/or Scott's ack for that one. I believe Kumar was waiting on your review of 5/5. I'll let you guys discuss. -M ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] powerpc/setup_{32, 64}.c: remove unneeded boot_cpuid{, _phys}
boot_cpuid and init_thread_info.cpu are redundant, just use the var that stays around longer and add a define to make boot_cpuid point at the correct value boot_cpudid_phys is not needed and can completly go away from the SMP case, we leave it there for the non-SMP case since the paca struct is not around to store this info This patch also has the effect of having the logical cpu number of the boot cpu be updated correctly independently of the ordering of the cpu nodes in the device tree. Signed-off-by: Matthew McClintock m...@freescale.com --- v2: Fix compile issue for peries Remove '-1' initial value arch/powerpc/include/asm/smp.h |2 +- arch/powerpc/kernel/setup_32.c |5 +++-- arch/powerpc/kernel/setup_64.c |1 - arch/powerpc/sysdev/xics/xics-common.c |1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index adba970..f26c554 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -29,7 +29,7 @@ #endif #include asm/percpu.h -extern int boot_cpuid; +#define boot_cpuid (init_thread_info.cpu) extern int spinning_secondaries; extern void cpu_die(void); diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c index ac76108..8d4df4c 100644 --- a/arch/powerpc/kernel/setup_32.c +++ b/arch/powerpc/kernel/setup_32.c @@ -46,10 +46,11 @@ extern void bootx_init(unsigned long r4, unsigned long phys); -int boot_cpuid = -1; -EXPORT_SYMBOL_GPL(boot_cpuid); +/* we need a place to store phys cpu for non-SMP case */ +#ifndef CONFIG_SMP int boot_cpuid_phys; EXPORT_SYMBOL_GPL(boot_cpuid_phys); +#endif int smp_hw_index[NR_CPUS]; diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index fb9bb46..6d0f00f 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -73,7 +73,6 @@ #define DBG(fmt...) #endif -int boot_cpuid = 0; int __initdata spinning_secondaries; u64 ppc64_pft_size; diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c index d72eda6..8998b7a 100644 --- a/arch/powerpc/sysdev/xics/xics-common.c +++ b/arch/powerpc/sysdev/xics/xics-common.c @@ -20,6 +20,7 @@ #include linux/of.h #include linux/slab.h #include linux/spinlock.h +#include linux/sched.h #include asm/prom.h #include asm/io.h -- 1.7.6.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
