ARM: v7 setup function should invalidate L1 cache

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

Tested-by Michael Niewoehner <[email protected]>

Tested on Radxa Rock Pro with RK3188.
The kernel panics on reboot I had before and also a kernel BUG when running "memtester 1900M" went away and the rock seems to run stable now.

Michael Niewoehner tested this on a rk3188 (Cortex-A9) and wrote in [0]

Quoted 5 lines

Tested-by Michael Niewoehner <[email protected]>

Tested on Radxa Rock Pro with RK3188.
The kernel panics on reboot I had before and also a kernel BUG when running "memtester 1900M" went away and the rock seems to run stable now.

Quoted 8 lines · On Tuesday 19 May 2015 23:44:58 Heiko Stuebner wrote:

Michael Niewoehner tested this on a rk3188 (Cortex-A9) and wrote in [0]

Quoted 5 lines

Tested-by Michael Niewoehner <[email protected]>

Tested on Radxa Rock Pro with RK3188.
The kernel panics on reboot I had before and also a kernel BUG when running "memtester 1900M" went away and the rock seems to run stable now.

We should probably create a separate fix for that and add it to the stable kernels then. I would suggest something like the untested patch below, which takes advantage of the fact that we already have separate assignments for the start_secondary function pointer for A9 and A17.

If you look carefully at this, you'll notice that it's utter crap. (Sorry, but it is.) It has two problems:

1. It'll never match a Cortex-A9 CPU. Cortex-A9 has a MIDR value of 0x412fc09a, not 0x00000c09. The bit position of the part number field isn't even right.

2. If it does match, then we branch to "v7_invalidate_l1" without setting the link register: we'll never return back here (we'll return to whatever random value the link register contains) and so we'll never make it to secondary_startup. *Thankfully*, because of (1), this branch will never be taken - this is it's saving grace.

Your patch introduces a /third/ form of crapiness:

3. If the PSR happens to have Z=1, the "beq" instruction will be taken, thereby crashing the system because of (2).

The /simplest/ change which would fix this problem is to just change proc-v7.S. The remainder is effectively a cleanup removing redundant code.

Quoted 3 lines · On Tuesday 19 May 2015 23:07:21 Russell King - ARM Linux wrote:

The /simplest/ change which would fix this problem is to just change proc-v7.S. The remainder is effectively a cleanup removing redundant code.

Fair enough. I wasn't sure if we're confident enough about that change already to put it into stable backports. If the risk is low enough, that's fine.

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

---

  arch/arm/mach-berlin/headsmp.S        |  6 ------
  arch/arm/mach-berlin/platsmp.c        |  3 +--

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

---

 arch/arm/mach-imx/headsmp.S           |  1 -

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

---

 arch/arm/mach-tegra/Makefile          |  2 +-
 arch/arm/mach-tegra/headsmp.S         | 12 ------------
 arch/arm/mach-tegra/reset.c           |  2 +-
 arch/arm/mach-tegra/reset.h           |  1 -

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

Quoted 18 lines · Russell King

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Quoted 5 lines

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

For various shmobile:
- sh73a0/kzm9g
- r8a7740/armadillo
- r8a73a4/ape6evm
- r8a7791/koelsch

Tested-by Geert Uytterhoeven <[email protected]>

FWIW, I have the feeling this has a slight influence on boot reliability on two of my boards:
- r8a7740/armadillo, which is known to suffer from a cache-related bug in its bootloader, seems to have a higher change of booting successfully on cold boot,
- sh73a0/kzm9g, which has known cache-issues with secondary CPU boot up, seems to have a lower chance of booting successfully.

No time to spend all week turning this into a statistical significant test project... The reset button is my friend...

Quoted 8 lines · Geert Uytterhoeven

FWIW, I have the feeling this has a slight influence on boot reliability on two of my boards:
- r8a7740/armadillo, which is known to suffer from a cache-related bug in its bootloader, seems to have a higher change of booting successfully on cold boot,
- sh73a0/kzm9g, which has known cache-issues with secondary CPU boot up, seems to have a lower chance of booting successfully.

