mach-spear SMP questions
I'm doing some code cleanup and have a question about something related to SMP in mach-spear. Can anyone help me with these questions?
My question has to do with how secondary cores start for SMP. Most machines need to send a wakeup event from the boot core to each secondary core as it is told to start. But mach-spear does not do this. I'd like to know if this is an oversight, or confirm that this is simply not necessary for this machine.
Also, while most machines touch a memory location and then do a targeted cache involving only that location, mach-spear does a full flush. Is the full flush really required for some reason?
Thanks.
-Alex
Quoted 3 lines · Alex Elder
I'm doing some code cleanup and have a question about something related to SMP in mach-spear. Can anyone help me with these questions?
I think you're working in a similar area to that which I have been - but with different ends in mind. Have a look for my L2 cache series on LAKML.
Quoted 6 lines
My question has to do with how secondary cores start for SMP. Most machines need to send a wakeup event from the boot core to each secondary core as it is told to start. But mach-spear does not do this. I'd like to know if this is an oversight, or confirm that this is simply not necessary for this machine.
It could be that the "BootMonitor" waits for spear13xx_smp_prepare_cpus() to write the address of the function to jump to into SYS_LOCATION - I can't see any other way this code would work (there's nothing else there to tell the secondary CPUs where to start executing from or when to start executing from that point.)
Quoted 4 lines
Also, while most machines touch a memory location and then do a targeted cache involving only that location, mach-spear does a full flush. Is the full flush really required for some reason?
It's not required. As part of the L2 cache cleanup, I have this change:
Author: Russell King <[email protected]> Date: Sat Mar 15 16:47:47 2014 +0000
ARM: l2c: avoid calling outer_flush_all() unnecessarily (Spear)
Spear calls outer_flush_all() from it's SMP bringup function. This is potentially dangerous as the L2C set/way operations which implement this don't take kindly to concurrent operations. Besides, there's better solutions to this, as implemented on other platforms.
@@ -20,6 +20,18 @@
+/*
+ * Write pen_release in a way that is guaranteed to be visible to all
+ * observers, irrespective of whether they're taking part in coherency
+ * or not. This is necessary for the hotplug code to work reliably.
+ */
+static void write_pen_release(int val)
+{
+ pen_release = val; + smp_wmb(); + sync_cache_w(&pen_release);
+}
+
...
@@ -30,8 +42,7 @@ static void spear13xx_secondary_init(unsigned int cpu)
- pen_release = -1; - smp_wmb(); + write_pen_release(-1); ... @@ -58,9 +69,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct t - pen_release = cpu; - flush_cache_all(); - outer_flush_all(); + write_pen_release(cpu);
for Spear, which makes it behave like every other platform we have using this pen release method.
Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it.
Fixing Shiraz's id and Cc'ing Vipin as well..
Quoted 20 lines · Alex Elder
I'm doing some code cleanup and have a question about something related to SMP in mach-spear. Can anyone help me with these questions?
My question has to do with how secondary cores start for SMP. Most machines need to send a wakeup event from the boot core to each secondary core as it is told to start. But mach-spear does not do this. I'd like to know if this is an oversight, or confirm that this is simply not necessary for this machine.
Also, while most machines touch a memory location and then do a targeted cache involving only that location, mach-spear does a full flush. Is the full flush really required for some reason?
Thanks.
-Alex
Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Alex Elder
I'm doing some code cleanup and have a question about something related to SMP in mach-spear. Can anyone help me with these questions?
I think you're working in a similar area to that which I have been - but with different ends in mind. Have a look for my L2 cache series on LAKML.
By "different ends in mind," do you mean conflicting goals, or simply for different reasons?
Quoted 7 lines
Quoted 6 lines
My question has to do with how secondary cores start for SMP. Most machines need to send a wakeup event from the boot core to each secondary core as it is told to start. But mach-spear does not do this. I'd like to know if this is an oversight, or confirm that this is simply not necessary for this machine.
Quoted 5 lines
It could be that the "BootMonitor" waits for
spear13xx_smp_prepare_cpus()to write the address of the function to jump to intoSYS_LOCATION- I can't see any other way this code would work (there's nothing else there to tell the secondary CPUs where to start executing from or when to start executing from that point.)
That could be.
My next question is whether sending a wakeup event would be *harmful*. If it's not, this machine can use the common pen-release code I'm consolidating things to use.
Quoted 7 lines
Quoted 4 lines
Also, while most machines touch a memory location and then do a targeted cache involving only that location, mach-spear does a full flush. Is the full flush really required for some reason?
It's not required. As part of the L2 cache cleanup, I have this change:
That's perfect. Just like all other implementations (so I can factor it out). Now I just need to know whether I can use the shared the smp_boot_secondary method.
Thanks.
-Alex
Quoted 40 lines
Author: Russell King <[email protected]> Date: Sat Mar 15 16:47:47 2014 +0000ARM: l2c: avoid calling
outer_flush_all()unnecessarily (Spear)Spear calls
outer_flush_all()from it's SMP bringup function. This is potentially dangerous as the L2C set/way operations which implement this don't take kindly to concurrent operations. Besides, there's better solutions to this, as implemented on other platforms.Signed-off-by Russell King <[email protected]>(quoted diff) +14
-5@@ -20,6 +20,18 @@ +/* + * Write pen_release in a way that is guaranteed to be visible to all + * observers, irrespective of whether they're taking part in coherency + * or not. This is necessary for the hotplug code to work reliably. + */ +static void write_pen_release(int val) +{ + pen_release = val; + smp_wmb(); + sync_cache_w(&pen_release); +} +@@ -30,8 +42,7 @@ static void spear13xx_secondary_init(unsigned int cpu) - pen_release = -1; - smp_wmb(); + write_pen_release(-1);@@ -58,9 +69,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct t - pen_release = cpu; - flush_cache_all(); - outer_flush_all(); + write_pen_release(cpu);for Spear, which makes it behave like every other platform we have using this pen release method.
Quoted 11 lines · Alex Elder
Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Alex Elder
I'm doing some code cleanup and have a question about something related to SMP in mach-spear. Can anyone help me with these questions?
I think you're working in a similar area to that which I have been - but with different ends in mind. Have a look for my L2 cache series on LAKML.
By "different ends in mind," do you mean conflicting goals, or simply for different reasons?
Hmm, "L2 cache series" doesn't suggest anything? The L2 cache series is... a series cleaning up the L2 cache code, which includes removing incorrect uses of outer_flush_all().
Merely removing that call would likely break Spear, so it had to be fixed properly.
Quoted 13 lines
Quoted 7 lines
Quoted 6 lines
My question has to do with how secondary cores start for SMP. Most machines need to send a wakeup event from the boot core to each secondary core as it is told to start. But mach-spear does not do this. I'd like to know if this is an oversight, or confirm that this is simply not necessary for this machine.
Quoted 5 lines
It could be that the "BootMonitor" waits for
spear13xx_smp_prepare_cpus()to write the address of the function to jump to intoSYS_LOCATION- I can't see any other way this code would work (there's nothing else there to tell the secondary CPUs where to start executing from or when to start executing from that point.)
Quoted 5 lines
That could be.
My next question is whether sending a wakeup event would be *harmful*. If it's not, this machine can use the common pen-release code I'm consolidating things to use.
I've seen this attempt many times before... you're not the first. Somehow, saying "no" to something never sticks. It just postpones it until someone else thinks it's a good idea and tries the same thing again. :(
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Also, while most machines touch a memory location and then do a targeted cache involving only that location, mach-spear does a full flush. Is the full flush really required for some reason?
Quoted 7 lines
Quoted 2 lines
It's not required. As part of the L2 cache cleanup, I have this change:
That's perfect. Just like all other implementations (so I can factor it out). Now I just need to know whether I can use the shared the
smp_boot_secondarymethod.
People keep trying to do this, and I keep saying no to it because (as I keep stating each time this comes up) the majority of implementations haven't /thought/ about their hotplug implementations at all, and have just gone down the path of "let's copy-n-paste the first solution we find" which has historically been that used on the ARM Realview platform.
The problem with that is the ARM Realview is a really dumb piece of
hardware: it has no control over the secondary CPUs. When it powers up, all CPUs come out of reset simultaneously, and enter the boot monitor.
The boot monitor then holds the secondary CPUs itself, allowing the boot CPU to run the rest of the boot monitor code, eventually falling through to the boot loader and ultimately to Linux.
Linux then sets the address of the CPU holding pen code, and pokes the boot monitor to release the secondary CPUs to that holding place - it results in all three secondary CPUs ending up there simultaneously. The holding pen's purpose is to hold the CPUs there while Linux continues the boot process to the point of requesting each CPU individually to come up.
The secondary function of the pen release code is to deal with hotplug on this platform: once a secondary CPU is released into the kernel code, there is no way to take it back out of the kernel. So, the CPU hotplug code has to "fake" taking a CPU offline. It does this by taking the CPU out of coherency, and putting it into a low power state, waiting for an event to wake it back up. That wakeup event is based upon the pen thing again selecting the appropriate CPU to "plug" back in.
This approach also has the problem that it is /not/ kexec friendly - since we can't get rid of the secondary CPUs, a kexec'd kernel ends up simply overwriting the code that the secondary CPUs happened to be executing at the time, probably crashing them. In any case, the secondary CPUs will not come back up after a kexec since they'll probably have crashed.
In the case of a system where the secondary CPUs can be held in reset independently of the boot CPU, that hardware should be used when bringing up or taking down a secondary CPU. If you have that facility, then there is absolutely no reason to use the "pen" stuff.
I consider any platform which copies the Realview SMP code to be broken by definition. I do not wish this code to be turned into some kind of generic helper, because by doing so, we're endorsing it as a proper way to do something. It is *not* a proper way to implement this. It's the implementation used by ARM Ltd's platforms for the sole reason that this hardware has no form of controls on the secondary CPUs what so ever.
Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it.
Quoted 10 lines · Russell King - ARM Linux
Quoted 9 lines · Alex Elder
Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Alex Elder
I'm doing some code cleanup and have a question about something related to SMP in mach-spear. Can anyone help me with these questions?
I think you're working in a similar area to that which I have been - but with different ends in mind. Have a look for my L2 cache series on LAKML.
Quoted 6 lines
Quoted 2 lines
By "different ends in mind," do you mean conflicting goals, or simply for different reasons?
Hmm, "L2 cache series" doesn't suggest anything? The L2 cache series is... a series cleaning up the L2 cache code, which includes removing incorrect uses of
outer_flush_all().
I merely wanted to be clarify your meaning.
Quoted 18 lines
Merely removing that call would likely break Spear, so it had to be fixed properly.
Quoted 15 lines
Quoted 7 lines
Quoted 6 lines
My question has to do with how secondary cores start for SMP. Most machines need to send a wakeup event from the boot core to each secondary core as it is told to start. But mach-spear does not do this. I'd like to know if this is an oversight, or confirm that this is simply not necessary for this machine.
Quoted 5 lines
It could be that the "BootMonitor" waits for
spear13xx_smp_prepare_cpus()to write the address of the function to jump to intoSYS_LOCATION- I can't see any other way this code would work (there's nothing else there to tell the secondary CPUs where to start executing from or when to start executing from that point.)That could be.
Quoted 8 lines
Quoted 3 lines
My next question is whether sending a wakeup event would be *harmful*. If it's not, this machine can use the common pen-release code I'm consolidating things to use.
I've seen this attempt many times before... you're not the first. Somehow, saying "no" to something never sticks. It just postpones it until someone else thinks it's a good idea and tries the same thing again. :(
Maybe this time will be better? :)
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Also, while most machines touch a memory location and then do a targeted cache involving only that location, mach-spear does a full flush. Is the full flush really required for some reason?
Quoted 7 lines
Quoted 2 lines
It's not required. As part of the L2 cache cleanup, I have this change:
That's perfect. Just like all other implementations (so I can factor it out). Now I just need to know whether I can use the shared the
smp_boot_secondarymethod.
Quoted 6 lines
People keep trying to do this, and I keep saying no to it because (as I keep stating each time this comes up) the majority of implementations haven't /thought/ about their hotplug implementations at all, and have just gone down the path of "let's copy-n-paste the first solution we find" which has historically been that used on the ARM Realview platform.
It looks that way to me. But how do you stop people from spreading the copy/paste disease if there's no obvious alternative--or if it appears that it's what "everybody does?"
What I really am trying to do is get to the point where we can specify an "enable-method" in the device tree. To do that I looked at how these various platforms were starting their secondary cores and they're almost all the same "pen" method. And obvious first step is to factor out the common code.
Quoted 9 lines
The problem with that is the ARM Realview is a really dumb piece of
hardware: it has no control over the secondary CPUs. When it powers up, all CPUs come out of reset simultaneously, and enter the boot monitor.The boot monitor then holds the secondary CPUs itself, allowing the boot CPU to run the rest of the boot monitor code, eventually falling through to the boot loader and ultimately to Linux.
Quoted 6 lines
Linux then sets the address of the CPU holding pen code, and pokes the boot monitor to release the secondary CPUs to that holding place - it results in all three secondary CPUs ending up there simultaneously. The holding pen's purpose is to hold the CPUs there while Linux continues the boot process to the point of requesting each CPU individually to come up.
Yes, that's exactly how it works.
Quoted 7 lines
The secondary function of the pen release code is to deal with hotplug on this platform: once a secondary CPU is released into the kernel code, there is no way to take it back out of the kernel. So, the CPU hotplug code has to "fake" taking a CPU offline. It does this by taking the CPU out of coherency, and putting it into a low power state, waiting for an event to wake it back up. That wakeup event is based upon the pen thing again selecting the appropriate CPU to "plug" back in.
At this point I have not been looking at its use in hotplug.
Quoted 6 lines
This approach also has the problem that it is /not/ kexec friendly - since we can't get rid of the secondary CPUs, a kexec'd kernel ends up simply overwriting the code that the secondary CPUs happened to be executing at the time, probably crashing them. In any case, the secondary CPUs will not come back up after a kexec since they'll probably have crashed.
Why is this even used then? (Sorry, like I said, I've been looking at SMP startup only.)
Quoted 4 lines
In the case of a system where the secondary CPUs can be held in reset independently of the boot CPU, that hardware should be used when bringing up or taking down a secondary CPU. If you have that facility, then there is absolutely no reason to use the "pen" stuff.
And what if there is no such facility?
Quoted 6 lines
I consider any platform which copies the Realview SMP code to be broken by definition. I do not wish this code to be turned into some kind of generic helper, because by doing so, we're endorsing it as a proper way to do something. It is *not* a proper way to implement this. It's the implementation used by ARM Ltd's platforms for the sole reason that this hardware has no form of controls on the secondary CPUs what so ever.
I have no ability to change how these machines control their secondary CPUs. Nor do I have knowledge of whether they have the ability to independently hold secondary cores in reset. Even if they did, I don't have the information I'd require to implement such a thing so they do it the right way.
What I *can* do is identify a half dozen essentially identical implementations of code littering the code base and consolidate them into a single implementation. In doing so I will not change their behavior at all (unpleasant as it may be), but I will improve the code overall. It will make it transparent that these are in fact all doing the same thing, and I hope it will lead to it becoming easier to implement SMP startup in a better way for all implementations if/when there is such a thing. As I said above, my broader aim is to support a device-tree specified way of booting secondary cores, and in doing so I'm trying to be consistent with how it's being done for ARM64.
I'm not endorsing this way of starting secondary cores. But looking at all these similar (but not quite identical) blocks of code makes it harder than it should be to get a handle on what's going on, and suggests that maybe they *must* all have custom implementations.
I understand your point that having a common set of functions implies an endorsement of that way of doing things. But I'd argue that everyone using a shared implementation (even if it's the wrong way) is better than everyone *copying* the same implementation. People can be pressured at review time to do it differently if there is a better way available.
I think everyone knows that duplicated code is bad, and that's why I'm not the first one to set out to do this. I'm about half done and I find that it adds about 120 new lines of common code but removes 340 lines that were duplicated (including 5 files completely going away). All of this duplicated code is under the arch/arm/mach-* directories, by the way, so this helps the goal of doing away with that stuff as well.
Even if "everybody is doing it wrong," I feel pretty strongly that the net effect of consolidating is positive.
I hope you'll reconsider your "no." I'll have the patches available soon (to allow an objective look at the outcome).
-Alex