diff mbox

[RFC] ARM: Make CPU_DCACHE_DISABLE depend on !SMP

Message ID 1420683088-1856-1-git-send-email-f.fainelli@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Florian Fainelli Jan. 8, 2015, 2:11 a.m. UTC
Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
kernel from booting because of the following ldrex instruction in
arch_spin_lock:

(gdb) x/10i $pc
=> 0xc053cfa8 <_raw_spin_lock+4>:       ldrex   r3, [r0]
   0xc053cfac <_raw_spin_lock+8>:       add     r2, r3, #65536  ; 0x10000

which is taken by the very first printk call:

    at /home/fainelli/work/linux/arch/arm/include/asm/spinlock.h:65
    fmt=0xc0637650 "\001\066Booting Linux on physical CPU 0x%x\n", args=<incomplete type>)
    at kernel/printk/printk.c:1525
    fmt=0xc05370f4 <printk+52> "\024\320\215\342\004\340\235\344\020\320\215\342\036\377/\341\017") at kernel/printk/printk.c:1688

ldrex requires exclusive monitor(s) (local or global) which are no longer
working when the Data cache is disabled in CP15 and will just hang the CPU
there.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/mm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Arnd Bergmann Jan. 8, 2015, 8:33 a.m. UTC | #1
On Wednesday 07 January 2015 18:11:28 Florian Fainelli wrote:
> Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
> kernel from booting because of the following ldrex instruction in
> arch_spin_lock:
> 
> (gdb) x/10i $pc
> => 0xc053cfa8 <_raw_spin_lock+4>:       ldrex   r3, [r0]
>    0xc053cfac <_raw_spin_lock+8>:       add     r2, r3, #65536  ; 0x10000
> 
> which is taken by the very first printk call:
> 
>     at /home/fainelli/work/linux/arch/arm/include/asm/spinlock.h:65
>     fmt=0xc0637650 "\001\066Booting Linux on physical CPU 0x%x\n", args=<incomplete type>)
>     at kernel/printk/printk.c:1525
>     fmt=0xc05370f4 <printk+52> "\024\320\215\342\004\340\235\344\020\320\215\342\036\377/\341\017") at kernel/printk/printk.c:1688
> 
> ldrex requires exclusive monitor(s) (local or global) which are no longer
> working when the Data cache is disabled in CP15 and will just hang the CPU
> there.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

We stumbled on this a while back when Kaixu was trying to fix allmodconfig
builds to run on real hardware, but never submitted it in the end when
Russell didn't like some of the other parts required for that to work.

This one clearly makes sense independently.

> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 03823e784f63..09245fd1e900 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -738,7 +738,7 @@ config CPU_ICACHE_DISABLE
>  
>  config CPU_DCACHE_DISABLE
>  	bool "Disable D-Cache (C-bit)"
> -	depends on CPU_CP15
> +	depends on CPU_CP15 && !SMP
>  	help
>  	  Say Y here to disable the processor data cache. Unless
>  	  you have a reason not to or are unsure, say N.
>
Russell King - ARM Linux Jan. 8, 2015, 11:52 a.m. UTC | #2
On Thu, Jan 08, 2015 at 09:33:05AM +0100, Arnd Bergmann wrote:
> On Wednesday 07 January 2015 18:11:28 Florian Fainelli wrote:
> > Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
> > kernel from booting because of the following ldrex instruction in
> > arch_spin_lock:
> > 
> > (gdb) x/10i $pc
> > => 0xc053cfa8 <_raw_spin_lock+4>:       ldrex   r3, [r0]
> >    0xc053cfac <_raw_spin_lock+8>:       add     r2, r3, #65536  ; 0x10000
> > 
> > which is taken by the very first printk call:
> > 
> >     at /home/fainelli/work/linux/arch/arm/include/asm/spinlock.h:65
> >     fmt=0xc0637650 "\001\066Booting Linux on physical CPU 0x%x\n", args=<incomplete type>)
> >     at kernel/printk/printk.c:1525
> >     fmt=0xc05370f4 <printk+52> "\024\320\215\342\004\340\235\344\020\320\215\342\036\377/\341\017") at kernel/printk/printk.c:1688
> > 
> > ldrex requires exclusive monitor(s) (local or global) which are no longer
> > working when the Data cache is disabled in CP15 and will just hang the CPU
> > there.
> > 
> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> We stumbled on this a while back when Kaixu was trying to fix allmodconfig
> builds to run on real hardware, but never submitted it in the end when
> Russell didn't like some of the other parts required for that to work.
> 
> This one clearly makes sense independently.