Quoted 2 lines

No time to spend all week turning this into a statistical significant test project... The reset button is my friend...

Damn it, you sent this right after I merged and pushed out this change in my for-arm-soc branch, and was just about to send it to the arm-soc people. What excellent timing you have. :)

What happens on the kzm9g if you revert the mach-shmobile changes?

For armadillo, do you use the decompressor? That should be doing all the cache cleaning already, prior to the kernel being entered.

Hi Russell,

Quoted 9 lines · Russell King - ARM Linux

Quoted 8 lines · Geert Uytterhoeven

FWIW, I have the feeling this has a slight influence on boot reliability on two of my boards:
- r8a7740/armadillo, which is known to suffer from a cache-related bug in its bootloader, seems to have a higher change of booting successfully on cold boot,
- sh73a0/kzm9g, which has known cache-issues with secondary CPU boot up, seems to have a lower chance of booting successfully.

Quoted 6 lines

Quoted 2 lines

No time to spend all week turning this into a statistical significant test project... The reset button is my friend...

Damn it, you sent this right after I merged and pushed out this change in my for-arm-soc branch, and was just about to send it to the arm-soc people. What excellent timing you have. :)

Don't worry, I didn't send that email to make you postpone this change. Giving the fuzziness of reproduction, and the flakiness (esp. on Armadillo) of the boot loader, and these are old SoCs, please go ahead.

Quoted 1 line

What happens on the kzm9g if you revert the mach-shmobile changes?

Seems to make no difference.

Quoted 2 lines

For armadillo, do you use the decompressor? That should be doing all the cache cleaning already, prior to the kernel being entered.

I think so.

Corruption pattern ranges from lock up, over "Error: unrecognized/unsupported machine ID", to booting almost completely, but lacking a few devices due to a corrupted DTB. Been like that as long as I remember, i.e. since I got the board ca. 1 year ago. Boots fine (100%) with kexec.

Quoted 31 lines · Geert Uytterhoeven

Hi Russell,

Quoted 9 lines · Russell King - ARM Linux

Quoted 8 lines · Geert Uytterhoeven

FWIW, I have the feeling this has a slight influence on boot reliability on two of my boards:
- r8a7740/armadillo, which is known to suffer from a cache-related bug in its bootloader, seems to have a higher change of booting successfully on cold boot,
- sh73a0/kzm9g, which has known cache-issues with secondary CPU boot up, seems to have a lower chance of booting successfully.

Quoted 6 lines

Quoted 2 lines

No time to spend all week turning this into a statistical significant test project... The reset button is my friend...

Damn it, you sent this right after I merged and pushed out this change in my for-arm-soc branch, and was just about to send it to the arm-soc people. What excellent timing you have. :)

Don't worry, I didn't send that email to make you postpone this change. Giving the fuzziness of reproduction, and the flakiness (esp. on Armadillo) of the boot loader, and these are old SoCs, please go ahead.

Quoted 1 line

What happens on the kzm9g if you revert the mach-shmobile changes?

Seems to make no difference.

Quoted 2 lines

For armadillo, do you use the decompressor? That should be doing all the cache cleaning already, prior to the kernel being entered.

Quoted 7 lines

I think so.

Corruption pattern ranges from lock up, over "Error: unrecognized/unsupported machine ID", to booting almost completely, but lacking a few devices due to a corrupted DTB. Been like that as long as I remember, i.e. since I got the board ca. 1 year ago. Boots fine (100%) with kexec.

It seems like this patch is causing the SoCFPGA to not boot with SMP reliably. About 1 out of every 10 reboots, I'm seeing the boot failure below. The error seems to only happen when I do a cold or warm reboot, but never occurs during a power-up. If I revert this patch, or put back the call to v7_invalidate_l1 in socfpga_secondary_startup , then its able to boot 100% of the time.

Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted
4.1.0-rc8-next-20150617-00002-gdd1f624 #1
Hardware name: Altera SOCFPGA
task: eecaeac0 ti: eecce000 task.ti: eecce000
PC is at vfp_notifier+0x58/0x12c
LR is at notifier_call_chain+0x44/0x84

