| Message ID | 1312823424-9654-5-git-send-email-will.deacon@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/08/2011 10:10 AM, Will Deacon wrote: > > > +void __init smp_setup_processor_id(void) > +{ > + u32 mpidr; > + > + /* Read the MPIDR to find the hardware ID of the current CPU. */ > + asm("1: mrc p15, 0, %0, c0, c0, 5\n" > + " .pushsection \".alt.smp.init\", \"a\"\n" > + " .long 1b\n" > + " mov %0, #0\n" > + " .popsection" > + : "=r" (mpidr)); Would it be a good idea to put this into asm/cputype.h? I suppose the smp alternatives part would need to be written in C. mpidr = is_smp() ? read_cpuid_mpidr() : 0; > > + > + current_thread_info()->cpu = mpidr & 0xff; > + printk("Booting Linux on CPU %d\n", current_thread_info()->cpu); > +} > + Should there be a KERN_INFO there?
On Mon, Aug 08, 2011 at 06:33:38PM +0100, Stephen Boyd wrote: > On 08/08/2011 10:10 AM, Will Deacon wrote: > > > > > > +void __init smp_setup_processor_id(void) > > +{ > > + u32 mpidr; > > + > > + /* Read the MPIDR to find the hardware ID of the current CPU. */ > > + asm("1: mrc p15, 0, %0, c0, c0, 5\n" > > + " .pushsection \".alt.smp.init\", \"a\"\n" > > + " .long 1b\n" > > + " mov %0, #0\n" > > + " .popsection" > > + : "=r" (mpidr)); > > Would it be a good idea to put this into asm/cputype.h? I suppose the > smp alternatives part would need to be written in C. > > mpidr = is_smp() ? read_cpuid_mpidr() : 0; Yes, that's much better. We used to have asm/smp_mpidr.h which did this, so I lifted the code from there but doing it in C is easier to read. > > > > + > > + current_thread_info()->cpu = mpidr & 0xff; > > + printk("Booting Linux on CPU %d\n", current_thread_info()->cpu); > > +} > > + > > Should there be a KERN_INFO there? Can be. I just followed suit with secondary_start_kernel, but smp.c doesn't seem to be consistent anyway so I'll add the specifier. Thanks for the comments, Will
On Mon, Aug 08, 2011 at 07:14:59PM +0100, Will Deacon wrote: > On Mon, Aug 08, 2011 at 06:33:38PM +0100, Stephen Boyd wrote: > > On 08/08/2011 10:10 AM, Will Deacon wrote: > > > > > > > > > +void __init smp_setup_processor_id(void) > > > +{ > > > + u32 mpidr; > > > + > > > + /* Read the MPIDR to find the hardware ID of the current CPU. */ > > > + asm("1: mrc p15, 0, %0, c0, c0, 5\n" > > > + " .pushsection \".alt.smp.init\", \"a\"\n" > > > + " .long 1b\n" > > > + " mov %0, #0\n" > > > + " .popsection" > > > + : "=r" (mpidr)); > > > > Would it be a good idea to put this into asm/cputype.h? I suppose the > > smp alternatives part would need to be written in C. > > > > mpidr = is_smp() ? read_cpuid_mpidr() : 0; > > Yes, that's much better. We used to have asm/smp_mpidr.h which did this, so I > lifted the code from there but doing it in C is easier to read. > > > > > > > + > > > + current_thread_info()->cpu = mpidr & 0xff; > > > + printk("Booting Linux on CPU %d\n", current_thread_info()->cpu); > > > +} > > > + > > > > Should there be a KERN_INFO there? > > Can be. I just followed suit with secondary_start_kernel, but smp.c doesn't > seem to be consistent anyway so I'll add the specifier. Actually, it's probably much better to keep logical CPU0 as the boot CPU. The hotplug code makes the assumption that the lowest online CPU number is the boot CPU. So unless we want even more pain, I suggest we just ensure logical CPU0 is the boot CPU. We're already fairly clean on our interactions between CPU numbering and the hardware CPU numbering - there's only a limited number of places where the two interact. Those are the GIC, IPI, secondary CPU bringup, and hotplug code. I suggest that we preserve the independence, and introduce a mapping between logical CPU numbering and hardware CPU numbering.
Hi Russell, On Mon, Aug 08, 2011 at 09:02:56PM +0100, Russell King - ARM Linux wrote: > On Mon, Aug 08, 2011 at 07:14:59PM +0100, Will Deacon wrote: > > Can be. I just followed suit with secondary_start_kernel, but smp.c doesn't > > seem to be consistent anyway so I'll add the specifier. > > Actually, it's probably much better to keep logical CPU0 as the boot CPU. > The hotplug code makes the assumption that the lowest online CPU number > is the boot CPU. Yes, you're right. I only tested this on a dual-core platform so I didn't get a chance to play with hotplug (except to see that it failed, like I expected). That assumption is also in the generic code so I guess it's easier to leave it be. > So unless we want even more pain, I suggest we just ensure logical CPU0 > is the boot CPU. Right. Ensuring the logical CPU number isn't too hard, but we should aim to support an arbitrary physical CPU number for the boot ID, even if most platforms can't cater for that at the moment. > We're already fairly clean on our interactions between CPU numbering and > the hardware CPU numbering - there's only a limited number of places where > the two interact. Those are the GIC, IPI, secondary CPU bringup, and > hotplug code. I suggest that we preserve the independence, and introduce > a mapping between logical CPU numbering and hardware CPU numbering. This was my initial approach but then I thought I'd try and cheat my messing with the logical numbering. I'll look at implementing the mapping and then updating the GIC and friends to indirect their CPU number lookups through that instead. I'll post it as its own series since it will be more than just a simple hack now. Stay tuned... Cheers, Will
Hello. On 08-08-2011 21:10, Will Deacon wrote: > On ARM, Linux assumes that the boot CPU has ID 0. If this ends up being > out of sync with the hardware CPU number, we will configure the GIC > incorrectly and route interrupts to the CPU with hardware ID 0. > This patch implements smp_setup_processor_id for ARM, using the MPIDR to > set the CPU of the boot thread. > Signed-off-by: Will Deacon<will.deacon@arm.com> > --- > arch/arm/kernel/smp.c | 16 ++++++++++++++++ > 1 files changed, 16 insertions(+), 0 deletions(-) > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > index d88ff02..8e60a4f 100644 > --- a/arch/arm/kernel/smp.c > +++ b/arch/arm/kernel/smp.c > @@ -259,6 +259,22 @@ void __ref cpu_die(void) > } > #endif /* CONFIG_HOTPLUG_CPU */ > > +void __init smp_setup_processor_id(void) > +{ > + u32 mpidr; > + > + /* Read the MPIDR to find the hardware ID of the current CPU. */ > + asm("1: mrc p15, 0, %0, c0, c0, 5\n" > + " .pushsection \".alt.smp.init\", \"a\"\n" > + " .long 1b\n" > + " mov %0, #0\n" > + " .popsection" > + : "=r" (mpidr)); > + > + current_thread_info()->cpu = mpidr& 0xff; > + printk("Booting Linux on CPU %d\n", current_thread_info()->cpu); printk() should have the KERN_* logging facility. WBR, Sergei
On Tue, Aug 09, 2011 at 12:48:05PM +0100, Sergei Shtylyov wrote: > Hello. > > On 08-08-2011 21:10, Will Deacon wrote: > > > On ARM, Linux assumes that the boot CPU has ID 0. If this ends up being > > out of sync with the hardware CPU number, we will configure the GIC > > incorrectly and route interrupts to the CPU with hardware ID 0. > > > This patch implements smp_setup_processor_id for ARM, using the MPIDR to > > set the CPU of the boot thread. > > > Signed-off-by: Will Deacon<will.deacon@arm.com> > > --- > > arch/arm/kernel/smp.c | 16 ++++++++++++++++ > > 1 files changed, 16 insertions(+), 0 deletions(-) > > > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > > index d88ff02..8e60a4f 100644 > > --- a/arch/arm/kernel/smp.c > > +++ b/arch/arm/kernel/smp.c > > @@ -259,6 +259,22 @@ void __ref cpu_die(void) > > } > > #endif /* CONFIG_HOTPLUG_CPU */ > > > > +void __init smp_setup_processor_id(void) > > +{ > > + u32 mpidr; > > + > > + /* Read the MPIDR to find the hardware ID of the current CPU. */ > > + asm("1: mrc p15, 0, %0, c0, c0, 5\n" > > + " .pushsection \".alt.smp.init\", \"a\"\n" > > + " .long 1b\n" > > + " mov %0, #0\n" > > + " .popsection" > > + : "=r" (mpidr)); > > + > > + current_thread_info()->cpu = mpidr& 0xff; > > + printk("Booting Linux on CPU %d\n", current_thread_info()->cpu); > > printk() should have the KERN_* logging facility. There's an echo around here :) This patch is pretty much dead anyway after the discussion with Russell. I'll post some other guys soon. Will
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index d88ff02..8e60a4f 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -259,6 +259,22 @@ void __ref cpu_die(void) } #endif /* CONFIG_HOTPLUG_CPU */ +void __init smp_setup_processor_id(void) +{ + u32 mpidr; + + /* Read the MPIDR to find the hardware ID of the current CPU. */ + asm("1: mrc p15, 0, %0, c0, c0, 5\n" + " .pushsection \".alt.smp.init\", \"a\"\n" + " .long 1b\n" + " mov %0, #0\n" + " .popsection" + : "=r" (mpidr)); + + current_thread_info()->cpu = mpidr & 0xff; + printk("Booting Linux on CPU %d\n", current_thread_info()->cpu); +} + /* * Called by both boot and secondaries to move global data into * per-processor storage.
On ARM, Linux assumes that the boot CPU has ID 0. If this ends up being out of sync with the hardware CPU number, we will configure the GIC incorrectly and route interrupts to the CPU with hardware ID 0. This patch implements smp_setup_processor_id for ARM, using the MPIDR to set the CPU of the boot thread. Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm/kernel/smp.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-)