What about platforms where exclusives to strongly ordered memory do work?

I seem to remember patches from Tony for the pstore/ram_core driver to
allow it to map stuff strongly ordered, and use exclusives on it.
Florian Fainelli Jan. 8, 2015, 3:53 p.m. UTC | #3
Le 08/01/2015 03:52, Russell King - ARM Linux a écrit :
> On Thu, Jan 08, 2015 at 09:33:05AM +0100, Arnd Bergmann wrote:
>> On Wednesday 07 January 2015 18:11:28 Florian Fainelli wrote:
>>> Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
>>> kernel from booting because of the following ldrex instruction in
>>> arch_spin_lock:
>>>
>>> (gdb) x/10i $pc
>>> => 0xc053cfa8 <_raw_spin_lock+4>:       ldrex   r3, [r0]
>>>    0xc053cfac <_raw_spin_lock+8>:       add     r2, r3, #65536  ; 0x10000
>>>
>>> which is taken by the very first printk call:
>>>
>>>     at /home/fainelli/work/linux/arch/arm/include/asm/spinlock.h:65
>>>     fmt=0xc0637650 "\001\066Booting Linux on physical CPU 0x%x\n", args=<incomplete type>)
>>>     at kernel/printk/printk.c:1525
>>>     fmt=0xc05370f4 <printk+52> "\024\320\215\342\004\340\235\344\020\320\215\342\036\377/\341\017") at kernel/printk/printk.c:1688
>>>
>>> ldrex requires exclusive monitor(s) (local or global) which are no longer
>>> working when the Data cache is disabled in CP15 and will just hang the CPU
>>> there.
>>>
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>
>> We stumbled on this a while back when Kaixu was trying to fix allmodconfig
>> builds to run on real hardware, but never submitted it in the end when
>> Russell didn't like some of the other parts required for that to work.
>>
>> This one clearly makes sense independently.
> 
> What about platforms where exclusives to strongly ordered memory do work?

Do we have a comprehensive list of these platforms?

> 
> I seem to remember patches from Tony for the pstore/ram_core driver to
> allow it to map stuff strongly ordered, and use exclusives on it.

Would you have links to these patches handy?
--
Florian
Gregory Fong Jan. 8, 2015, 9:54 p.m. UTC | #4
On Thu, Jan 8, 2015 at 7:53 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Le 08/01/2015 03:52, Russell King - ARM Linux a écrit :
>> On Thu, Jan 08, 2015 at 09:33:05AM +0100, Arnd Bergmann wrote:
>>> On Wednesday 07 January 2015 18:11:28 Florian Fainelli wrote:
>>>> Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
>>>> kernel from booting because of the following ldrex instruction in
>>>> arch_spin_lock:
>>>>
>>>> (gdb) x/10i $pc
>>>> => 0xc053cfa8 <_raw_spin_lock+4>:       ldrex   r3, [r0]
>>>>    0xc053cfac <_raw_spin_lock+8>:       add     r2, r3, #65536  ; 0x10000
>>>>
>>>> which is taken by the very first printk call:
>>>>
>>>>     at /home/fainelli/work/linux/arch/arm/include/asm/spinlock.h:65
>>>>     fmt=0xc0637650 "\001\066Booting Linux on physical CPU 0x%x\n", args=<incomplete type>)
>>>>     at kernel/printk/printk.c:1525
>>>>     fmt=0xc05370f4 <printk+52> "\024\320\215\342\004\340\235\344\020\320\215\342\036\377/\341\017") at kernel/printk/printk.c:1688
>>>>
>>>> ldrex requires exclusive monitor(s) (local or global) which are no longer
>>>> working when the Data cache is disabled in CP15 and will just hang the CPU
>>>> there.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>
>>> We stumbled on this a while back when Kaixu was trying to fix allmodconfig
>>> builds to run on real hardware, but never submitted it in the end when
>>> Russell didn't like some of the other parts required for that to work.
>>>
>>> This one clearly makes sense independently.
>>
>> What about platforms where exclusives to strongly ordered memory do work?
>
> Do we have a comprehensive list of these platforms?