[<c000a6bc>] (vfp_notifier) from [<c003d134>] (notifier_call_chain+0x44/0x84) [<c003d134>] (notifier_call_chain) from [<c003d18c>] (__atomic_notifier_call_chain+0x18/0x20)
[<c003d18c>] (__atomic_notifier_call_chain) from [<c003d1ac>] (atomic_notifier_call_chain+0x18/0x20)
[<c003d1ac>] (atomic_notifier_call_chain) from [<c001369c>] (__switch_to+0x34/0x58)

Quoted 40 lines · Dinh Nguyen

Quoted 31 lines · Geert Uytterhoeven

Hi Russell,

Quoted 9 lines · Russell King - ARM Linux

Quoted 8 lines · Geert Uytterhoeven

FWIW, I have the feeling this has a slight influence on boot reliability on two of my boards:
- r8a7740/armadillo, which is known to suffer from a cache-related bug in its bootloader, seems to have a higher change of booting successfully on cold boot,
- sh73a0/kzm9g, which has known cache-issues with secondary CPU boot up, seems to have a lower chance of booting successfully.

Quoted 6 lines

Quoted 2 lines

No time to spend all week turning this into a statistical significant test project... The reset button is my friend...

Damn it, you sent this right after I merged and pushed out this change in my for-arm-soc branch, and was just about to send it to the arm-soc people. What excellent timing you have. :)

Don't worry, I didn't send that email to make you postpone this change. Giving the fuzziness of reproduction, and the flakiness (esp. on Armadillo) of the boot loader, and these are old SoCs, please go ahead.

Quoted 1 line

What happens on the kzm9g if you revert the mach-shmobile changes?

Seems to make no difference.

Quoted 2 lines

For armadillo, do you use the decompressor? That should be doing all the cache cleaning already, prior to the kernel being entered.

Quoted 7 lines

I think so.

Corruption pattern ranges from lock up, over "Error: unrecognized/unsupported machine ID", to booting almost completely, but lacking a few devices due to a corrupted DTB. Been like that as long as I remember, i.e. since I got the board ca. 1 year ago. Boots fine (100%) with kexec.

Quoted 6 lines

It seems like this patch is causing the SoCFPGA to not boot with SMP reliably. About 1 out of every 10 reboots, I'm seeing the boot failure below. The error seems to only happen when I do a cold or warm reboot, but never occurs during a power-up. If I revert this patch, or put back the call to v7_invalidate_l1 in socfpga_secondary_startup , then its able to boot 100% of the time.

It really sucks that you're only just testing this change now, because I've frozen my tree, and removing it for the next merge window is going to be an entirely non-trivial matter. You were copied on the original patch, which you failed to test... I can't say I have _much_ sympathy for a bug report at this point in time.

Quoted 48 lines · Russell King - ARM Linux

Quoted 40 lines · Dinh Nguyen

Quoted 31 lines · Geert Uytterhoeven

Hi Russell,

Quoted 9 lines · Russell King - ARM Linux

Quoted 8 lines · Geert Uytterhoeven

FWIW, I have the feeling this has a slight influence on boot reliability on two of my boards:
- r8a7740/armadillo, which is known to suffer from a cache-related bug in its bootloader, seems to have a higher change of booting successfully on cold boot,
- sh73a0/kzm9g, which has known cache-issues with secondary CPU boot up, seems to have a lower chance of booting successfully.

Quoted 6 lines

Quoted 2 lines

No time to spend all week turning this into a statistical significant test project... The reset button is my friend...

Damn it, you sent this right after I merged and pushed out this change in my for-arm-soc branch, and was just about to send it to the arm-soc people. What excellent timing you have. :)