I haven't had a chance to go through all of the TRMs, but section
A3.4.5 of the ARMv7-A ARM says:

"It is IMPLEMENTATION DEFINED whether LDREX and STREX operations can
be performed to a memory region
with the Device or Strongly-ordered memory attribute. Unless the
implementation documentation explicitly
states that LDREX and STREX operations to a memory region with the
Device or Strongly-ordered attribute are
permitted, the effect of such operations is UNPREDICTABLE ."

From 6.4.5 "Synchronization primitives" in the Cortex-A15 TRM:

"Use of synchronization primitives on addresses in regions marked as
Strongly-ordered or
Device is UNPREDICTABLE in the ARMv7-A Architecture. Code that makes
such accesses
is not portable."

It looks like this is not generally permissible for ARMv7, maybe
someone else can check the other versions.  Perhaps it would make
sense to disallow disabling the D-cache for !SMP in general and
special-case those architectures which do support exclusives to
strongly ordered memory.

Best regards,
Gregory
Arnd Bergmann Jan. 8, 2015, 10:10 p.m. UTC | #5
On Thursday 08 January 2015 13:54:29 Gregory Fong wrote:
> On Thu, Jan 8, 2015 at 7:53 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > Le 08/01/2015 03:52, Russell King - ARM Linux a écrit :
> >> On Thu, Jan 08, 2015 at 09:33:05AM +0100, Arnd Bergmann wrote:
> >>>
> >>> We stumbled on this a while back when Kaixu was trying to fix allmodconfig
> >>> builds to run on real hardware, but never submitted it in the end when
> >>> Russell didn't like some of the other parts required for that to work.
> >>>
> >>> This one clearly makes sense independently.
> >>
> >> What about platforms where exclusives to strongly ordered memory do work?
> >
> > Do we have a comprehensive list of these platforms?
> 
> I haven't had a chance to go through all of the TRMs, but section
> A3.4.5 of the ARMv7-A ARM says:
> 
> "It is IMPLEMENTATION DEFINED whether LDREX and STREX operations can
> be performed to a memory region
> with the Device or Strongly-ordered memory attribute. Unless the
> implementation documentation explicitly
> states that LDREX and STREX operations to a memory region with the
> Device or Strongly-ordered attribute are
> permitted, the effect of such operations is UNPREDICTABLE ."
> 
> From 6.4.5 "Synchronization primitives" in the Cortex-A15 TRM:
> 
> "Use of synchronization primitives on addresses in regions marked as
> Strongly-ordered or
> Device is UNPREDICTABLE in the ARMv7-A Architecture. Code that makes
> such accesses
> is not portable."
> 
> It looks like this is not generally permissible for ARMv7, maybe
> someone else can check the other versions.  Perhaps it would make
> sense to disallow disabling the D-cache for !SMP in general and
> special-case those architectures which do support exclusives to
> strongly ordered memory.

Too many negations, I assume you meant on SMP rather than !SMP.

My guess is that any implementation that has working LDREX/STREX
on uncached memory does not support SMP, because these instructions
tend to be implemented through the cache coherency logic on SMP
systems but could be implemented more easily without a dcache
on uniprocessor cores.

	Arnd
Gregory Fong Jan. 8, 2015, 10:11 p.m. UTC | #6
On Thu, Jan 8, 2015 at 1:54 PM, Gregory Fong <gregory.0xf0@gmail.com> wrote:
> On Thu, Jan 8, 2015 at 7:53 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> Le 08/01/2015 03:52, Russell King - ARM Linux a écrit :
>>> On Thu, Jan 08, 2015 at 09:33:05AM +0100, Arnd Bergmann wrote:
>>>> On Wednesday 07 January 2015 18:11:28 Florian Fainelli wrote:
>>>>> Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
>>>>> kernel from booting because of the following ldrex instruction in
>>>>> arch_spin_lock:
>>>>>
>>>>> (gdb) x/10i $pc
>>>>> => 0xc053cfa8 <_raw_spin_lock+4>:       ldrex   r3, [r0]
>>>>>    0xc053cfac <_raw_spin_lock+8>:       add     r2, r3, #65536  ; 0x10000
>>>>>
>>>>> which is taken by the very first printk call:
>>>>>
>>>>>     at /home/fainelli/work/linux/arch/arm/include/asm/spinlock.h:65
>>>>>     fmt=0xc0637650 "\001\066Booting Linux on physical CPU 0x%x\n", args=<incomplete type>)
>>>>>     at kernel/printk/printk.c:1525
>>>>>     fmt=0xc05370f4 <printk+52> "\024\320\215\342\004\340\235\344\020\320\215\342\036\377/\341\017") at kernel/printk/printk.c:1688
>>>>>
>>>>> ldrex requires exclusive monitor(s) (local or global) which are no longer
>>>>> working when the Data cache is disabled in CP15 and will just hang the CPU
>>>>> there.
>>>>>
>>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>
>>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>>
>>>> We stumbled on this a while back when Kaixu was trying to fix allmodconfig
>>>> builds to run on real hardware, but never submitted it in the end when
>>>> Russell didn't like some of the other parts required for that to work.
>>>>
>>>> This one clearly makes sense independently.
>>>
>>> What about platforms where exclusives to strongly ordered memory do work?
>>
>> Do we have a comprehensive list of these platforms?
>
> I haven't had a chance to go through all of the TRMs, but section
> A3.4.5 of the ARMv7-A ARM says:
>
> "It is IMPLEMENTATION DEFINED whether LDREX and STREX operations can
> be performed to a memory region
> with the Device or Strongly-ordered memory attribute. Unless the
> implementation documentation explicitly
> states that LDREX and STREX operations to a memory region with the
> Device or Strongly-ordered attribute are
> permitted, the effect of such operations is UNPREDICTABLE ."
>
> From 6.4.5 "Synchronization primitives" in the Cortex-A15 TRM:
>
> "Use of synchronization primitives on addresses in regions marked as
> Strongly-ordered or
> Device is UNPREDICTABLE in the ARMv7-A Architecture. Code that makes
> such accesses
> is not portable."
>
> It looks like this is not generally permissible for ARMv7, maybe
> someone else can check the other versions.  Perhaps it would make
> sense to disallow disabling the D-cache for !SMP in general and
> special-case those architectures which do support exclusives to
> strongly ordered memory.

Er, I meant "disallow disabling the D-cache for SMP", sorry.  Too many
negatives.
Russell King - ARM Linux Jan. 8, 2015, 10:13 p.m. UTC | #7
On Thu, Jan 08, 2015 at 11:10:47PM +0100, Arnd Bergmann wrote:
> My guess is that any implementation that has working LDREX/STREX
> on uncached memory does not support SMP, because these instructions
> tend to be implemented through the cache coherency logic on SMP
> systems but could be implemented more easily without a dcache
> on uniprocessor cores.