Don't worry, I didn't send that email to make you postpone this change. Giving the fuzziness of reproduction, and the flakiness (esp. on Armadillo) of the boot loader, and these are old SoCs, please go ahead.

Quoted 1 line

What happens on the kzm9g if you revert the mach-shmobile changes?

Seems to make no difference.

Quoted 2 lines

For armadillo, do you use the decompressor? That should be doing all the cache cleaning already, prior to the kernel being entered.

Quoted 7 lines

I think so.

Corruption pattern ranges from lock up, over "Error: unrecognized/unsupported machine ID", to booting almost completely, but lacking a few devices due to a corrupted DTB. Been like that as long as I remember, i.e. since I got the board ca. 1 year ago. Boots fine (100%) with kexec.

Quoted 6 lines

It seems like this patch is causing the SoCFPGA to not boot with SMP reliably. About 1 out of every 10 reboots, I'm seeing the boot failure below. The error seems to only happen when I do a cold or warm reboot, but never occurs during a power-up. If I revert this patch, or put back the call to v7_invalidate_l1 in socfpga_secondary_startup , then its able to boot 100% of the time.

Quoted 6 lines

It really sucks that you're only just testing this change now, because I've frozen my tree, and removing it for the next merge window is going to be an entirely non-trivial matter. You were copied on the original patch, which you failed to test... I can't say I have _much_ sympathy for a bug report at this point in time.

I apologize for not catching this error while testing this patch. But I did test it when you first sent it out..I probably didn't do a stress test. Sometimes the reboot fails in the 1st attempt, sometimes it fails in the 9th attempt.

I only caught this error when I was testing my recent changes to use CPU_METHOD_OF_DECLARE.

For me, I don't think you need to revert this patch or anything, but a fix can go in for a -rcX?

Also, I am not seeing the error on the SoCFPGA Arria 10 platform at all. This Arria10 platform is running a different version of bootloader than the Cyclone5. Although, I also did test with the latest version of U-Boot on the Cyclone5.

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

---

 arch/arm/mach-zynq/common.h           |  2 --
 arch/arm/mach-zynq/headsmp.S          |  5 -----
 arch/arm/mach-zynq/platsmp.c          |  5 ++---

All ARMv5 and older CPUs invalidate their caches in the early assembly setup function, prior to enabling the MMU. This is because the L1 cache should not contain any data relevant to the execution of the kernel at this point; all data should have been flushed out to memory.

This requirement should also be true for ARMv6 and ARMv7 CPUs - indeed, these typically do not search their caches when caching is disabled (as it needs to be when the MMU is disabled) so this change should be safe.

ARMv7 allows there to be CPUs which search their caches while caching is disabled, and it's permitted that the cache is uninitialised at boot; for these, the architecture reference manual requires that an implementation specific code sequence is used immediately after reset to ensure that the cache is placed into a sane state. Such functionality is definitely outside the remit of the Linux kernel, and must be done by the SoC's firmware before _any_ CPU gets to the Linux kernel.

Changing the data cache clean+invalidate to a mere invalidate allows us to get rid of a lot of platform specific hacks around this issue for their secondary CPU bringup paths - some of which were buggy.

Signed-off-by Russell King <[email protected]>

---

 arch/arm/mach-hisi/Makefile           |  2 +-
 arch/arm/mach-hisi/core.h             |  1 -
 arch/arm/mach-hisi/headsmp.S          | 16 ----------------
 arch/arm/mach-hisi/platsmp.c          |  4 ++--

With commit 02b4e2756e01c623cc4dbceae4b07be75252db5b ("ARM: v7 setup function should invalidate L1 cache"), the default secondary_startup function for ARMv7 CPUs does invalidate the L1 cache, which was the sole reason why BCM63xx had to have its own secondary_startup implementation.

Now that the secondary_startup takes care of this, we can completely remove that code.

Signed-off-by Florian Fainelli <[email protected]>