Yes, I negated Tony's patches... Tony was fixing pstore to use
writecombine memory.

So this change is fine.
Florian Fainelli Jan. 8, 2015, 10:25 p.m. UTC | #8
On 08/01/15 14:13, Russell King - ARM Linux wrote:
> On Thu, Jan 08, 2015 at 11:10:47PM +0100, Arnd Bergmann wrote:
>> My guess is that any implementation that has working LDREX/STREX
>> on uncached memory does not support SMP, because these instructions
>> tend to be implemented through the cache coherency logic on SMP
>> systems but could be implemented more easily without a dcache
>> on uniprocessor cores.
> 
> Yes, I negated Tony's patches... Tony was fixing pstore to use
> writecombine memory.
> 
> So this change is fine.

Do you want me to submit the patch as-is to your patch tracking, or
would you want to include Gregory's quotes for completeness?
--
Florian
Russell King - ARM Linux Jan. 9, 2015, 4:16 p.m. UTC | #9
On Thu, Jan 08, 2015 at 02:25:57PM -0800, Florian Fainelli wrote:
> On 08/01/15 14:13, Russell King - ARM Linux wrote:
> > On Thu, Jan 08, 2015 at 11:10:47PM +0100, Arnd Bergmann wrote:
> >> My guess is that any implementation that has working LDREX/STREX
> >> on uncached memory does not support SMP, because these instructions
> >> tend to be implemented through the cache coherency logic on SMP
> >> systems but could be implemented more easily without a dcache
> >> on uniprocessor cores.
> > 
> > Yes, I negated Tony's patches... Tony was fixing pstore to use
> > writecombine memory.
> > 
> > So this change is fine.
> 
> Do you want me to submit the patch as-is to your patch tracking, or
> would you want to include Gregory's quotes for completeness?

I think as-is.

Thanks.
Rabin Vincent Jan. 9, 2015, 5:57 p.m. UTC | #10
On Wed, Jan 07, 2015 at 06:11:28PM -0800, Florian Fainelli wrote:
> Enabling CPU_DCACHE_DISABLE on a SMP capable system will prevent the
> kernel from booting because of the following ldrex instruction in
> arch_spin_lock:

BTW, it's not just SMP spin_lock which uses ldrex/strex.  The atomic
ops, set_bit() and friends, and cmpxchg() and related functions all use
ldrex/strex on ARMv6+ processors, even if you disable SMP.
Florian Fainelli Jan. 9, 2015, 6:35 p.m. UTC | #11
On 09/01/15 08:16, Russell King - ARM Linux wrote:
> On Thu, Jan 08, 2015 at 02:25:57PM -0800, Florian Fainelli wrote:
>> On 08/01/15 14:13, Russell King - ARM Linux wrote:
>>> On Thu, Jan 08, 2015 at 11:10:47PM +0100, Arnd Bergmann wrote:
>>>> My guess is that any implementation that has working LDREX/STREX
>>>> on uncached memory does not support SMP, because these instructions
>>>> tend to be implemented through the cache coherency logic on SMP
>>>> systems but could be implemented more easily without a dcache
>>>> on uniprocessor cores.
>>>
>>> Yes, I negated Tony's patches... Tony was fixing pstore to use
>>> writecombine memory.
>>>
>>> So this change is fine.
>>
>> Do you want me to submit the patch as-is to your patch tracking, or
>> would you want to include Gregory's quotes for completeness?
> 
> I think as-is.

Thanks, submitted as 8276/1.
diff mbox

Patch

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 03823e784f63..09245fd1e900 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -738,7 +738,7 @@  config CPU_ICACHE_DISABLE
 
 config CPU_DCACHE_DISABLE
 	bool "Disable D-Cache (C-bit)"
-	depends on CPU_CP15
+	depends on CPU_CP15 && !SMP
 	help
 	  Say Y here to disable the processor data cache. Unless
 	  you have a reason not to or are unsure, say N.