parallel load of modules on an ARM multicore

parallel load of modules on an ARM multicore

[EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31)]

Hi,

I'm getting unexpected results from loading several modules - some
of them in parallel - on an ARM11 MPcore system.

The system does not use "single sequential" modprobe commands - instead it
starts several shells doing insmod sequences like this:

shell A:
insmod a
insmod ab
insmod abc

shell B:
insmod b
insmod bc
insmod bcd

shell C:
insmod c
insmod cd
insmod cde

This is done to crash^H^H^H^H^Hboot faster ;)

While one insmod operation is protected via the module_mutex - I'm wondering
what happens with the instruction cache invalidation.
AFAICT the flush_icache_range only invalidates the ICache on the running cpu.
The module_mutex is unlocked after _loading_ the module, do_mod_ctors() and
do_one_initcall() are called without that lock - can they run on a different cpu?
It's an preemptible system (SMP PREEMPT armv6l).

Wouldn't it be required to flush the icache on _all_ cpus?
It might be highly dependable from the cache organization, but isn't it possible
that the cpu that didn't load the module, is scheduled to run a function from it
with a "stale" instruction cache line???

I sometimes see oopses where I wonder how such a state could be reached.
The code and register state seem not to fit - looks like an interrupt does not
restore all registers properly :)

Also an Oops happens when loading module abc, and still the module list is
reported empty in the Oops despite the fact that "abc" depends on "a". Hmh.


Best regards

        Peter

<snip kernel/module.c:SYSCALL_DEFINE3(init_module>
[..]
        /* Only one module load at a time, please */
        if (mutex_lock_interruptible(&module_mutex) != 0)
                return -EINTR;

        /* Do all the hard work */
        mod = load_module(umod, len, uargs);
        if (IS_ERR(mod)) {
                mutex_unlock(&module_mutex);
                return PTR_ERR(mod);
        }

        /* Drop lock so they can recurse */
        mutex_unlock(&module_mutex);

        blocking_notifier_call_chain(&module_notify_list,
                        MODULE_STATE_COMING, mod);

        do_mod_ctors(mod);
        /* Start the module */
        if (mod->init != NULL)
                ret = do_one_initcall(mod->init);
</snip>

<snip kernel/module.c:load_module>
[..]
        old_fs = get_fs();
        set_fs(KERNEL_DS);

        /*
         * Flush the instruction cache, since we've played with text.
         * Do it before processing of module parameters, so the module
         * can provide parameter accessor functions of its own.
         */
        if (mod->module_init)
                flush_icache_range((unsigned long)mod->module_init,
                                   (unsigned long)mod->module_init
                                   + mod->init_size);
        flush_icache_range((unsigned long)mod->module_core,
                           (unsigned long)mod->module_core + mod->core_size);

        set_fs(old_fs);

</snip>

arch/arm/mm/cache-v6.S:

ENTRY(v6_flush_kern_cache_all)
        mov     r0, #0
#ifdef HARVARD_CACHE
        mcr     p15, 0, r0, c7, c14, 0          @ D cache clean+invalidate
#ifndef CONFIG_ARM_ERRATA_411920
        mcr     p15, 0, r0, c7, c5, 0           @ I+BTB cache invalidate
#else
        b       v6_icache_inval_all
#endif
#else
        mcr     p15, 0, r0, c7, c15, 0          @ Cache clean+invalidate
#endif
        mov     pc, lr

parallel load of modules on an ARM multicore

[Catalin Marinas]

On Mon, Jun 20, 2011 at 03:43:27PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> I'm getting unexpected results from loading several modules - some
> of them in parallel - on an ARM11 MPcore system.
> 
> The system does not use "single sequential" modprobe commands - instead it
> starts several shells doing insmod sequences like this:
> 
> shell A:
> insmod a
> insmod ab
> insmod abc
> 
> shell B:
> insmod b
> insmod bc
> insmod bcd
> 
> shell C:
> insmod c
> insmod cd
> insmod cde
> 
> This is done to crash^H^H^H^H^Hboot faster ;)
> 
> While one insmod operation is protected via the module_mutex - I'm wondering
> what happens with the instruction cache invalidation.
> AFAICT the flush_icache_range only invalidates the ICache on the running cpu.
> The module_mutex is unlocked after _loading_ the module, do_mod_ctors() and
> do_one_initcall() are called without that lock - can they run on a different cpu?
> It's an preemptible system (SMP PREEMPT armv6l).
> 
> Wouldn't it be required to flush the icache on _all_ cpus?

I theory, you would need to flush both the I and D cache on all the CPUs
but that's not easily possible (well, I don't think there are many users
of flush_icache_range(), so we could make this do an
smp_call_function().

I think what happens (I'm looking at a 3.0-rc3 kernel) is that the
module code is copied into RAM and the process could migrate to another
CPU before flush_icache_range() gets called (this function is called
outside the mutex_lock regions). We therefore don't flush the data out
of the D-cache that was allocated during copy_from_user().

You can try the patch below (I haven't tested it for some time), it may
fix the issue.

8<-----------------------------------------------

ARM: Allow lazy cache flushing on ARM11MPCore

From: Catalin Marinas <catalin.marinas@arm.com>

The ARM11MPCore doesn't broadcast the cache maintenance operations in
hardware, therefore the flush_dcache_page() currently performs the cache
flushing non-lazily. But since not all drivers call this function after
writing to a page cache page, the kernel needs a different approach like
using read-for-ownership on the CPU flushing the cache to force the
dirty cache lines migration from other CPUs. This way the cache flushing
operation can be done lazily via __sync_icache_dcache().

Since we cannot force read-for-ownership on the I-cache, the patch
invalidates the whole I-cache when a thread migrates to another CPU.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/include/asm/mmu_context.h |    3 ++-
 arch/arm/mm/cache-v6.S             |    8 ++++++++
 arch/arm/mm/flush.c                |    3 +--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h
index 71605d9..d116a23 100644
--- a/arch/arm/include/asm/mmu_context.h
+++ b/arch/arm/include/asm/mmu_context.h
@@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
 #ifdef CONFIG_SMP
 	/* check for possible thread migration */
 	if (!cpumask_empty(mm_cpumask(next)) &&
-	    !cpumask_test_cpu(cpu, mm_cpumask(next)))
+	    (cache_ops_need_broadcast() ||
+	     !cpumask_test_cpu(cpu, mm_cpumask(next))))
 		__flush_icache_all();
 #endif
 	if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) || prev != next) {
diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 73b4a8b..bdc1cc1 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -133,6 +133,10 @@ ENTRY(v6_coherent_user_range)
 #ifdef HARVARD_CACHE
 	bic	r0, r0, #CACHE_LINE_SIZE - 1
 1:
+#ifdef CONFIG_SMP
+	/* no cache maintenance broadcasting on ARM11MPCore */
+ USER(	ldr	r2, [r0]		)	@ read for ownership
+#endif
  USER(	mcr	p15, 0, r0, c7, c10, 1	)	@ clean D line
 	add	r0, r0, #CACHE_LINE_SIZE
 2:
@@ -178,6 +182,10 @@ ENTRY(v6_flush_kern_dcache_area)
 	add	r1, r0, r1
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
+#ifdef CONFIG_SMP
+	/* no cache maintenance broadcasting on ARM11MPCore */
+	ldr	r2, [r0]			@ read for ownership
+#endif
 #ifdef HARVARD_CACHE
 	mcr	p15, 0, r0, c7, c14, 1		@ clean & invalidate D line
 #else
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 1a8d4aa..72f9333 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -291,8 +291,7 @@ void flush_dcache_page(struct page *page)
 
 	mapping = page_mapping(page);
 
-	if (!cache_ops_need_broadcast() &&
-	    mapping && !mapping_mapped(mapping))
+	if (mapping && !mapping_mapped(mapping))
 		clear_bit(PG_dcache_clean, &page->flags);
 	else {
 		__flush_dcache_page(mapping, page);

-- 
Catalin

AW: parallel load of modules on an ARM multicore

[EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31)]

Catalin,

it's interesting that you almost agree with me.

But isn't it really expensive to flush the icache on switch_mm?
Is that meant as a test to see if the problem goes away?

Wouldn't it suffice to get_cpu/put_cpu to disable preemption while
load_module() works? A running task in kernel mode will not be
migrated, wouldn't it?


I think the other way will cause trouble: the module is loaded on
cpu0 for example, preempted, woken up on cpu1 with a stale icache
line not holding the "newly loaded code" and running mod->init peng!
Nobody told cpu1 to revalidate it's icache.
Don't know if this is possible though.

The data of the module won't get through the icache anyway.


AFAIK we are not able to reproduce quickly and it will take some
time before I can try...

Mit freundlichen Gr??en / Best regards

Peter W?chtler


> -----Urspr?ngliche Nachricht-----
> Von: Catalin Marinas [mailto:catalin.marinas at gmail.com] Im
> Auftrag von Catalin Marinas
> Gesendet: Dienstag, 21. Juni 2011 17:51
> An: EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31)
> Cc: linux-arm-kernel at lists.infradead.org
> Betreff: Re: parallel load of modules on an ARM multicore
>
> On Mon, Jun 20, 2011 at 03:43:27PM +0200, EXTERNAL Waechtler
> Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> > I'm getting unexpected results from loading several modules - some
> > of them in parallel - on an ARM11 MPcore system.
> >
> > The system does not use "single sequential" modprobe
> commands - instead it
> > starts several shells doing insmod sequences like this:
> >
> > shell A:
> > insmod a
> > insmod ab
> > insmod abc
> >
> > shell B:
> > insmod b
> > insmod bc
> > insmod bcd
> >
> > shell C:
> > insmod c
> > insmod cd
> > insmod cde
> >
> > This is done to crash^H^H^H^H^Hboot faster ;)
> >
> > While one insmod operation is protected via the
> module_mutex - I'm wondering
> > what happens with the instruction cache invalidation.
> > AFAICT the flush_icache_range only invalidates the ICache
> on the running cpu.
> > The module_mutex is unlocked after _loading_ the module,
> do_mod_ctors() and
> > do_one_initcall() are called without that lock - can they
> run on a different cpu?
> > It's an preemptible system (SMP PREEMPT armv6l).
> >
> > Wouldn't it be required to flush the icache on _all_ cpus?
>
> I theory, you would need to flush both the I and D cache on
> all the CPUs
> but that's not easily possible (well, I don't think there are
> many users
> of flush_icache_range(), so we could make this do an
> smp_call_function().
>
> I think what happens (I'm looking at a 3.0-rc3 kernel) is that the
> module code is copied into RAM and the process could migrate
> to another
> CPU before flush_icache_range() gets called (this function is called
> outside the mutex_lock regions). We therefore don't flush the data out
> of the D-cache that was allocated during copy_from_user().
>
> You can try the patch below (I haven't tested it for some
> time), it may
> fix the issue.
>
> 8<-----------------------------------------------
>
> ARM: Allow lazy cache flushing on ARM11MPCore
>
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> The ARM11MPCore doesn't broadcast the cache maintenance operations in
> hardware, therefore the flush_dcache_page() currently
> performs the cache
> flushing non-lazily. But since not all drivers call this
> function after
> writing to a page cache page, the kernel needs a different
> approach like
> using read-for-ownership on the CPU flushing the cache to force the
> dirty cache lines migration from other CPUs. This way the
> cache flushing
> operation can be done lazily via __sync_icache_dcache().
>
> Since we cannot force read-for-ownership on the I-cache, the patch
> invalidates the whole I-cache when a thread migrates to another CPU.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/include/asm/mmu_context.h |    3 ++-
>  arch/arm/mm/cache-v6.S             |    8 ++++++++
>  arch/arm/mm/flush.c                |    3 +--
>  3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/mmu_context.h
> b/arch/arm/include/asm/mmu_context.h
> index 71605d9..d116a23 100644
> --- a/arch/arm/include/asm/mmu_context.h
> +++ b/arch/arm/include/asm/mmu_context.h
> @@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct
> mm_struct *next,
>  #ifdef CONFIG_SMP
>       /* check for possible thread migration */
>       if (!cpumask_empty(mm_cpumask(next)) &&
> -         !cpumask_test_cpu(cpu, mm_cpumask(next)))
> +         (cache_ops_need_broadcast() ||
> +          !cpumask_test_cpu(cpu, mm_cpumask(next))))
>               __flush_icache_all();
>  #endif
>       if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) ||
> prev != next) {
> diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
> index 73b4a8b..bdc1cc1 100644
> --- a/arch/arm/mm/cache-v6.S
> +++ b/arch/arm/mm/cache-v6.S
> @@ -133,6 +133,10 @@ ENTRY(v6_coherent_user_range)
>  #ifdef HARVARD_CACHE
>       bic     r0, r0, #CACHE_LINE_SIZE - 1
>  1:
> +#ifdef CONFIG_SMP
> +     /* no cache maintenance broadcasting on ARM11MPCore */
> + USER(       ldr     r2, [r0]                )       @ read
> for ownership
> +#endif
>   USER(       mcr     p15, 0, r0, c7, c10, 1  )       @ clean D line
>       add     r0, r0, #CACHE_LINE_SIZE
>  2:
> @@ -178,6 +182,10 @@ ENTRY(v6_flush_kern_dcache_area)
>       add     r1, r0, r1
>       bic     r0, r0, #D_CACHE_LINE_SIZE - 1
>  1:
> +#ifdef CONFIG_SMP
> +     /* no cache maintenance broadcasting on ARM11MPCore */
> +     ldr     r2, [r0]                        @ read for ownership
> +#endif
>  #ifdef HARVARD_CACHE
>       mcr     p15, 0, r0, c7, c14, 1          @ clean &
> invalidate D line
>  #else
> diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> index 1a8d4aa..72f9333 100644
> --- a/arch/arm/mm/flush.c
> +++ b/arch/arm/mm/flush.c
> @@ -291,8 +291,7 @@ void flush_dcache_page(struct page *page)
>
>       mapping = page_mapping(page);
>
> -     if (!cache_ops_need_broadcast() &&
> -         mapping && !mapping_mapped(mapping))
> +     if (mapping && !mapping_mapped(mapping))
>               clear_bit(PG_dcache_clean, &page->flags);
>       else {
>               __flush_dcache_page(mapping, page);
>
> --
> Catalin
>

parallel load of modules on an ARM multicore

[Catalin Marinas]

Peter,

On Thu, Jun 23, 2011 at 04:39:01PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> it's interesting that you almost agree with me.
> 
> But isn't it really expensive to flush the icache on switch_mm?
> Is that meant as a test to see if the problem goes away?

Who said anything about flushing the icache on switch_mm()? My patch
doesn't do this, it actually reduces the amount of cache flushing on
ARM11MPCore.

> Wouldn't it suffice to get_cpu/put_cpu to disable preemption while
> load_module() works?

It may work, just give it a try.

> I think the other way will cause trouble: the module is loaded on
> cpu0 for example, preempted, woken up on cpu1 with a stale icache
> line not holding the "newly loaded code" and running mod->init peng!
> Nobody told cpu1 to revalidate it's icache.
> Don't know if this is possible though.

That's possible as well if the pages allocated for the module code have
been previously used for other code.

To resolve the stale I-cache lines, you would have to broadcast the
cache maintenance to all the CPUs. For the D-cache we could trick the
CPU via some reading to force the dirty cache lines migration but that's
not possible for the I-cache.

> The data of the module won't get through the icache anyway.

No but the module is copied into the new allocated space via the
D-cache. This needs to be flushed so that the I-cache would get the
right instructions.

> AFAIK we are not able to reproduce quickly and it will take some
> time before I can try...

OK, just let us know how it goes.

-- 
Catalin

parallel load of modules on an ARM multicore

[Russell King - ARM Linux]

On Thu, Jun 23, 2011 at 03:52:29PM +0100, Catalin Marinas wrote:
> Peter,
> 
> On Thu, Jun 23, 2011 at 04:39:01PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> > it's interesting that you almost agree with me.
> > 
> > But isn't it really expensive to flush the icache on switch_mm?
> > Is that meant as a test to see if the problem goes away?
> 
> Who said anything about flushing the icache on switch_mm()? My patch
> doesn't do this, it actually reduces the amount of cache flushing on
> ARM11MPCore.

Ahem, your patch does:

--- a/arch/arm/include/asm/mmu_context.h
+++ b/arch/arm/include/asm/mmu_context.h
@@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
 #ifdef CONFIG_SMP
        /* check for possible thread migration */
        if (!cpumask_empty(mm_cpumask(next)) &&
-           !cpumask_test_cpu(cpu, mm_cpumask(next)))
+           (cache_ops_need_broadcast() ||
+            !cpumask_test_cpu(cpu, mm_cpumask(next))))
                __flush_icache_all();
 #endif
        if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) || prev != next) {

This means that if cache_ops_need_broadcast() is true, we will flush
the I-cache at every MM context switch as very few MMs will have an
empty mm_cpumask().

Far from reducing the number of I-cache flushes, this will significantly
increase the flushing.

AW: parallel load of modules on an ARM multicore

[EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31)]

No, you didn't say that, but the patch ;)

--- a/arch/arm/include/asm/mmu_context.h
+++ b/arch/arm/include/asm/mmu_context.h
@@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
 #ifdef CONFIG_SMP
        /* check for possible thread migration */
        if (!cpumask_empty(mm_cpumask(next)) &&
-           !cpumask_test_cpu(cpu, mm_cpumask(next)))
+           (cache_ops_need_broadcast() ||
+            !cpumask_test_cpu(cpu, mm_cpumask(next))))
                __flush_icache_all();


On ARM11MPcore cache_ops_need_broadcast() returns true -
with that, anytime the thread migrates the icache is flushed?


Mit freundlichen Gr??en / Best regards

Peter W?chtler

> -----Urspr?ngliche Nachricht-----
> Von: Catalin Marinas [mailto:catalin.marinas at gmail.com] Im
> Auftrag von Catalin Marinas
> Gesendet: Donnerstag, 23. Juni 2011 16:52
> An: EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31)
> Cc: linux-arm-kernel at lists.infradead.org
> Betreff: Re: parallel load of modules on an ARM multicore
>
> Peter,
>
> On Thu, Jun 23, 2011 at 04:39:01PM +0200, EXTERNAL Waechtler
> Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> > it's interesting that you almost agree with me.
> >
> > But isn't it really expensive to flush the icache on switch_mm?
> > Is that meant as a test to see if the problem goes away?
>
> Who said anything about flushing the icache on switch_mm()? My patch
> doesn't do this, it actually reduces the amount of cache flushing on
> ARM11MPCore.
>
> > Wouldn't it suffice to get_cpu/put_cpu to disable preemption while
> > load_module() works?
>
> It may work, just give it a try.
>
> > I think the other way will cause trouble: the module is loaded on
> > cpu0 for example, preempted, woken up on cpu1 with a stale icache
> > line not holding the "newly loaded code" and running mod->init peng!
> > Nobody told cpu1 to revalidate it's icache.
> > Don't know if this is possible though.
>
> That's possible as well if the pages allocated for the module
> code have
> been previously used for other code.
>
> To resolve the stale I-cache lines, you would have to broadcast the
> cache maintenance to all the CPUs. For the D-cache we could trick the
> CPU via some reading to force the dirty cache lines migration
> but that's
> not possible for the I-cache.
>
> > The data of the module won't get through the icache anyway.
>
> No but the module is copied into the new allocated space via the
> D-cache. This needs to be flushed so that the I-cache would get the
> right instructions.
>
> > AFAIK we are not able to reproduce quickly and it will take some
> > time before I can try...
>
> OK, just let us know how it goes.
>
> --
> Catalin
>

parallel load of modules on an ARM multicore

[Catalin Marinas]

On Thu, Jun 23, 2011 at 04:12:49PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 23, 2011 at 03:52:29PM +0100, Catalin Marinas wrote:
> > Peter,
> > 
> > On Thu, Jun 23, 2011 at 04:39:01PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> > > it's interesting that you almost agree with me.
> > > 
> > > But isn't it really expensive to flush the icache on switch_mm?
> > > Is that meant as a test to see if the problem goes away?
> > 
> > Who said anything about flushing the icache on switch_mm()? My patch
> > doesn't do this, it actually reduces the amount of cache flushing on
> > ARM11MPCore.
> 
> Ahem, your patch does:
> 
> --- a/arch/arm/include/asm/mmu_context.h
> +++ b/arch/arm/include/asm/mmu_context.h
> @@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  #ifdef CONFIG_SMP
>         /* check for possible thread migration */
>         if (!cpumask_empty(mm_cpumask(next)) &&
> -           !cpumask_test_cpu(cpu, mm_cpumask(next)))
> +           (cache_ops_need_broadcast() ||
> +            !cpumask_test_cpu(cpu, mm_cpumask(next))))
>                 __flush_icache_all();
>  #endif
>         if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) || prev != next) {
> 
> This means that if cache_ops_need_broadcast() is true, we will flush
> the I-cache at every MM context switch as very few MMs will have an
> empty mm_cpumask().

Ah, I forgot about this. That's left over from some earlier version of
the patch where cache_ops_need_broadcast() was set to 0 for ARMv6 SMP
but then I realised that it is still needed for some ptrace stuff and
re-instated it.

So this hunk is definitely not needed.

BTW, the patch (with the fix above) would be useful for the cases where
we have block drivers not calling flush_dcache_page().

-- 
Catalin

parallel load of modules on an ARM multicore

[Catalin Marinas]

On Thu, Jun 23, 2011 at 04:34:24PM +0100, Catalin Marinas wrote:
> On Thu, Jun 23, 2011 at 04:12:49PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Jun 23, 2011 at 03:52:29PM +0100, Catalin Marinas wrote:
> > > Peter,
> > > 
> > > On Thu, Jun 23, 2011 at 04:39:01PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> > > > it's interesting that you almost agree with me.
> > > > 
> > > > But isn't it really expensive to flush the icache on switch_mm?
> > > > Is that meant as a test to see if the problem goes away?
> > > 
> > > Who said anything about flushing the icache on switch_mm()? My patch
> > > doesn't do this, it actually reduces the amount of cache flushing on
> > > ARM11MPCore.
> > 
> > Ahem, your patch does:
> > 
> > --- a/arch/arm/include/asm/mmu_context.h
> > +++ b/arch/arm/include/asm/mmu_context.h
> > @@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
> >  #ifdef CONFIG_SMP
> >         /* check for possible thread migration */
> >         if (!cpumask_empty(mm_cpumask(next)) &&
> > -           !cpumask_test_cpu(cpu, mm_cpumask(next)))
> > +           (cache_ops_need_broadcast() ||
> > +            !cpumask_test_cpu(cpu, mm_cpumask(next))))
> >                 __flush_icache_all();
> >  #endif
> >         if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) || prev != next) {
> > 
> > This means that if cache_ops_need_broadcast() is true, we will flush
> > the I-cache at every MM context switch as very few MMs will have an
> > empty mm_cpumask().
> 
> Ah, I forgot about this. That's left over from some earlier version of
> the patch where cache_ops_need_broadcast() was set to 0 for ARMv6 SMP
> but then I realised that it is still needed for some ptrace stuff and
> re-instated it.
> 
> So this hunk is definitely not needed.

BTW, do we actually need the I-cache invalidation during thread
migration with cache ops broadcasting in hardware?

-- 
Catalin

parallel load of modules on an ARM multicore

[George G. Davis]

Hi,

On Tue, Jun 21, 2011 at 04:50:30PM +0100, Catalin Marinas wrote:

// CUT

> ARM: Allow lazy cache flushing on ARM11MPCore
> 
> From: Catalin Marinas <catalin.marinas@arm.com>
> 
> The ARM11MPCore doesn't broadcast the cache maintenance operations in
> hardware, therefore the flush_dcache_page() currently performs the cache
> flushing non-lazily. But since not all drivers call this function after
> writing to a page cache page, the kernel needs a different approach like
> using read-for-ownership on the CPU flushing the cache to force the
> dirty cache lines migration from other CPUs. This way the cache flushing
> operation can be done lazily via __sync_icache_dcache().
> 
> Since we cannot force read-for-ownership on the I-cache, the patch
> invalidates the whole I-cache when a thread migrates to another CPU.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/include/asm/mmu_context.h |    3 ++-
>  arch/arm/mm/cache-v6.S             |    8 ++++++++
>  arch/arm/mm/flush.c                |    3 +--
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h
> index 71605d9..d116a23 100644
> --- a/arch/arm/include/asm/mmu_context.h
> +++ b/arch/arm/include/asm/mmu_context.h
> @@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  #ifdef CONFIG_SMP
>  	/* check for possible thread migration */
>  	if (!cpumask_empty(mm_cpumask(next)) &&
> -	    !cpumask_test_cpu(cpu, mm_cpumask(next)))
> +	    (cache_ops_need_broadcast() ||
> +	     !cpumask_test_cpu(cpu, mm_cpumask(next))))
>  		__flush_icache_all();
>  #endif
>  	if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) || prev != next) {
> diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
> index 73b4a8b..bdc1cc1 100644
> --- a/arch/arm/mm/cache-v6.S
> +++ b/arch/arm/mm/cache-v6.S
> @@ -133,6 +133,10 @@ ENTRY(v6_coherent_user_range)
>  #ifdef HARVARD_CACHE
>  	bic	r0, r0, #CACHE_LINE_SIZE - 1
>  1:
> +#ifdef CONFIG_SMP

s/CONFIG_SMP/CONFIG_RWFO/

> +	/* no cache maintenance broadcasting on ARM11MPCore */
> + USER(	ldr	r2, [r0]		)	@ read for ownership
> +#endif
>   USER(	mcr	p15, 0, r0, c7, c10, 1	)	@ clean D line
>  	add	r0, r0, #CACHE_LINE_SIZE
>  2:
> @@ -178,6 +182,10 @@ ENTRY(v6_flush_kern_dcache_area)
>  	add	r1, r0, r1
>  	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
>  1:
> +#ifdef CONFIG_SMP

s/CONFIG_SMP/CONFIG_RWFO/


--
Regards,
George

> +	/* no cache maintenance broadcasting on ARM11MPCore */
> +	ldr	r2, [r0]			@ read for ownership
> +#endif
>  #ifdef HARVARD_CACHE
>  	mcr	p15, 0, r0, c7, c14, 1		@ clean & invalidate D line
>  #else
> diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> index 1a8d4aa..72f9333 100644
> --- a/arch/arm/mm/flush.c
> +++ b/arch/arm/mm/flush.c
> @@ -291,8 +291,7 @@ void flush_dcache_page(struct page *page)
>  
>  	mapping = page_mapping(page);
>  
> -	if (!cache_ops_need_broadcast() &&
> -	    mapping && !mapping_mapped(mapping))
> +	if (mapping && !mapping_mapped(mapping))
>  		clear_bit(PG_dcache_clean, &page->flags);
>  	else {
>  		__flush_dcache_page(mapping, page);
> 
> -- 
> Catalin
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

parallel load of modules on an ARM multicore

[George G. Davis]

Hello,

On Mon, Jun 20, 2011 at 03:43:27PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> Hi,
> 
> I'm getting unexpected results from loading several modules - some
> of them in parallel - on an ARM11 MPcore system.
> 
> The system does not use "single sequential" modprobe commands - instead it
> starts several shells doing insmod sequences like this:
> 
> shell A:
> insmod a
> insmod ab
> insmod abc
> 
> shell B:
> insmod b
> insmod bc
> insmod bcd
> 
> shell C:
> insmod c
> insmod cd
> insmod cde
> 
> This is done to crash^H^H^H^H^Hboot faster ;)
> 
> While one insmod operation is protected via the module_mutex - I'm wondering
> what happens with the instruction cache invalidation.
> AFAICT the flush_icache_range only invalidates the ICache on the running cpu.
> The module_mutex is unlocked after _loading_ the module, do_mod_ctors() and
> do_one_initcall() are called without that lock - can they run on a different cpu?
> It's an preemptible system (SMP PREEMPT armv6l).

In case anyone missed the subtlety, this report was for an ARM11 MPCore system
with CONFIG_PREEMPT enabled.  I've also been looking into this and various other
memory corruption issues on ARM11 MPCore with CONFIG_PREEMPT enabled and have
come to the conclusion that CONFIG_PREEMPT is broken on ARM11 MPCore.

I added the following instrumentation in 3.1.0-rc4ish to watch for
process migration in a few places of interest:

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 22de005..e36d682 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -62,11 +62,18 @@ static inline pte_t *
 pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
 {
 	pte_t *pte;
+	int cpu = raw_smp_processor_id();
 
 	pte = (pte_t *)__get_free_page(PGALLOC_GFP);
 	if (pte)
 		clean_pte_table(pte);
 
+//	WARN_ON(cpu != raw_smp_processor_id());
+	if (cpu != raw_smp_processor_id())
+		printk(KERN_EMERG "%s:%d: cpu was %d but is now %d, memory corruption is possible\n",
+		       __func__, __LINE__, cpu, raw_smp_processor_id());
+
+
 	return pte;
 }
 
@@ -74,6 +81,7 @@ static inline pgtable_t
 pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 {
 	struct page *pte;
+	int cpu = raw_smp_processor_id();
 
 #ifdef CONFIG_HIGHPTE
 	pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
@@ -86,6 +94,11 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 		pgtable_page_ctor(pte);
 	}
 
+//	WARN_ON(cpu != raw_smp_processor_id());
+	if (cpu != raw_smp_processor_id())
+		printk(KERN_EMERG "%s:%d: cpu was %d but is now %d, memory corruption is possible\n",
+		       __func__, __LINE__, cpu, raw_smp_processor_id());
+
 	return pte;
 }
 
@@ -108,9 +121,14 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
 	unsigned long prot)
 {
 	unsigned long pmdval = (pte + PTE_HWTABLE_OFF) | prot;
+	int cpu = raw_smp_processor_id();
 	pmdp[0] = __pmd(pmdval);
 	pmdp[1] = __pmd(pmdval + 256 * sizeof(pte_t));
 	flush_pmd_entry(pmdp);
+//	WARN_ON(cpu != raw_smp_processor_id());
+	if (cpu != raw_smp_processor_id())
+		printk(KERN_EMERG "%s:%d: cpu was %d but is now %d, memory corruption is possible\n",
+		       __func__, __LINE__, cpu, raw_smp_processor_id());
 }
 
 /*
diff --git a/kernel/module.c b/kernel/module.c
index e0ddcec..6a43af8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2807,6 +2807,7 @@ static struct module *load_module(void __user *umod,
 	struct load_info info = { NULL, };
 	struct module *mod;
 	long err;
+	int cpu = raw_smp_processor_id();
 
 	DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n",
 	       umod, len, uargs);
@@ -2852,6 +2853,9 @@ static struct module *load_module(void __user *umod,
 	if (err < 0)
 		goto free_modinfo;
 
+	if (cpu != raw_smp_processor_id())
+		printk(KERN_EMERG "%s:%d: cpu was %d but is now %d, memory corruption is possible\n",
+		       __func__, __LINE__, cpu, raw_smp_processor_id());
 	flush_module_icache(mod);
 
 	/* Now copy in args */

Now with sufficient system stress, I get the following recurring problems
(it's a 3-core system : ):

load_module:2858: cpu was 0 but is now 1, memory corruption is possible
load_module:2858: cpu was 0 but is now 2, memory corruption is possible
load_module:2858: cpu was 1 but is now 0, memory corruption is possible
load_module:2858: cpu was 1 but is now 2, memory corruption is possible
load_module:2858: cpu was 2 but is now 0, memory corruption is possible
load_module:2858: cpu was 2 but is now 1, memory corruption is possible
pte_alloc_one:100: cpu was 0 but is now 1, memory corruption is possible
pte_alloc_one:100: cpu was 0 but is now 2, memory corruption is possible
pte_alloc_one:100: cpu was 1 but is now 0, memory corruption is possible
pte_alloc_one:100: cpu was 1 but is now 2, memory corruption is possible
pte_alloc_one:100: cpu was 2 but is now 0, memory corruption is possible
pte_alloc_one:100: cpu was 2 but is now 1, memory corruption is possible
pte_alloc_one_kernel:74: cpu was 2 but is now 1, memory corruption is possible

With sufficient stress and extended run time, the system will eventually
hang or oops with non-sensical oops traces - machine state does not
make sense relative to the code excuting@the time of the oops.

The interesting point here is that each of the above contain critical
sections in which ARM11 MPCore memory is inconsistent, i.e. cache on
CPU A contains modified entries but then migration occurs and the
cache is flushed on CPU B yet those cache ops called in the above
cases do not implement ARM11 MPCore RWFO workarounds.  Furthermore,
the current ARM11 MPCore RWFO workarounds for DMA et al are unsafe
as well for the CONFIG_PREEMPT case because, again, process migration
can occur during DMA cache maintance operations in between RWFO and
cache op instructions resulting in memory inconsistencies for the
DMA case - a very narrow but real window.

So what's the recommendation, don't use CONFIG_PREEMPT on ARM11 MPCore?

Are there any known fixes for CONFIG_PREEMPT on ARM11 MPCore if it
is indeed broken as it appears?

Thanks in advance for comments.

--
Regards,
George

> 
> Wouldn't it be required to flush the icache on _all_ cpus?
> It might be highly dependable from the cache organization, but isn't it possible
> that the cpu that didn't load the module, is scheduled to run a function from it
> with a "stale" instruction cache line???
> 
> I sometimes see oopses where I wonder how such a state could be reached.
> The code and register state seem not to fit - looks like an interrupt does not
> restore all registers properly :)
> 
> Also an Oops happens when loading module abc, and still the module list is
> reported empty in the Oops despite the fact that "abc" depends on "a". Hmh.
> 
> 
> Best regards
> 
>         Peter
> 
> <snip kernel/module.c:SYSCALL_DEFINE3(init_module>
> [..]
>         /* Only one module load at a time, please */
>         if (mutex_lock_interruptible(&module_mutex) != 0)
>                 return -EINTR;
> 
>         /* Do all the hard work */
>         mod = load_module(umod, len, uargs);
>         if (IS_ERR(mod)) {
>                 mutex_unlock(&module_mutex);
>                 return PTR_ERR(mod);
>         }
> 
>         /* Drop lock so they can recurse */
>         mutex_unlock(&module_mutex);
> 
>         blocking_notifier_call_chain(&module_notify_list,
>                         MODULE_STATE_COMING, mod);
> 
>         do_mod_ctors(mod);
>         /* Start the module */
>         if (mod->init != NULL)
>                 ret = do_one_initcall(mod->init);
> </snip>
> 
> <snip kernel/module.c:load_module>
> [..]
>         old_fs = get_fs();
>         set_fs(KERNEL_DS);
> 
>         /*
>          * Flush the instruction cache, since we've played with text.
>          * Do it before processing of module parameters, so the module
>          * can provide parameter accessor functions of its own.
>          */
>         if (mod->module_init)
>                 flush_icache_range((unsigned long)mod->module_init,
>                                    (unsigned long)mod->module_init
>                                    + mod->init_size);
>         flush_icache_range((unsigned long)mod->module_core,
>                            (unsigned long)mod->module_core + mod->core_size);
> 
>         set_fs(old_fs);
> 
> </snip>
> 
> arch/arm/mm/cache-v6.S:
> 
> ENTRY(v6_flush_kern_cache_all)
>         mov     r0, #0
> #ifdef HARVARD_CACHE
>         mcr     p15, 0, r0, c7, c14, 0          @ D cache clean+invalidate
> #ifndef CONFIG_ARM_ERRATA_411920
>         mcr     p15, 0, r0, c7, c5, 0           @ I+BTB cache invalidate
> #else
>         b       v6_icache_inval_all
> #endif
> #else
>         mcr     p15, 0, r0, c7, c15, 0          @ Cache clean+invalidate
> #endif
>         mov     pc, lr
> 
> 
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

parallel load of modules on an ARM multicore

[Catalin Marinas]

Hi George,

On 22 September 2011 08:29, George G. Davis <gdavis@mvista.com> wrote:
> On Mon, Jun 20, 2011 at 03:43:27PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
>> I'm getting unexpected results from loading several modules - some
>> of them in parallel - on an ARM11 MPcore system.
...
> In case anyone missed the subtlety, this report was for an ARM11 MPCore system
> with CONFIG_PREEMPT enabled. ?I've also been looking into this and various other
> memory corruption issues on ARM11 MPCore with CONFIG_PREEMPT enabled and have
> come to the conclusion that CONFIG_PREEMPT is broken on ARM11 MPCore.
>
> I added the following instrumentation in 3.1.0-rc4ish to watch for
> process migration in a few places of interest:
...
> Now with sufficient system stress, I get the following recurring problems
> (it's a 3-core system : ):
>
> load_module:2858: cpu was 0 but is now 1, memory corruption is possible
> load_module:2858: cpu was 0 but is now 2, memory corruption is possible
> load_module:2858: cpu was 1 but is now 0, memory corruption is possible
> load_module:2858: cpu was 1 but is now 2, memory corruption is possible
> load_module:2858: cpu was 2 but is now 0, memory corruption is possible
> load_module:2858: cpu was 2 but is now 1, memory corruption is possible
> pte_alloc_one:100: cpu was 0 but is now 1, memory corruption is possible
> pte_alloc_one:100: cpu was 0 but is now 2, memory corruption is possible
> pte_alloc_one:100: cpu was 1 but is now 0, memory corruption is possible
> pte_alloc_one:100: cpu was 1 but is now 2, memory corruption is possible
> pte_alloc_one:100: cpu was 2 but is now 0, memory corruption is possible
> pte_alloc_one:100: cpu was 2 but is now 1, memory corruption is possible
> pte_alloc_one_kernel:74: cpu was 2 but is now 1, memory corruption is possible
>
> With sufficient stress and extended run time, the system will eventually
> hang or oops with non-sensical oops traces - machine state does not
> make sense relative to the code excuting at the time of the oops.

I think your analysis is valid and these places are not safe with
CONFIG_PREEMPT enabled.

> The interesting point here is that each of the above contain critical
> sections in which ARM11 MPCore memory is inconsistent, i.e. cache on
> CPU A contains modified entries but then migration occurs and the
> cache is flushed on CPU B yet those cache ops called in the above
> cases do not implement ARM11 MPCore RWFO workarounds.

I agree, my follow-up patch to implement lazy cache flushing on
ARM11MPCore was meant for other uses (like drivers not calling
flush_dcache_page), I never had PREEMPT in mind.

> Furthermore,
> the current ARM11 MPCore RWFO workarounds for DMA et al are unsafe
> as well for the CONFIG_PREEMPT case because, again, process migration
> can occur during DMA cache maintance operations in between RWFO and
> cache op instructions resulting in memory inconsistencies for the
> DMA case - a very narrow but real window.

Yes, that's correct.

> So what's the recommendation, don't use CONFIG_PREEMPT on ARM11 MPCore?
>
> Are there any known fixes for CONFIG_PREEMPT on ARM11 MPCore if it
> is indeed broken as it appears?

The scenarios you have described look valid to me. I think for now we
can say that ARM11MPCore and PREEMPT don't go well together. This can
be fixed though by making sure that cache maintenance places with the
RWFO trick have the preemption disabled. But the RWFO has some
performance impact as well, so I would only use it where absolutely
necessary. In this case, I would just disable PREEMPT.

-- 
Catalin

parallel load of modules on an ARM multicore

[George G. Davis]

Hello Catalin,

On Sep 22, 2011, at 4:52 AM, Catalin Marinas wrote:

Ugh, sorry, I've been having problems with fetchmail/POP and did not see your
reply until just now, logged into IMAP finally.  : /

> Hi George,
> 
> On 22 September 2011 08:29, George G. Davis <gdavis@mvista.com> wrote:
>> On Mon, Jun 20, 2011 at 03:43:27PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
>>> I'm getting unexpected results from loading several modules - some
>>> of them in parallel - on an ARM11 MPcore system.
> ...
>> In case anyone missed the subtlety, this report was for an ARM11 MPCore system
>> with CONFIG_PREEMPT enabled.  I've also been looking into this and various other
>> memory corruption issues on ARM11 MPCore with CONFIG_PREEMPT enabled and have
>> come to the conclusion that CONFIG_PREEMPT is broken on ARM11 MPCore.
>> 
>> I added the following instrumentation in 3.1.0-rc4ish to watch for
>> process migration in a few places of interest:
> ...
>> Now with sufficient system stress, I get the following recurring problems
>> (it's a 3-core system : ):
>> 
>> load_module:2858: cpu was 0 but is now 1, memory corruption is possible
>> load_module:2858: cpu was 0 but is now 2, memory corruption is possible
>> load_module:2858: cpu was 1 but is now 0, memory corruption is possible
>> load_module:2858: cpu was 1 but is now 2, memory corruption is possible
>> load_module:2858: cpu was 2 but is now 0, memory corruption is possible
>> load_module:2858: cpu was 2 but is now 1, memory corruption is possible
>> pte_alloc_one:100: cpu was 0 but is now 1, memory corruption is possible
>> pte_alloc_one:100: cpu was 0 but is now 2, memory corruption is possible
>> pte_alloc_one:100: cpu was 1 but is now 0, memory corruption is possible
>> pte_alloc_one:100: cpu was 1 but is now 2, memory corruption is possible
>> pte_alloc_one:100: cpu was 2 but is now 0, memory corruption is possible
>> pte_alloc_one:100: cpu was 2 but is now 1, memory corruption is possible
>> pte_alloc_one_kernel:74: cpu was 2 but is now 1, memory corruption is possible
>> 
>> With sufficient stress and extended run time, the system will eventually
>> hang or oops with non-sensical oops traces - machine state does not
>> make sense relative to the code excuting at the time of the oops.
> 
> I think your analysis is valid and these places are not safe with
> CONFIG_PREEMPT enabled.

Alas, the stress test stability problems persist even with CONFIG_PREEMPT off.
Perhaps the windows are smaller, but they still exist.

>> The interesting point here is that each of the above contain critical
>> sections in which ARM11 MPCore memory is inconsistent, i.e. cache on
>> CPU A contains modified entries but then migration occurs and the
>> cache is flushed on CPU B yet those cache ops called in the above
>> cases do not implement ARM11 MPCore RWFO workarounds.
> 
> I agree, my follow-up patch to implement lazy cache flushing on
> ARM11MPCore was meant for other uses (like drivers not calling
> flush_dcache_page), I never had PREEMPT in mind.
> 
>> Furthermore,
>> the current ARM11 MPCore RWFO workarounds for DMA et al are unsafe
>> as well for the CONFIG_PREEMPT case because, again, process migration
>> can occur during DMA cache maintance operations in between RWFO and
>> cache op instructions resulting in memory inconsistencies for the
>> DMA case - a very narrow but real window.
> 
> Yes, that's correct.
> 
>> So what's the recommendation, don't use CONFIG_PREEMPT on ARM11 MPCore?
>> 
>> Are there any known fixes for CONFIG_PREEMPT on ARM11 MPCore if it
>> is indeed broken as it appears?
> 
> The scenarios you have described look valid to me. I think for now we
> can say that ARM11MPCore and PREEMPT don't go well together.

But it is unreliable even for the !PREEMPT case based on my stress testing,
even with your lazy cache flushing workaound applied.  : /

> This can
> be fixed though by making sure that cache maintenance places with the
> RWFO trick have the preemption disabled. But the RWFO has some
> performance impact as well, so I would only use it where absolutely
> necessary. In this case, I would just disable PREEMPT.

I'll post a series of RFC patches which adress the "low hanging fruit".  I'm still
working on the harder nuts which of course have performance trade offs
between RWFO v. broadcast cache ops to consider...

Thanks and apologies again for lack of follow up reply on my part.  I blame
my fetchmail/POP as I'm still getting at least some LAKML messages, just
not all.  : /

--
Regards,
George

> 
> -- 
> Catalin

[RFC/PATCH 0/7] ARM: ARM11 MPCore preemption/task migration cache coherency fixups

[gdavis at mvista.com]

Greetings,

On ARM11 MPCore, the "SCU does not handle coherency consequences of CP15
cache operations" [1].  So cache maintenance functions have to insure
that memory is globally consistent.  Although the current Linux kernel
works reasonably well on ARM11 MPCore machines, PREEMPT stress testing,
e.g. parallel module loading and hackbench, results in crashes which
exhibit non-sense oops traces where machine state does not make sense
relative to the code executing at the time of the oops.

Review and analysis of the various ARM11 MPCore cache maintenance
functions reveal that there are a number critical sections in which
ARM11 MPCore caches and/or memory may become inconsistent, i.e. a
cache line on CPU A contains a modified entry but preemption and task
migration occurs after which the same cache line is cleaned/flushed
on CPU B.  This can obviously lead to inconsistent memory and/or
cache state as cache ops on ARM11 MPCore are non-coherent.

The following is a partial series of ARM11 MPCore preemption/task
migration fixes to resolve cache coherency problems on these machines:

George G. Davis (6):
      ARM: ARM11 MPCore: pte_alloc_one{,_kernel} are not preempt safe
      ARM: ARM11 MPCore: {clean,flush}_pmd_entry are not preempt safe
      ARM: ARM11 MPCore: clean_dcache_area is not preempt safe
      ARM: Move get_thread_info macro definition to <asm/assembler.h>
      ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe
      ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

Konstantin Baidarov (1):
      ARM: ARM11 MPCore: pgd_alloc is not preempt safe

 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/include/asm/pgalloc.h   |   23 ++++++++++++++++++++---
 arch/arm/include/asm/pgtable.h   |    9 +++++++++
 arch/arm/include/asm/smp_plat.h  |    2 ++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/mm/cache-v6.S           |   33 +++++++++++++++++++++++++++++++++
 arch/arm/mm/idmap.c              |    5 +++++
 arch/arm/mm/ioremap.c            |    9 +++++++++
 arch/arm/mm/mmu.c                |   12 ++++++++++++
 arch/arm/mm/pgd.c                |    7 +++++++
 arch/arm/mm/proc-v6.S            |   11 +++++++++++
 arch/arm/plat-omap/iommu.c       |   10 ++++++++++
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 14 files changed, 139 insertions(+), 16 deletions(-)

The above changes are among the "low hanging fruit" where the
workarounds are relatively low impact from a performance and/or
implementation effort perspective.  I'm still working on fixes
for the harder problems.  Also, I believe that Catalin's "ARM: Allow
lazy cache flushing on ARM11MPCore" [2][3] is required for ARM11
MPCore machines and would like to see that or similar/alternative
solution applied.

Comments/feedback greatly appreciated.

TIA!

--
Regards,
George

[1] http://infocenter.arm.com/help/topic/com.arm.doc.dai0228a/index.html#arm_toc9
[2] http://www.spinics.net/lists/arm-kernel/msg129403.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-May/014990.html

[RFC/PATCH 1/7] ARM: ARM11 MPCore: pgd_alloc is not preempt safe

[gdavis at mvista.com]

From: Konstantin Baidarov <kbaidarov@mvista.com>

If preemption and subsequent task migration occurs during a call to
pgd_alloc on ARM11 MPCore machines, global memory state can become
inconsistent.  To prevent inconsistent memory state on these
machines, disable preemption around initialization and flushing
of new PGDs.

Signed-off-by: Konstantin Baidarov <kbaidarov@mvista.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/pgd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index b2027c1..6f6213e 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -14,6 +14,7 @@
 #include <asm/pgalloc.h>
 #include <asm/page.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #include "mm.h"
 
@@ -31,6 +32,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 	if (!new_pgd)
 		goto no_pgd;
 
+	if (cache_ops_need_broadcast())
+		get_cpu();
+
 	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
 
 	/*
@@ -42,6 +46,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 
 	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
 
+	if (cache_ops_need_broadcast())
+		put_cpu();
+
 	if (!vectors_high()) {
 		/*
 		 * On ARM, first page must always be allocated since it
-- 
1.7.4.4

[RFC/PATCH 2/7] ARM: ARM11 MPCore: pte_alloc_one{, _kernel} are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during a call to
pte_alloc_one{,_kernel} on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption around initialization and flushing
of new PTEs.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 22de005..cc2ac8b 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -17,6 +17,7 @@
 #include <asm/processor.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #define check_pgt_cache()		do { } while (0)
 
@@ -35,7 +36,7 @@
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
 extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
 
-#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
+#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
 
 static inline void clean_pte_table(pte_t *pte)
 {
@@ -64,8 +65,14 @@ pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
 	pte_t *pte;
 
 	pte = (pte_t *)__get_free_page(PGALLOC_GFP);
-	if (pte)
+	if (pte) {
+		if (cache_ops_need_broadcast())
+			get_cpu();
+		memset(pte, 0, PAGE_SIZE);
 		clean_pte_table(pte);
+		if (cache_ops_need_broadcast())
+			put_cpu();
+	}
 
 	return pte;
 }
@@ -81,8 +88,14 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 	pte = alloc_pages(PGALLOC_GFP, 0);
 #endif
 	if (pte) {
+		void *p = page_address(pte);
+		if (cache_ops_need_broadcast())
+			get_cpu();
+		memset(p, 0, PAGE_SIZE);
 		if (!PageHighMem(pte))
-			clean_pte_table(page_address(pte));
+			clean_pte_table(p);
+		if (cache_ops_need_broadcast())
+			put_cpu();
 		pgtable_page_ctor(pte);
 	}
 
-- 
1.7.4.4

[RFC/PATCH 3/7] ARM: ARM11 MPCore: {clean, flush}_pmd_entry are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
{clean,flush}_pmd_entry on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions around
PMD modifications and subsequent {clean,flush}_pmd_entry calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h  |    4 ++++
 arch/arm/include/asm/pgtable.h  |    9 +++++++++
 arch/arm/include/asm/smp_plat.h |    2 ++
 arch/arm/mm/idmap.c             |    5 +++++
 arch/arm/mm/ioremap.c           |    9 +++++++++
 arch/arm/mm/mmu.c               |   12 ++++++++++++
 6 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index cc2ac8b..6ba637a 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -121,9 +121,13 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
 	unsigned long prot)
 {
 	unsigned long pmdval = (pte + PTE_HWTABLE_OFF) | prot;
+	if (cache_ops_need_broadcast())
+		get_cpu();
 	pmdp[0] = __pmd(pmdval);
 	pmdp[1] = __pmd(pmdval + 256 * sizeof(pte_t));
 	flush_pmd_entry(pmdp);
+	if (cache_ops_need_broadcast())
+		put_cpu();
 }
 
 /*
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 5750704..bfef350 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -13,6 +13,7 @@
 #include <linux/const.h>
 #include <asm-generic/4level-fixup.h>
 #include <asm/proc-fns.h>
+#include <asm/smp_plat.h>
 
 #ifndef CONFIG_MMU
 
@@ -313,16 +314,24 @@ extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
 
 #define copy_pmd(pmdpd,pmdps)		\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			get_cpu();	\
 		pmdpd[0] = pmdps[0];	\
 		pmdpd[1] = pmdps[1];	\
 		flush_pmd_entry(pmdpd);	\
+		if (cache_ops_need_broadcast())	\
+			put_cpu();	\
 	} while (0)
 
 #define pmd_clear(pmdp)			\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			get_cpu();	\
 		pmdp[0] = __pmd(0);	\
 		pmdp[1] = __pmd(0);	\
 		clean_pmd_entry(pmdp);	\
+		if (cache_ops_need_broadcast())	\
+			put_cpu();	\
 	} while (0)
 
 static inline pte_t *pmd_page_vaddr(pmd_t pmd)
diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index f24c1b9..5a8d3df 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -5,6 +5,7 @@
 #ifndef __ASMARM_SMP_PLAT_H
 #define __ASMARM_SMP_PLAT_H
 
+#ifndef	__ASSEMBLY__
 #include <asm/cputype.h>
 
 /*
@@ -42,5 +43,6 @@ static inline int cache_ops_need_broadcast(void)
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 1;
 }
 #endif
+#endif
 
 #endif
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 2be9139..49c7fee 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -3,17 +3,22 @@
 #include <asm/cputype.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include <asm/smp_plat.h>
 
 static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	unsigned long prot)
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
 
+	if (cache_ops_need_broadcast())
+		get_cpu();
 	addr = (addr & PMD_MASK) | prot;
 	pmd[0] = __pmd(addr);
 	addr += SECTION_SIZE;
 	pmd[1] = __pmd(addr);
 	flush_pmd_entry(pmd);
+	if (cache_ops_need_broadcast())
+		put_cpu();
 }
 
 static void idmap_add_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ab50627..39de56e 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -32,6 +32,7 @@
 #include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
 #include <asm/sizes.h>
+#include <asm/smp_plat.h>
 
 #include <asm/mach/map.h>
 #include "mm.h"
@@ -135,11 +136,15 @@ remap_area_sections(unsigned long virt, unsigned long pfn,
 	do {
 		pmd_t *pmd = pmd_offset(pgd, addr);
 
+		if (cache_ops_need_broadcast())
+			get_cpu();
 		pmd[0] = __pmd(__pfn_to_phys(pfn) | type->prot_sect);
 		pfn += SZ_1M >> PAGE_SHIFT;
 		pmd[1] = __pmd(__pfn_to_phys(pfn) | type->prot_sect);
 		pfn += SZ_1M >> PAGE_SHIFT;
 		flush_pmd_entry(pmd);
+		if (cache_ops_need_broadcast())
+			put_cpu();
 
 		addr += PGDIR_SIZE;
 		pgd++;
@@ -172,9 +177,13 @@ remap_area_supersections(unsigned long virt, unsigned long pfn,
 		for (i = 0; i < 8; i++) {
 			pmd_t *pmd = pmd_offset(pgd, addr);
 
+			if (cache_ops_need_broadcast())
+				get_cpu();
 			pmd[0] = __pmd(super_pmd_val);
 			pmd[1] = __pmd(super_pmd_val);
 			flush_pmd_entry(pmd);
+			if (cache_ops_need_broadcast())
+				put_cpu();
 
 			addr += PGDIR_SIZE;
 			pgd++;
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 594d677..03562a9 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
 		if (addr & SECTION_SIZE)
 			pmd++;
 
+		if (cache_ops_need_broadcast())
+			get_cpu();
 		do {
 			*pmd = __pmd(phys | type->prot_sect);
 			phys += SECTION_SIZE;
 		} while (pmd++, addr += SECTION_SIZE, addr != end);
 
+		/* FIXME: Multiple PMD entries may be written above
+		 * but only one cache line, up to 8 PMDs depending
+		 * on the alignment of this mapping, is flushed below.
+		 * IFF this mapping spans >8MiB, then only the first
+		 * 8MiB worth of entries will be flushed.  Entries
+		 * above the 8MiB limit will not be flushed if I
+		 * read this correctly.
+		 */
 		flush_pmd_entry(p);
+		if (cache_ops_need_broadcast())
+			put_cpu();
 	} else {
 		/*
 		 * No need to loop; pte's aren't interested in the
-- 
1.7.4.4

[RFC/PATCH 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
clean_dcache_area on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions
around memory modifications and subsequent clean_dcache_area calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/plat-omap/iommu.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 34fc31e..ef8e065 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -1014,8 +1014,12 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto err_pgd;
 	}
+	if (cache_ops_need_broadcast())
+		get_cpu();
 	memset(p, 0, IOPGD_TABLE_SIZE);
 	clean_dcache_area(p, IOPGD_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		put_cpu();
 	obj->iopgd = p;
 
 	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
@@ -1069,7 +1073,13 @@ static struct platform_driver omap_iommu_driver = {
 
 static void iopte_cachep_ctor(void *iopte)
 {
+	if (cache_ops_need_broadcast())
+		get_cpu();
+	/* FIXME: This will not work on ARM11 MPCore.
+	 */
 	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		put_cpu();
 }
 
 static int __init omap_iommu_init(void)
-- 
1.7.4.4

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The get_thread_info assembler macro is currently defined in
arch/arm/kernel/entry-header.S and used in the following
files:

	arch/arm/kernel/entry-armv.S
	arch/arm/kernel/entry-common.S
	arch/arm/vfp/vfphw.S
	arch/arm/vfp/entry.S

The first two cases above use #include "entry-header.S" while
the later two cases use #include "../kernel/entry-header.S"
to obtain the get_thread_info assembler macro definition.

Move the get_thread_info assembler macro definition into
arch/arm/include/asm/assember.h and clean up inclusion
of this macro definition to use #include <asm/assember.h>.

This change facilitates use of the get_thread_info assembler
macro in other assembler functions without more of the same
path relative include directives.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 29035e8..78397d0 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -23,6 +23,19 @@
 #include <asm/ptrace.h>
 #include <asm/domain.h>
 
+#ifndef CONFIG_THUMB2_KERNEL
+	.macro	get_thread_info, rd
+	mov	\rd, sp, lsr #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#else   /* CONFIG_THUMB2_KERNEL */
+	.macro	get_thread_info, rd
+	mov	\rd, sp
+	lsr	\rd, \rd, #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#endif  /* !CONFIG_THUMB2_KERNEL */
+
 /*
  * Endian independent macros for shifting bytes within registers.
  */
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..d03b6a9 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -108,11 +108,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp, lsr #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
@@ -148,12 +143,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp
-	lsr	\rd, \rd, #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 4fa9903..e475a5a 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -15,9 +15,12 @@
  *  r10 = thread_info structure
  *  lr  = failure return
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 ENTRY(do_vfp)
 #ifdef CONFIG_PREEMPT
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 2d30c7f..68ca5af 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -14,9 +14,12 @@
  * r10 points at the start of the private FP workspace in the thread structure
  * sp points to a struct pt_regs (as defined in include/asm/proc/ptrace.h)
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 	.macro	DBGSTR, str
 #ifdef DEBUG
-- 
1.7.4.4

[RFC/PATCH 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The DMA_CACHE_RWFO operations are not preempt safe.  If preemption
occurs immediately following a RWFO operation on a cache line of
CPU A, it is possible for task migration to occur on resume at
which point the subsequent cache maintenance operation on the
same cache line of CPU B will have no affect on the CPU A cache
line leading to inconsistent memory and/or cache state.  To prevent
this, disable preemption during RWFO operations.

This changes depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/cache-v6.S |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 74c2e5a..445349c 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -205,6 +205,13 @@ ENTRY(v6_flush_kern_dcache_area)
  */
 v6_dma_inv_range:
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	stmdb	sp!, {r4, r10, r11}
+	get_thread_info r10
+	ldr	r4, [r10, #TI_PREEMPT]		@ get preempt count
+	add	r11, r4, #1			@ increment it
+	str	r11, [r10, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]			@ read for ownership
 	strb	r2, [r0]			@ write for ownership
 #endif
@@ -241,6 +248,10 @@ v6_dma_inv_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r4, [r10, #TI_PREEMPT]		@ restore preempt count
+	ldmia	sp!, {r4, r10, r11}
+#endif
 	mov	pc, lr
 
 /*
@@ -249,6 +260,13 @@ v6_dma_inv_range:
  *	- end     - virtual end address of region
  */
 v6_dma_clean_range:
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	stmdb	sp!, {r4, r10, r11}
+	get_thread_info r10
+	ldr	r4, [r10, #TI_PREEMPT]		@ get preempt count
+	add	r11, r4, #1			@ increment it
+	str	r11, [r10, #TI_PREEMPT]		@ disable preempt
+#endif
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
 #ifdef CONFIG_DMA_CACHE_RWFO
@@ -264,6 +282,10 @@ v6_dma_clean_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r4, [r10, #TI_PREEMPT]		@ restore preempt count
+	ldmia	sp!, {r4, r10, r11}
+#endif
 	mov	pc, lr
 
 /*
@@ -273,6 +295,13 @@ v6_dma_clean_range:
  */
 ENTRY(v6_dma_flush_range)
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	stmdb	sp!, {r4, r10, r11}
+	get_thread_info r10
+	ldr	r4, [r10, #TI_PREEMPT]		@ get preempt count
+	add	r11, r4, #1			@ increment it
+	str	r11, [r10, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]		@ read for ownership
 	strb	r2, [r0]		@ write for ownership
 #endif
@@ -292,6 +321,10 @@ ENTRY(v6_dma_flush_range)
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r4, [r10, #TI_PREEMPT]		@ restore preempt count
+	ldmia	sp!, {r4, r10, r11}
+#endif
 	mov	pc, lr
 
 /*
-- 
1.7.4.4

[RFC/PATCH 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in cpu_v6_set_pte_ext.

This changes depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/proc-v6.S |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
index a923aa0..dd2e5e5 100644
--- a/arch/arm/mm/proc-v6.S
+++ b/arch/arm/mm/proc-v6.S
@@ -122,7 +122,18 @@ ENTRY(cpu_v6_switch_mm)
 
 ENTRY(cpu_v6_set_pte_ext)
 #ifdef CONFIG_MMU
+#ifdef CONFIG_PREEMPT
+	ALT_SMP(stmdb	sp!, {r4, r10, r11})
+	ALT_SMP(get_thread_info r10)
+	ALT_SMP(ldr	r4, [r10, #TI_PREEMPT])	@ get preempt count
+	ALT_SMP(add	r11, r4, #1)		@ increment it
+	ALT_SMP(str	r11, [r10, #TI_PREEMPT])	@ disable preempt
+#endif
 	armv6_set_pte_ext cpu_v6
+#ifdef CONFIG_PREEMPT
+	ALT_SMP(str	r4, [r10, #TI_PREEMPT])	@ restore preempt count
+	ALT_SMP(ldmia	sp!, {r4, r10, r11})
+#endif
 #endif
 	mov	pc, lr
 
-- 
1.7.4.4

[RFC/PATCH 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[Russell King - ARM Linux]

On Thu, Oct 06, 2011 at 01:08:34AM -0400, gdavis at mvista.com wrote:
> diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
> index a923aa0..dd2e5e5 100644
> --- a/arch/arm/mm/proc-v6.S
> +++ b/arch/arm/mm/proc-v6.S
> @@ -122,7 +122,18 @@ ENTRY(cpu_v6_switch_mm)
>  
>  ENTRY(cpu_v6_set_pte_ext)
>  #ifdef CONFIG_MMU
> +#ifdef CONFIG_PREEMPT
> +	ALT_SMP(stmdb	sp!, {r4, r10, r11})
> +	ALT_SMP(get_thread_info r10)
> +	ALT_SMP(ldr	r4, [r10, #TI_PREEMPT])	@ get preempt count
> +	ALT_SMP(add	r11, r4, #1)		@ increment it
> +	ALT_SMP(str	r11, [r10, #TI_PREEMPT])	@ disable preempt
> +#endif
>  	armv6_set_pte_ext cpu_v6
> +#ifdef CONFIG_PREEMPT
> +	ALT_SMP(str	r4, [r10, #TI_PREEMPT])	@ restore preempt count
> +	ALT_SMP(ldmia	sp!, {r4, r10, r11})
> +#endif
>  #endif

And what effect do you expect the above to have?  Do you understand
the purpose of the ALT_SMP macro?  Do you understand that it should
be paired with ALT_UP ?

[RFC/PATCH 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[George G. Davis]

On Oct 6, 2011, at 3:46 AM, Russell King - ARM Linux wrote:

> On Thu, Oct 06, 2011 at 01:08:34AM -0400, gdavis at mvista.com wrote:
>> diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
>> index a923aa0..dd2e5e5 100644
>> --- a/arch/arm/mm/proc-v6.S
>> +++ b/arch/arm/mm/proc-v6.S
>> @@ -122,7 +122,18 @@ ENTRY(cpu_v6_switch_mm)
>> 
>> ENTRY(cpu_v6_set_pte_ext)
>> #ifdef CONFIG_MMU
>> +#ifdef CONFIG_PREEMPT
>> +	ALT_SMP(stmdb	sp!, {r4, r10, r11})
>> +	ALT_SMP(get_thread_info r10)
>> +	ALT_SMP(ldr	r4, [r10, #TI_PREEMPT])	@ get preempt count
>> +	ALT_SMP(add	r11, r4, #1)		@ increment it
>> +	ALT_SMP(str	r11, [r10, #TI_PREEMPT])	@ disable preempt
>> +#endif
>> 	armv6_set_pte_ext cpu_v6
>> +#ifdef CONFIG_PREEMPT
>> +	ALT_SMP(str	r4, [r10, #TI_PREEMPT])	@ restore preempt count
>> +	ALT_SMP(ldmia	sp!, {r4, r10, r11})
>> +#endif
>> #endif
> 
> And what effect do you expect the above to have?

My goal was to be proactive, heading off a request to use this macro for
the SMP case since preempt disable is only required on ARM11 MPCore.
I thought this would achieve that goal, only using those instructions on SMP,
not UP machines.

> Do you understand the purpose of the ALT_SMP macro?

Clearly I don't understand the use case.

> Do you understand that it should be paired with ALT_UP ?

No, I did not know that.  I'll remove use of ALT_SMP and add CONFIG_SMP
as a conditional compile dependency then.  Is that OK?

Thanks!

--
Regards,
George

[RFC/PATCH 1/7] ARM: ARM11 MPCore: pgd_alloc is not preempt safe

[Russell King - ARM Linux]

On Thu, Oct 06, 2011 at 01:08:28AM -0400, gdavis at mvista.com wrote:
> @@ -31,6 +32,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
>  	if (!new_pgd)
>  		goto no_pgd;
>  
> +	if (cache_ops_need_broadcast())
> +		get_cpu();

Is there something wrong with preempt_disable() here and preempt_enable()
below?  If it's preempt that we're concerned about, these are the correct
interfaces to be used.

> +
>  	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
>  
>  	/*
> @@ -42,6 +46,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
>  
>  	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
>  
> +	if (cache_ops_need_broadcast())
> +		put_cpu();
> +

[RFC/PATCH 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[Russell King - ARM Linux]

On Thu, Oct 06, 2011 at 01:08:33AM -0400, gdavis at mvista.com wrote:
>  v6_dma_inv_range:
>  #ifdef CONFIG_DMA_CACHE_RWFO
> +#ifdef CONFIG_PREEMPT
> +	stmdb	sp!, {r4, r10, r11}
> +	get_thread_info r10
> +	ldr	r4, [r10, #TI_PREEMPT]		@ get preempt count
> +	add	r11, r4, #1			@ increment it
> +	str	r11, [r10, #TI_PREEMPT]		@ disable preempt
> +#endif

r11 is the frame pointer.  On kernels built with the frame pointer
enabled, this register must either be a valid frame pointer or zero.

There's no reason to use r4, r10 and r11 here - you could use r4, r5
and ip (r12) - and then there's no need to save ip as that's allowed
to be corrupted by called functions.

[RFC/PATCH 1/7] ARM: ARM11 MPCore: pgd_alloc is not preempt safe

[George G. Davis]

On Oct 6, 2011, at 12:35 PM, Russell King - ARM Linux wrote:

> On Thu, Oct 06, 2011 at 01:08:28AM -0400, gdavis at mvista.com wrote:
>> @@ -31,6 +32,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
>> 	if (!new_pgd)
>> 		goto no_pgd;
>> 
>> +	if (cache_ops_need_broadcast())
>> +		get_cpu();
> 
> Is there something wrong with preempt_disable() here and preempt_enable()
> below?

I used the {get,put}_cpu() variants merely to (try to) make it clear that we want
this region to be non-preemptible for the current CPU.

>  If it's preempt that we're concerned about, these are the correct
> interfaces to be used.

Since  {get,put}_cpu() are wrappers for preempt_{enable,disable}, it works
either way and I'm happy to just use preempt_{enable,disable} instead.  It
achieves the same goal with slightly less overhead.  Also, while we're on
the subject of preempt_enable, an earlier version of this change used
preempt_enable_no_resched() but I chose to avoid the no_reched flavor
since I felt that it was worth minimizing scheduler latencies.

I'll change all of the  {get,put}_cpu() variants to  preempt_{enable,disable}
and submit an updated series.

Thanks!

--
Regards,
George


>> +
>> 	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
>> 
>> 	/*
>> @@ -42,6 +46,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
>> 
>> 	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
>> 
>> +	if (cache_ops_need_broadcast())
>> +		put_cpu();
>> +

[RFC/PATCH 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[George G. Davis]

On Oct 6, 2011, at 12:40 PM, Russell King - ARM Linux wrote:

> On Thu, Oct 06, 2011 at 01:08:33AM -0400, gdavis at mvista.com wrote:
>> v6_dma_inv_range:
>> #ifdef CONFIG_DMA_CACHE_RWFO
>> +#ifdef CONFIG_PREEMPT
>> +	stmdb	sp!, {r4, r10, r11}
>> +	get_thread_info r10
>> +	ldr	r4, [r10, #TI_PREEMPT]		@ get preempt count
>> +	add	r11, r4, #1			@ increment it
>> +	str	r11, [r10, #TI_PREEMPT]		@ disable preempt
>> +#endif
> 
> r11 is the frame pointer.  On kernels built with the frame pointer
> enabled, this register must either be a valid frame pointer or zero.

Doh!

> There's no reason to use r4, r10 and r11 here - you could use r4, r5
> and ip (r12) - and then there's no need to save ip as that's allowed
> to be corrupted by called functions.

OK, I'll make these changes in the next round.

Thanks again!

--
Regards,
George

[RFC/PATCH v2 0/7] ARM11 MPCore: preemption/task migration cache coherency fixups

[gdavis at mvista.com]

Greetings,

On ARM11 MPCore, the "SCU does not handle coherency consequences of CP15
cache operations" [1]. ?So cache maintenance functions have to insure
that memory is globally consistent. ?Although the current Linux kernel
works reasonably well on ARM11 MPCore machines, PREEMPT stress testing,
e.g. parallel module loading and hackbench, results in crashes which
exhibit non-sense oops traces where machine state does not make sense
relative to the code executing at the time of the oops.

Review and analysis of the various ARM11 MPCore cache maintenance
functions reveal that there are a number critical sections in which
ARM11 MPCore caches and/or memory may become inconsistent, i.e. a
cache line on CPU A contains a modified entry but preemption and task
migration occurs after which the same cache line is cleaned/flushed
on CPU B. ?This can obviously lead to inconsistent memory and/or
cache state as cache ops on ARM11 MPCore are non-coherent.

The following is a partial series of ARM11 MPCore preemption/task
migration fixes to resolve cache coherency problems on these machines:

George G. Davis (6):
      ARM: ARM11 MPCore: pte_alloc_one{,_kernel} are not preempt safe
      ARM: ARM11 MPCore: {clean,flush}_pmd_entry are not preempt safe
      ARM: ARM11 MPCore: clean_dcache_area is not preempt safe
      ARM: Move get_thread_info macro definition to <asm/assembler.h>
      ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe
      ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

Konstantin Baidarov (1):
      ARM: ARM11 MPCore: pgd_alloc is not preempt safe

 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/include/asm/pgalloc.h   |   23 ++++++++++++++++++++---
 arch/arm/include/asm/pgtable.h   |    9 +++++++++
 arch/arm/include/asm/smp_plat.h  |    2 ++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/mm/cache-v6.S           |   27 +++++++++++++++++++++++++++
 arch/arm/mm/idmap.c              |    5 +++++
 arch/arm/mm/ioremap.c            |    9 +++++++++
 arch/arm/mm/mmu.c                |   12 ++++++++++++
 arch/arm/mm/pgd.c                |    7 +++++++
 arch/arm/mm/proc-v6.S            |   11 +++++++++++
 arch/arm/plat-omap/iommu.c       |   10 ++++++++++
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 14 files changed, 133 insertions(+), 16 deletions(-)

ChangeLog:

V2:
- Substitute {get,put}_cpu() with preempt_{disable,enable}().
- Fixed preempt {dis,en}able assembler code sequences to not
  use r11 since it is reserved for frame pointer use.  Also
  optimised these sequences to use r2, r3; ip scratch registers
  in most cases to eliminate stack push/pop.  In one case,
  cpu_v6_set_pte_ext, there are only two scratch registers
  available, r3 and ip.  However, both of these are used within
  the armv6_set_pte_ext macro.  So for this case, r3 is used
  as a temporary scratch when disabling preemption and r4 and
  r5 are pushed/popped as needed for other uses to avoid
  conflict with scratch register usage in armv6_set_pte_ext.
- Remove incorrect use of ALT_SMP macros in cpu_v6_set_pte_ext,
  making the preempt {dis,en}able assembler code sequences
  compile time dependent upon CONFIG_SMP instead.  This code
  is safe on UP machines anyway.

The above changes are among the "low hanging fruit" where the
workarounds are relatively low impact from a performance and/or
implementation effort perspective. ?I'm still working on fixes
for the harder problems. ?Also, I believe that Catalin's "ARM: Allow
lazy cache flushing on ARM11MPCore" [2][3] is required for ARM11
MPCore machines and would like to see that or similar/alternative
solution applied.

Comments/feedback greatly appreciated.

TIA!

--
Regards,
George

[1] http://infocenter.arm.com/help/topic/com.arm.doc.dai0228a/index.html#arm_toc9
[2] http://www.spinics.net/lists/arm-kernel/msg129403.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-May/014990.html

[RFC/PATCH 1/7] ARM: ARM11 MPCore: pgd_alloc is not preempt safe

[gdavis at mvista.com]

From: Konstantin Baidarov <kbaidarov@mvista.com>

If preemption and subsequent task migration occurs during a call to
pgd_alloc on ARM11 MPCore machines, global memory state can become
inconsistent.  To prevent inconsistent memory state on these
machines, disable preemption around initialization and flushing
of new PGDs.

Signed-off-by: Konstantin Baidarov <kbaidarov@mvista.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/pgd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index b2027c1..e608981 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -14,6 +14,7 @@
 #include <asm/pgalloc.h>
 #include <asm/page.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #include "mm.h"
 
@@ -31,6 +32,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 	if (!new_pgd)
 		goto no_pgd;
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+
 	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
 
 	/*
@@ -42,6 +46,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 
 	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
 
+	if (cache_ops_need_broadcast())
+		preempt_enable();
+
 	if (!vectors_high()) {
 		/*
 		 * On ARM, first page must always be allocated since it
-- 
1.7.4.4

[RFC/PATCH 2/7] ARM: ARM11 MPCore: pte_alloc_one{, _kernel} are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during a call to
pte_alloc_one{,_kernel} on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption around initialization and flushing
of new PTEs.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 22de005..2154ccab 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -17,6 +17,7 @@
 #include <asm/processor.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #define check_pgt_cache()		do { } while (0)
 
@@ -35,7 +36,7 @@
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
 extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
 
-#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
+#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
 
 static inline void clean_pte_table(pte_t *pte)
 {
@@ -64,8 +65,14 @@ pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
 	pte_t *pte;
 
 	pte = (pte_t *)__get_free_page(PGALLOC_GFP);
-	if (pte)
+	if (pte) {
+		if (cache_ops_need_broadcast())
+			preempt_disable();
+		memset(pte, 0, PAGE_SIZE);
 		clean_pte_table(pte);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
+	}
 
 	return pte;
 }
@@ -81,8 +88,14 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 	pte = alloc_pages(PGALLOC_GFP, 0);
 #endif
 	if (pte) {
+		void *p = page_address(pte);
+		if (cache_ops_need_broadcast())
+			preempt_disable();
+		memset(p, 0, PAGE_SIZE);
 		if (!PageHighMem(pte))
-			clean_pte_table(page_address(pte));
+			clean_pte_table(p);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
 		pgtable_page_ctor(pte);
 	}
 
-- 
1.7.4.4

[RFC/PATCH 3/7] ARM: ARM11 MPCore: {clean, flush}_pmd_entry are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
{clean,flush}_pmd_entry on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions around
PMD modifications and subsequent {clean,flush}_pmd_entry calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h  |    4 ++++
 arch/arm/include/asm/pgtable.h  |    9 +++++++++
 arch/arm/include/asm/smp_plat.h |    2 ++
 arch/arm/mm/idmap.c             |    5 +++++
 arch/arm/mm/ioremap.c           |    9 +++++++++
 arch/arm/mm/mmu.c               |   12 ++++++++++++
 6 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 2154ccab..e3c45b1 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -121,9 +121,13 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
 	unsigned long prot)
 {
 	unsigned long pmdval = (pte + PTE_HWTABLE_OFF) | prot;
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	pmdp[0] = __pmd(pmdval);
 	pmdp[1] = __pmd(pmdval + 256 * sizeof(pte_t));
 	flush_pmd_entry(pmdp);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 /*
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 5750704..00068dc 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -13,6 +13,7 @@
 #include <linux/const.h>
 #include <asm-generic/4level-fixup.h>
 #include <asm/proc-fns.h>
+#include <asm/smp_plat.h>
 
 #ifndef CONFIG_MMU
 
@@ -313,16 +314,24 @@ extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
 
 #define copy_pmd(pmdpd,pmdps)		\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdpd[0] = pmdps[0];	\
 		pmdpd[1] = pmdps[1];	\
 		flush_pmd_entry(pmdpd);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 #define pmd_clear(pmdp)			\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdp[0] = __pmd(0);	\
 		pmdp[1] = __pmd(0);	\
 		clean_pmd_entry(pmdp);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 static inline pte_t *pmd_page_vaddr(pmd_t pmd)
diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index f24c1b9..5a8d3df 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -5,6 +5,7 @@
 #ifndef __ASMARM_SMP_PLAT_H
 #define __ASMARM_SMP_PLAT_H
 
+#ifndef	__ASSEMBLY__
 #include <asm/cputype.h>
 
 /*
@@ -42,5 +43,6 @@ static inline int cache_ops_need_broadcast(void)
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 1;
 }
 #endif
+#endif
 
 #endif
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 2be9139..c04face 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -3,17 +3,22 @@
 #include <asm/cputype.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include <asm/smp_plat.h>
 
 static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	unsigned long prot)
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	addr = (addr & PMD_MASK) | prot;
 	pmd[0] = __pmd(addr);
 	addr += SECTION_SIZE;
 	pmd[1] = __pmd(addr);
 	flush_pmd_entry(pmd);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 static void idmap_add_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ab50627..b56d78a 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -32,6 +32,7 @@
 #include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
 #include <asm/sizes.h>
+#include <asm/smp_plat.h>
 
 #include <asm/mach/map.h>
 #include "mm.h"
@@ -135,11 +136,15 @@ remap_area_sections(unsigned long virt, unsigned long pfn,
 	do {
 		pmd_t *pmd = pmd_offset(pgd, addr);
 
+		if (cache_ops_need_broadcast())
+			preempt_disable();
 		pmd[0] = __pmd(__pfn_to_phys(pfn) | type->prot_sect);
 		pfn += SZ_1M >> PAGE_SHIFT;
 		pmd[1] = __pmd(__pfn_to_phys(pfn) | type->prot_sect);
 		pfn += SZ_1M >> PAGE_SHIFT;
 		flush_pmd_entry(pmd);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
 
 		addr += PGDIR_SIZE;
 		pgd++;
@@ -172,9 +177,13 @@ remap_area_supersections(unsigned long virt, unsigned long pfn,
 		for (i = 0; i < 8; i++) {
 			pmd_t *pmd = pmd_offset(pgd, addr);
 
+			if (cache_ops_need_broadcast())
+				preempt_disable();
 			pmd[0] = __pmd(super_pmd_val);
 			pmd[1] = __pmd(super_pmd_val);
 			flush_pmd_entry(pmd);
+			if (cache_ops_need_broadcast())
+				preempt_enable();
 
 			addr += PGDIR_SIZE;
 			pgd++;
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 594d677..3c8253f 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
 		if (addr & SECTION_SIZE)
 			pmd++;
 
+		if (cache_ops_need_broadcast())
+			preempt_disable();
 		do {
 			*pmd = __pmd(phys | type->prot_sect);
 			phys += SECTION_SIZE;
 		} while (pmd++, addr += SECTION_SIZE, addr != end);
 
+		/* FIXME: Multiple PMD entries may be written above
+		 * but only one cache line, up to 8 PMDs depending
+		 * on the alignment of this mapping, is flushed below.
+		 * IFF this mapping spans >8MiB, then only the first
+		 * 8MiB worth of entries will be flushed.  Entries
+		 * above the 8MiB limit will not be flushed if I
+		 * read this correctly.
+		 */
 		flush_pmd_entry(p);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
 	} else {
 		/*
 		 * No need to loop; pte's aren't interested in the
-- 
1.7.4.4

[RFC/PATCH 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
clean_dcache_area on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions
around memory modifications and subsequent clean_dcache_area calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/plat-omap/iommu.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 34fc31e..59836d1 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -1014,8 +1014,12 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto err_pgd;
 	}
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	memset(p, 0, IOPGD_TABLE_SIZE);
 	clean_dcache_area(p, IOPGD_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 	obj->iopgd = p;
 
 	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
@@ -1069,7 +1073,13 @@ static struct platform_driver omap_iommu_driver = {
 
 static void iopte_cachep_ctor(void *iopte)
 {
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+	/* FIXME: This will not work on ARM11 MPCore.
+	 */
 	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 static int __init omap_iommu_init(void)
-- 
1.7.4.4

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The get_thread_info assembler macro is currently defined in
arch/arm/kernel/entry-header.S and used in the following
files:

	arch/arm/kernel/entry-armv.S
	arch/arm/kernel/entry-common.S
	arch/arm/vfp/vfphw.S
	arch/arm/vfp/entry.S

The first two cases above use #include "entry-header.S" while
the later two cases use #include "../kernel/entry-header.S"
to obtain the get_thread_info assembler macro definition.

Move the get_thread_info assembler macro definition into
arch/arm/include/asm/assember.h and clean up inclusion
of this macro definition to use #include <asm/assember.h>.

This change facilitates use of the get_thread_info assembler
macro in other assembler functions without more of the same
path relative include directives.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 29035e8..78397d0 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -23,6 +23,19 @@
 #include <asm/ptrace.h>
 #include <asm/domain.h>
 
+#ifndef CONFIG_THUMB2_KERNEL
+	.macro	get_thread_info, rd
+	mov	\rd, sp, lsr #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#else   /* CONFIG_THUMB2_KERNEL */
+	.macro	get_thread_info, rd
+	mov	\rd, sp
+	lsr	\rd, \rd, #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#endif  /* !CONFIG_THUMB2_KERNEL */
+
 /*
  * Endian independent macros for shifting bytes within registers.
  */
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..d03b6a9 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -108,11 +108,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp, lsr #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
@@ -148,12 +143,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp
-	lsr	\rd, \rd, #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 4fa9903..e475a5a 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -15,9 +15,12 @@
  *  r10 = thread_info structure
  *  lr  = failure return
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 ENTRY(do_vfp)
 #ifdef CONFIG_PREEMPT
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 2d30c7f..68ca5af 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -14,9 +14,12 @@
  * r10 points at the start of the private FP workspace in the thread structure
  * sp points to a struct pt_regs (as defined in include/asm/proc/ptrace.h)
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 	.macro	DBGSTR, str
 #ifdef DEBUG
-- 
1.7.4.4

[RFC/PATCH 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The DMA_CACHE_RWFO operations are not preempt safe.  If preemption
occurs immediately following a RWFO operation on a cache line of
CPU A, it is possible for task migration to occur on resume at
which point the subsequent cache maintenance operation on the
same cache line of CPU B will have no affect on the CPU A cache
line leading to inconsistent memory and/or cache state.  To prevent
this, disable preemption during RWFO operations.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/cache-v6.S |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 74c2e5a..9c44752 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -205,6 +205,12 @@ ENTRY(v6_flush_kern_dcache_area)
  */
 v6_dma_inv_range:
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]			@ read for ownership
 	strb	r2, [r0]			@ write for ownership
 #endif
@@ -241,6 +247,9 @@ v6_dma_inv_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+#endif
 	mov	pc, lr
 
 /*
@@ -249,6 +258,12 @@ v6_dma_inv_range:
  *	- end     - virtual end address of region
  */
 v6_dma_clean_range:
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
 #ifdef CONFIG_DMA_CACHE_RWFO
@@ -264,6 +279,9 @@ v6_dma_clean_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+#endif
 	mov	pc, lr
 
 /*
@@ -273,6 +291,12 @@ v6_dma_clean_range:
  */
 ENTRY(v6_dma_flush_range)
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]		@ read for ownership
 	strb	r2, [r0]		@ write for ownership
 #endif
@@ -292,6 +316,9 @@ ENTRY(v6_dma_flush_range)
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+#endif
 	mov	pc, lr
 
 /*
-- 
1.7.4.4

[RFC/PATCH 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in cpu_v6_set_pte_ext.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/proc-v6.S |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
index a923aa0..57b359d 100644
--- a/arch/arm/mm/proc-v6.S
+++ b/arch/arm/mm/proc-v6.S
@@ -122,7 +122,18 @@ ENTRY(cpu_v6_switch_mm)
 
 ENTRY(cpu_v6_set_pte_ext)
 #ifdef CONFIG_MMU
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	stmdb	sp!, {r4, r5}
+	get_thread_info r5
+	ldr	r4, [r5, #TI_PREEMPT]	@ get preempt count
+	add	r3, r4, #1		@ increment it
+	str	r3, [r5, #TI_PREEMPT]	@ disable preempt
+#endif
 	armv6_set_pte_ext cpu_v6
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	str	r4, [r5, #TI_PREEMPT]	@ restore preempt count
+	ldmia	sp!, {r4, r5}
+#endif
 #endif
 	mov	pc, lr
 
-- 
1.7.4.4

[RFC/PATCH 2/7] ARM: ARM11 MPCore: pte_alloc_one{,_kernel} are not preempt safe

[Russell King - ARM Linux]

On Thu, Oct 06, 2011 at 10:38:36PM -0400, gdavis at mvista.com wrote:
> -#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
> +#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
> @@ -81,8 +88,14 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
>  	pte = alloc_pages(PGALLOC_GFP, 0);
>  #endif
>  	if (pte) {
> +		void *p = page_address(pte);
> +		if (cache_ops_need_broadcast())
> +			preempt_disable();
> +		memset(p, 0, PAGE_SIZE);
>  		if (!PageHighMem(pte))
> -			clean_pte_table(page_address(pte));
> +			clean_pte_table(p);
> +		if (cache_ops_need_broadcast())
> +			preempt_enable();

This won't work - page_address(pte) will be NULL for highmem pages, and
so will cause an oops, and removing the __GFP_ZERO will mean that highmem
pages will not be zeroed, meaning that such page tables will not have been
initialized.

[RFC/PATCH 2/7] ARM: ARM11 MPCore: pte_alloc_one{, _kernel} are not preempt safe

[George G. Davis]

On Oct 7, 2011, at 3:47 AM, Russell King - ARM Linux wrote:

> On Thu, Oct 06, 2011 at 10:38:36PM -0400, gdavis at mvista.com wrote:
>> -#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
>> +#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
>> @@ -81,8 +88,14 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
>> 	pte = alloc_pages(PGALLOC_GFP, 0);
>> #endif
>> 	if (pte) {
>> +		void *p = page_address(pte);
>> +		if (cache_ops_need_broadcast())
>> +			preempt_disable();
>> +		memset(p, 0, PAGE_SIZE);
>> 		if (!PageHighMem(pte))
>> -			clean_pte_table(page_address(pte));
>> +			clean_pte_table(p);
>> +		if (cache_ops_need_broadcast())
>> +			preempt_enable();
> 
> This won't work - page_address(pte) will be NULL for highmem pages, and
> so will cause an oops, and removing the __GFP_ZERO will mean that highmem
> pages will not be zeroed, meaning that such page tables will not have been
> initialized.

Yep, I had a feeling that it was not going to work correctly for the HIGHMEM case.
Alas, I'm not using HIGHMEM so I did not see the issue in my testing.  I'll respin it to fix
HIGHMEM breakage.

Thanks!

--
Regards,
George

[RFC/PATCH v3 0/7] ARM11 MPCore: preemption/task migration cache coherency fixups

[gdavis at mvista.com]

Greetings,

On ARM11 MPCore, the "SCU does not handle coherency consequences of CP15
cache operations" [1]. ?So cache maintenance functions have to insure
that memory is globally consistent. ?Although the current Linux kernel
works reasonably well on ARM11 MPCore machines, PREEMPT stress testing,
e.g. parallel module loading and hackbench, results in crashes which
exhibit non-sense oops traces where machine state does not make sense
relative to the code executing at the time of the oops.

Review and analysis of the various ARM11 MPCore cache maintenance
functions reveal that there are a number critical sections in which
ARM11 MPCore caches and/or memory may become inconsistent, i.e. a
cache line on CPU A contains a modified entry but preemption and task
migration occurs after which the same cache line is cleaned/flushed
on CPU B. ?This can obviously lead to inconsistent memory and/or
cache state as cache ops on ARM11 MPCore are non-coherent.

The following is a partial series of ARM11 MPCore preemption/task
migration fixes to resolve cache coherency problems on these machines:

George G. Davis (6):
      ARM: ARM11 MPCore: pte_alloc_one{,_kernel} are not preempt safe
      ARM: ARM11 MPCore: {clean,flush}_pmd_entry are not preempt safe
      ARM: ARM11 MPCore: clean_dcache_area is not preempt safe
      ARM: Move get_thread_info macro definition to <asm/assembler.h>
      ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe
      ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

Konstantin Baidarov (1):
      ARM: ARM11 MPCore: pgd_alloc is not preempt safe

 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/include/asm/pgalloc.h   |   28 +++++++++++++++++++++++-----
 arch/arm/include/asm/pgtable.h   |    9 +++++++++
 arch/arm/include/asm/smp_plat.h  |    2 ++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/mm/cache-v6.S           |   27 +++++++++++++++++++++++++++
 arch/arm/mm/idmap.c              |    5 +++++
 arch/arm/mm/ioremap.c            |    9 +++++++++
 arch/arm/mm/mmu.c                |   12 ++++++++++++
 arch/arm/mm/pgd.c                |    7 +++++++
 arch/arm/mm/proc-v6.S            |   11 +++++++++++
 arch/arm/plat-omap/iommu.c       |   10 ++++++++++
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 14 files changed, 136 insertions(+), 18 deletions(-)

ChangeLog:

V2:
- Substitute {get,put}_cpu() with preempt_{disable,enable}().
- Fixed preempt {dis,en}able assembler code sequences to not
  use r11 since it is reserved for frame pointer use.  Also
  optimised these sequences to use r2, r3; ip scratch registers
  in most cases to eliminate stack push/pop.  In one case,
  cpu_v6_set_pte_ext, there are only two scratch registers
  available, r3 and ip.  However, both of these are used within
  the armv6_set_pte_ext macro.  So for this case, r3 is used
  as a temporary scratch when disabling preemption and r4 and
  r5 are pushed/popped as needed for other uses to avoid
  conflict with scratch register usage in armv6_set_pte_ext.
- Remove incorrect use of ALT_SMP macros in cpu_v6_set_pte_ext,
  making the preempt {dis,en}able assembler code sequences
  compile time dependent upon CONFIG_SMP instead.  This code
  is safe on UP machines anyway.
V3:
- Fix HIGHMEM breakage.

The above changes are among the "low hanging fruit" where the
workarounds are relatively low impact from a performance and/or
implementation effort perspective. ?I'm still working on fixes
for the harder problems. ?Also, I believe that Catalin's "ARM: Allow
lazy cache flushing on ARM11MPCore" [2][3] is required for ARM11
MPCore machines and would like to see that or similar/alternative
solution applied.

Comments/feedback greatly appreciated.

TIA!

--
Regards,
George

[1] http://infocenter.arm.com/help/topic/com.arm.doc.dai0228a/index.html#arm_toc9
[2] http://www.spinics.net/lists/arm-kernel/msg129403.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-May/014990.html

[RFC/PATCH v3 1/7] ARM: ARM11 MPCore: pgd_alloc is not preempt safe

[gdavis at mvista.com]

From: Konstantin Baidarov <kbaidarov@mvista.com>

If preemption and subsequent task migration occurs during a call to
pgd_alloc on ARM11 MPCore machines, global memory state can become
inconsistent.  To prevent inconsistent memory state on these
machines, disable preemption around initialization and flushing
of new PGDs.

Signed-off-by: Konstantin Baidarov <kbaidarov@mvista.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/pgd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index b2027c1..e608981 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -14,6 +14,7 @@
 #include <asm/pgalloc.h>
 #include <asm/page.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #include "mm.h"
 
@@ -31,6 +32,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 	if (!new_pgd)
 		goto no_pgd;
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+
 	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
 
 	/*
@@ -42,6 +46,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 
 	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
 
+	if (cache_ops_need_broadcast())
+		preempt_enable();
+
 	if (!vectors_high()) {
 		/*
 		 * On ARM, first page must always be allocated since it
-- 
1.7.4.4

[RFC/PATCH v3 2/7] ARM: ARM11 MPCore: pte_alloc_one{, _kernel} are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during a call to
pte_alloc_one{,_kernel} on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption around initialization and flushing
of new PTEs.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 22de005..92d3f27 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -17,6 +17,7 @@
 #include <asm/processor.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #define check_pgt_cache()		do { } while (0)
 
@@ -35,7 +36,7 @@
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
 extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
 
-#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
+#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
 
 static inline void clean_pte_table(pte_t *pte)
 {
@@ -64,8 +65,14 @@ pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
 	pte_t *pte;
 
 	pte = (pte_t *)__get_free_page(PGALLOC_GFP);
-	if (pte)
+	if (pte) {
+		if (cache_ops_need_broadcast())
+			preempt_disable();
+		memset(pte, 0, PAGE_SIZE);
 		clean_pte_table(pte);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
+	}
 
 	return pte;
 }
@@ -76,13 +83,20 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 	struct page *pte;
 
 #ifdef CONFIG_HIGHPTE
-	pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
+	pte = alloc_pages(PGALLOC_GFP | __GFP_ZERO | __GFP_HIGHMEM, 0);
 #else
 	pte = alloc_pages(PGALLOC_GFP, 0);
 #endif
 	if (pte) {
-		if (!PageHighMem(pte))
-			clean_pte_table(page_address(pte));
+		if (!PageHighMem(pte)) {
+			void *page = page_address(pte);
+			if (cache_ops_need_broadcast())
+				preempt_disable();
+			memset(page, 0, PAGE_SIZE);
+			clean_pte_table(page);
+			if (cache_ops_need_broadcast())
+				preempt_enable();
+		}
 		pgtable_page_ctor(pte);
 	}
 
-- 
1.7.4.4

[RFC/PATCH v3 3/7] ARM: ARM11 MPCore: {clean, flush}_pmd_entry are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
{clean,flush}_pmd_entry on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions around
PMD modifications and subsequent {clean,flush}_pmd_entry calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h  |    4 ++++
 arch/arm/include/asm/pgtable.h  |    9 +++++++++
 arch/arm/include/asm/smp_plat.h |    2 ++
 arch/arm/mm/idmap.c             |    5 +++++
 arch/arm/mm/ioremap.c           |    9 +++++++++
 arch/arm/mm/mmu.c               |   12 ++++++++++++
 6 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 92d3f27..ba8cf6a 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -122,9 +122,13 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
 	unsigned long prot)
 {
 	unsigned long pmdval = (pte + PTE_HWTABLE_OFF) | prot;
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	pmdp[0] = __pmd(pmdval);
 	pmdp[1] = __pmd(pmdval + 256 * sizeof(pte_t));
 	flush_pmd_entry(pmdp);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 /*
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 5750704..00068dc 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -13,6 +13,7 @@
 #include <linux/const.h>
 #include <asm-generic/4level-fixup.h>
 #include <asm/proc-fns.h>
+#include <asm/smp_plat.h>
 
 #ifndef CONFIG_MMU
 
@@ -313,16 +314,24 @@ extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
 
 #define copy_pmd(pmdpd,pmdps)		\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdpd[0] = pmdps[0];	\
 		pmdpd[1] = pmdps[1];	\
 		flush_pmd_entry(pmdpd);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 #define pmd_clear(pmdp)			\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdp[0] = __pmd(0);	\
 		pmdp[1] = __pmd(0);	\
 		clean_pmd_entry(pmdp);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 static inline pte_t *pmd_page_vaddr(pmd_t pmd)
diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index f24c1b9..5a8d3df 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -5,6 +5,7 @@
 #ifndef __ASMARM_SMP_PLAT_H
 #define __ASMARM_SMP_PLAT_H
 
+#ifndef	__ASSEMBLY__
 #include <asm/cputype.h>
 
 /*
@@ -42,5 +43,6 @@ static inline int cache_ops_need_broadcast(void)
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 1;
 }
 #endif
+#endif
 
 #endif
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 2be9139..c04face 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -3,17 +3,22 @@
 #include <asm/cputype.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include <asm/smp_plat.h>
 
 static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	unsigned long prot)
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	addr = (addr & PMD_MASK) | prot;
 	pmd[0] = __pmd(addr);
 	addr += SECTION_SIZE;
 	pmd[1] = __pmd(addr);
 	flush_pmd_entry(pmd);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 static void idmap_add_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ab50627..b56d78a 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -32,6 +32,7 @@
 #include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
 #include <asm/sizes.h>
+#include <asm/smp_plat.h>
 
 #include <asm/mach/map.h>
 #include "mm.h"
@@ -135,11 +136,15 @@ remap_area_sections(unsigned long virt, unsigned long pfn,
 	do {
 		pmd_t *pmd = pmd_offset(pgd, addr);
 
+		if (cache_ops_need_broadcast())
+			preempt_disable();
 		pmd[0] = __pmd(__pfn_to_phys(pfn) | type->prot_sect);
 		pfn += SZ_1M >> PAGE_SHIFT;
 		pmd[1] = __pmd(__pfn_to_phys(pfn) | type->prot_sect);
 		pfn += SZ_1M >> PAGE_SHIFT;
 		flush_pmd_entry(pmd);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
 
 		addr += PGDIR_SIZE;
 		pgd++;
@@ -172,9 +177,13 @@ remap_area_supersections(unsigned long virt, unsigned long pfn,
 		for (i = 0; i < 8; i++) {
 			pmd_t *pmd = pmd_offset(pgd, addr);
 
+			if (cache_ops_need_broadcast())
+				preempt_disable();
 			pmd[0] = __pmd(super_pmd_val);
 			pmd[1] = __pmd(super_pmd_val);
 			flush_pmd_entry(pmd);
+			if (cache_ops_need_broadcast())
+				preempt_enable();
 
 			addr += PGDIR_SIZE;
 			pgd++;
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 594d677..3c8253f 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
 		if (addr & SECTION_SIZE)
 			pmd++;
 
+		if (cache_ops_need_broadcast())
+			preempt_disable();
 		do {
 			*pmd = __pmd(phys | type->prot_sect);
 			phys += SECTION_SIZE;
 		} while (pmd++, addr += SECTION_SIZE, addr != end);
 
+		/* FIXME: Multiple PMD entries may be written above
+		 * but only one cache line, up to 8 PMDs depending
+		 * on the alignment of this mapping, is flushed below.
+		 * IFF this mapping spans >8MiB, then only the first
+		 * 8MiB worth of entries will be flushed.  Entries
+		 * above the 8MiB limit will not be flushed if I
+		 * read this correctly.
+		 */
 		flush_pmd_entry(p);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
 	} else {
 		/*
 		 * No need to loop; pte's aren't interested in the
-- 
1.7.4.4

[RFC/PATCH v3 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
clean_dcache_area on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions
around memory modifications and subsequent clean_dcache_area calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/plat-omap/iommu.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 34fc31e..59836d1 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -1014,8 +1014,12 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto err_pgd;
 	}
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	memset(p, 0, IOPGD_TABLE_SIZE);
 	clean_dcache_area(p, IOPGD_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 	obj->iopgd = p;
 
 	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
@@ -1069,7 +1073,13 @@ static struct platform_driver omap_iommu_driver = {
 
 static void iopte_cachep_ctor(void *iopte)
 {
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+	/* FIXME: This will not work on ARM11 MPCore.
+	 */
 	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 static int __init omap_iommu_init(void)
-- 
1.7.4.4

[RFC/PATCH v3 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The get_thread_info assembler macro is currently defined in
arch/arm/kernel/entry-header.S and used in the following
files:

	arch/arm/kernel/entry-armv.S
	arch/arm/kernel/entry-common.S
	arch/arm/vfp/vfphw.S
	arch/arm/vfp/entry.S

The first two cases above use #include "entry-header.S" while
the later two cases use #include "../kernel/entry-header.S"
to obtain the get_thread_info assembler macro definition.

Move the get_thread_info assembler macro definition into
arch/arm/include/asm/assember.h and clean up inclusion
of this macro definition to use #include <asm/assember.h>.

This change facilitates use of the get_thread_info assembler
macro in other assembler functions without more of the same
path relative include directives.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 29035e8..78397d0 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -23,6 +23,19 @@
 #include <asm/ptrace.h>
 #include <asm/domain.h>
 
+#ifndef CONFIG_THUMB2_KERNEL
+	.macro	get_thread_info, rd
+	mov	\rd, sp, lsr #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#else   /* CONFIG_THUMB2_KERNEL */
+	.macro	get_thread_info, rd
+	mov	\rd, sp
+	lsr	\rd, \rd, #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#endif  /* !CONFIG_THUMB2_KERNEL */
+
 /*
  * Endian independent macros for shifting bytes within registers.
  */
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..d03b6a9 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -108,11 +108,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp, lsr #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
@@ -148,12 +143,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp
-	lsr	\rd, \rd, #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 4fa9903..e475a5a 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -15,9 +15,12 @@
  *  r10 = thread_info structure
  *  lr  = failure return
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 ENTRY(do_vfp)
 #ifdef CONFIG_PREEMPT
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 2d30c7f..68ca5af 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -14,9 +14,12 @@
  * r10 points at the start of the private FP workspace in the thread structure
  * sp points to a struct pt_regs (as defined in include/asm/proc/ptrace.h)
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 	.macro	DBGSTR, str
 #ifdef DEBUG
-- 
1.7.4.4

[RFC/PATCH v3 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The DMA_CACHE_RWFO operations are not preempt safe.  If preemption
occurs immediately following a RWFO operation on a cache line of
CPU A, it is possible for task migration to occur on resume at
which point the subsequent cache maintenance operation on the
same cache line of CPU B will have no affect on the CPU A cache
line leading to inconsistent memory and/or cache state.  To prevent
this, disable preemption during RWFO operations.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/cache-v6.S |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 74c2e5a..9c44752 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -205,6 +205,12 @@ ENTRY(v6_flush_kern_dcache_area)
  */
 v6_dma_inv_range:
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]			@ read for ownership
 	strb	r2, [r0]			@ write for ownership
 #endif
@@ -241,6 +247,9 @@ v6_dma_inv_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+#endif
 	mov	pc, lr
 
 /*
@@ -249,6 +258,12 @@ v6_dma_inv_range:
  *	- end     - virtual end address of region
  */
 v6_dma_clean_range:
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
 #ifdef CONFIG_DMA_CACHE_RWFO
@@ -264,6 +279,9 @@ v6_dma_clean_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+#endif
 	mov	pc, lr
 
 /*
@@ -273,6 +291,12 @@ v6_dma_clean_range:
  */
 ENTRY(v6_dma_flush_range)
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]		@ read for ownership
 	strb	r2, [r0]		@ write for ownership
 #endif
@@ -292,6 +316,9 @@ ENTRY(v6_dma_flush_range)
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+#endif
 	mov	pc, lr
 
 /*
-- 
1.7.4.4

[RFC/PATCH v3 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in cpu_v6_set_pte_ext.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/proc-v6.S |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
index a923aa0..57b359d 100644
--- a/arch/arm/mm/proc-v6.S
+++ b/arch/arm/mm/proc-v6.S
@@ -122,7 +122,18 @@ ENTRY(cpu_v6_switch_mm)
 
 ENTRY(cpu_v6_set_pte_ext)
 #ifdef CONFIG_MMU
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	stmdb	sp!, {r4, r5}
+	get_thread_info r5
+	ldr	r4, [r5, #TI_PREEMPT]	@ get preempt count
+	add	r3, r4, #1		@ increment it
+	str	r3, [r5, #TI_PREEMPT]	@ disable preempt
+#endif
 	armv6_set_pte_ext cpu_v6
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	str	r4, [r5, #TI_PREEMPT]	@ restore preempt count
+	ldmia	sp!, {r4, r5}
+#endif
 #endif
 	mov	pc, lr
 
-- 
1.7.4.4

[RFC/PATCH 3/7] ARM: ARM11 MPCore: {clean,flush}_pmd_entry are not preempt safe

[Catalin Marinas]

On Fri, Oct 07, 2011 at 03:38:37AM +0100, gdavis at mvista.com wrote:
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 594d677..3c8253f 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
>  		if (addr & SECTION_SIZE)
>  			pmd++;
>  
> +		if (cache_ops_need_broadcast())
> +			preempt_disable();
>  		do {
>  			*pmd = __pmd(phys | type->prot_sect);
>  			phys += SECTION_SIZE;
>  		} while (pmd++, addr += SECTION_SIZE, addr != end);
>  
> +		/* FIXME: Multiple PMD entries may be written above
> +		 * but only one cache line, up to 8 PMDs depending
> +		 * on the alignment of this mapping, is flushed below.
> +		 * IFF this mapping spans >8MiB, then only the first
> +		 * 8MiB worth of entries will be flushed.  Entries
> +		 * above the 8MiB limit will not be flushed if I
> +		 * read this correctly.
> +		 */
>  		flush_pmd_entry(p);
> +		if (cache_ops_need_broadcast())
> +			preempt_enable();

My reading of the create_mapping() code is that alloc_init_pud() and
alloc_init_section() are called with a 2MB range only (that's 2
entries) given by pgd_addr_end().

-- 
Catalin

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[Catalin Marinas]

On Fri, Oct 07, 2011 at 03:38:39AM +0100, gdavis at mvista.com wrote:
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 29035e8..78397d0 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -23,6 +23,19 @@
>  #include <asm/ptrace.h>
>  #include <asm/domain.h>
>  
> +#ifndef CONFIG_THUMB2_KERNEL
> +	.macro	get_thread_info, rd
> +	mov	\rd, sp, lsr #13
> +	mov	\rd, \rd, lsl #13
> +	.endm
> +#else   /* CONFIG_THUMB2_KERNEL */
> +	.macro	get_thread_info, rd
> +	mov	\rd, sp
> +	lsr	\rd, \rd, #13
> +	mov	\rd, \rd, lsl #13
> +	.endm
> +#endif  /* !CONFIG_THUMB2_KERNEL */

We could even write some preempt_enable and preempt_disable asm macros,
I think it would simplify the code in the other files.

-- 
Catalin

[RFC/PATCH 3/7] ARM: ARM11 MPCore: {clean, flush}_pmd_entry are not preempt safe

[George G. Davis]

On Oct 11, 2011, at 5:53 AM, Catalin Marinas wrote:

> On Fri, Oct 07, 2011 at 03:38:37AM +0100, gdavis at mvista.com wrote:
>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index 594d677..3c8253f 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
>> 		if (addr & SECTION_SIZE)
>> 			pmd++;
>> 
>> +		if (cache_ops_need_broadcast())
>> +			preempt_disable();
>> 		do {
>> 			*pmd = __pmd(phys | type->prot_sect);
>> 			phys += SECTION_SIZE;
>> 		} while (pmd++, addr += SECTION_SIZE, addr != end);
>> 
>> +		/* FIXME: Multiple PMD entries may be written above
>> +		 * but only one cache line, up to 8 PMDs depending
>> +		 * on the alignment of this mapping, is flushed below.
>> +		 * IFF this mapping spans >8MiB, then only the first
>> +		 * 8MiB worth of entries will be flushed.  Entries
>> +		 * above the 8MiB limit will not be flushed if I
>> +		 * read this correctly.
>> +		 */
>> 		flush_pmd_entry(p);
>> +		if (cache_ops_need_broadcast())
>> +			preempt_enable();
> 
> My reading of the create_mapping() code is that alloc_init_pud() and
> alloc_init_section() are called with a 2MB range only (that's 2
> entries) given by pgd_addr_end().

You're correct of course.  I initially stuck that FIXME note in there more as
a reminder to review it more carefully.  Alas, in my haste, I submitted as-is
w/o checking parameter bounds passed from callers which do as you
say, so it's never more than 2MiB in size.  Thanks for the feedback.
I'll remove that note.

--
Regards,
George

> 
> -- 
> Catalin

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[gdavis at mvista.com]

On Tue, Oct 11, 2011 at 10:56:39AM +0100, Catalin Marinas wrote:
> On Fri, Oct 07, 2011 at 03:38:39AM +0100, gdavis at mvista.com wrote:
> > diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> > index 29035e8..78397d0 100644
> > --- a/arch/arm/include/asm/assembler.h
> > +++ b/arch/arm/include/asm/assembler.h
> > @@ -23,6 +23,19 @@
> >  #include <asm/ptrace.h>
> >  #include <asm/domain.h>
> >  
> > +#ifndef CONFIG_THUMB2_KERNEL
> > +	.macro	get_thread_info, rd
> > +	mov	\rd, sp, lsr #13
> > +	mov	\rd, \rd, lsl #13
> > +	.endm
> > +#else   /* CONFIG_THUMB2_KERNEL */
> > +	.macro	get_thread_info, rd
> > +	mov	\rd, sp
> > +	lsr	\rd, \rd, #13
> > +	mov	\rd, \rd, lsl #13
> > +	.endm
> > +#endif  /* !CONFIG_THUMB2_KERNEL */
> 
> We could even write some preempt_enable and preempt_disable asm macros,
> I think it would simplify the code in the other files.

Like so (based on above patch and not even compile tested):

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 78397d0..eaf4939 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -36,6 +36,20 @@
 	.endm
 #endif  /* !CONFIG_THUMB2_KERNEL */
 
+	.macro	preempt_disable, tsk, cnt
+	get_thread_info \tsk
+	ldr	\cnt, [\tsk, #TI_PREEMPT]
+	add	\cnt, \cnt, #1
+	str	\cnt, [\tsk, #TI_PREEMPT]
+	.endm
+
+	.macro	preempt_enable, tsk, cnt
+	get_thread_info \tsk
+	ldr	\cnt, [\tsk, #TI_PREEMPT]
+	sub	\cnt, \cnt, #1
+	str	\cnt, [\tsk, #TI_PREEMPT]
+	.endm
+
 /*
  * Endian independent macros for shifting bytes within registers.
  */


Not as efficient as it could be but I imagine the macros could
be written to support optional load of \tsk and/or optional \tmp
parameters to cover other common cases.

--
Regards,
George

> 
> -- 
> Catalin

[RFC/PATCH 3/7] ARM: ARM11 MPCore: {clean,flush}_pmd_entry are not preempt safe

[Russell King - ARM Linux]

On Tue, Oct 11, 2011 at 10:34:17PM -0400, George G. Davis wrote:
> 
> On Oct 11, 2011, at 5:53 AM, Catalin Marinas wrote:
> 
> > On Fri, Oct 07, 2011 at 03:38:37AM +0100, gdavis at mvista.com wrote:
> >> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> >> index 594d677..3c8253f 100644
> >> --- a/arch/arm/mm/mmu.c
> >> +++ b/arch/arm/mm/mmu.c
> >> @@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
> >> 		if (addr & SECTION_SIZE)
> >> 			pmd++;
> >> 
> >> +		if (cache_ops_need_broadcast())
> >> +			preempt_disable();
> >> 		do {
> >> 			*pmd = __pmd(phys | type->prot_sect);
> >> 			phys += SECTION_SIZE;
> >> 		} while (pmd++, addr += SECTION_SIZE, addr != end);
> >> 
> >> +		/* FIXME: Multiple PMD entries may be written above
> >> +		 * but only one cache line, up to 8 PMDs depending
> >> +		 * on the alignment of this mapping, is flushed below.
> >> +		 * IFF this mapping spans >8MiB, then only the first
> >> +		 * 8MiB worth of entries will be flushed.  Entries
> >> +		 * above the 8MiB limit will not be flushed if I
> >> +		 * read this correctly.
> >> +		 */
> >> 		flush_pmd_entry(p);
> >> +		if (cache_ops_need_broadcast())
> >> +			preempt_enable();
> > 
> > My reading of the create_mapping() code is that alloc_init_pud() and
> > alloc_init_section() are called with a 2MB range only (that's 2
> > entries) given by pgd_addr_end().
> 
> You're correct of course.  I initially stuck that FIXME note in there more as
> a reminder to review it more carefully.  Alas, in my haste, I submitted as-is
> w/o checking parameter bounds passed from callers which do as you
> say, so it's never more than 2MiB in size.  Thanks for the feedback.
> I'll remove that note.

Note also that the early bringup of MMU mappings (alloc_init_section
etc) are running before SMP bringup, so they're already bound to a
single CPU.  So these shouldn't need 'fixing'.

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[Russell King - ARM Linux]

On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 78397d0..eaf4939 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -36,6 +36,20 @@
>  	.endm
>  #endif  /* !CONFIG_THUMB2_KERNEL */
>  
> +	.macro	preempt_disable, tsk, cnt
> +	get_thread_info \tsk
> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> +	add	\cnt, \cnt, #1
> +	str	\cnt, [\tsk, #TI_PREEMPT]
> +	.endm
> +
> +	.macro	preempt_enable, tsk, cnt
> +	get_thread_info \tsk
> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> +	sub	\cnt, \cnt, #1
> +	str	\cnt, [\tsk, #TI_PREEMPT]
> +	.endm
> +
>  /*
>   * Endian independent macros for shifting bytes within registers.
>   */
> 
> 
> Not as efficient as it could be but I imagine the macros could
> be written to support optional load of \tsk and/or optional \tmp
> parameters to cover other common cases.

It's actually not that simple either: if you disable preemption, then you
need to check for a preempt event after re-enabling preemption.  The
C level code does this:

#define preempt_enable_no_resched() \
do { \
        barrier(); \
        dec_preempt_count(); \
} while (0)

#define preempt_check_resched() \
do { \
        if (unlikely(test_thread_flag(TIF_NEED_RESCHED))) \
                preempt_schedule(); \
} while (0)

#define preempt_enable() \
do { \
        preempt_enable_no_resched(); \
        barrier(); \
        preempt_check_resched(); \
} while (0)

Note that preempt_schedule() will check the preempt count itself and
return immediately if non-zero or irqs are disabled.

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[Catalin Marinas]

On Thu, Oct 13, 2011 at 03:34:20PM +0100, Russell King - ARM Linux wrote:
> On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
> > diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> > index 78397d0..eaf4939 100644
> > --- a/arch/arm/include/asm/assembler.h
> > +++ b/arch/arm/include/asm/assembler.h
> > @@ -36,6 +36,20 @@
> >  	.endm
> >  #endif  /* !CONFIG_THUMB2_KERNEL */
> >  
> > +	.macro	preempt_disable, tsk, cnt
> > +	get_thread_info \tsk
> > +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> > +	add	\cnt, \cnt, #1
> > +	str	\cnt, [\tsk, #TI_PREEMPT]
> > +	.endm
> > +
> > +	.macro	preempt_enable, tsk, cnt
> > +	get_thread_info \tsk
> > +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> > +	sub	\cnt, \cnt, #1
> > +	str	\cnt, [\tsk, #TI_PREEMPT]
> > +	.endm
> > +
> >  /*
> >   * Endian independent macros for shifting bytes within registers.
> >   */
> > 
> > 
> > Not as efficient as it could be but I imagine the macros could
> > be written to support optional load of \tsk and/or optional \tmp
> > parameters to cover other common cases.
> 
> It's actually not that simple either: if you disable preemption, then you
> need to check for a preempt event after re-enabling preemption.

That's not easily possible in assembly as calling a function would
corrupt some registers. Is there any disadvantage with just doing the
equivalent of preempt_enable_no_resched() for a few asm cases where this
is needed?

-- 
Catalin

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[Russell King - ARM Linux]

On Thu, Oct 13, 2011 at 03:49:23PM +0100, Catalin Marinas wrote:
> On Thu, Oct 13, 2011 at 03:34:20PM +0100, Russell King - ARM Linux wrote:
> > On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
> > > diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> > > index 78397d0..eaf4939 100644
> > > --- a/arch/arm/include/asm/assembler.h
> > > +++ b/arch/arm/include/asm/assembler.h
> > > @@ -36,6 +36,20 @@
> > >  	.endm
> > >  #endif  /* !CONFIG_THUMB2_KERNEL */
> > >  
> > > +	.macro	preempt_disable, tsk, cnt
> > > +	get_thread_info \tsk
> > > +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> > > +	add	\cnt, \cnt, #1
> > > +	str	\cnt, [\tsk, #TI_PREEMPT]
> > > +	.endm
> > > +
> > > +	.macro	preempt_enable, tsk, cnt
> > > +	get_thread_info \tsk
> > > +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> > > +	sub	\cnt, \cnt, #1
> > > +	str	\cnt, [\tsk, #TI_PREEMPT]
> > > +	.endm
> > > +
> > >  /*
> > >   * Endian independent macros for shifting bytes within registers.
> > >   */
> > > 
> > > 
> > > Not as efficient as it could be but I imagine the macros could
> > > be written to support optional load of \tsk and/or optional \tmp
> > > parameters to cover other common cases.
> > 
> > It's actually not that simple either: if you disable preemption, then you
> > need to check for a preempt event after re-enabling preemption.
> 
> That's not easily possible in assembly as calling a function would
> corrupt some registers. Is there any disadvantage with just doing the
> equivalent of preempt_enable_no_resched() for a few asm cases where this
> is needed?

It means you drop preemption reschedule events, which means that you can
no longer do hard-RT in any shape or form (because you're dropping the
reschedule for an indefinite amount of time.)

preempt_enable_no_resched() is fine if you're going to also put an
explicit check a bit later in the code path when it is convenient to
do so.

[RFC/PATCH 3/7] ARM: ARM11 MPCore: {clean, flush}_pmd_entry are not preempt safe

[George G. Davis]

On Oct 13, 2011, at 10:31 AM, Russell King - ARM Linux wrote:

> On Tue, Oct 11, 2011 at 10:34:17PM -0400, George G. Davis wrote:
>> 
>> On Oct 11, 2011, at 5:53 AM, Catalin Marinas wrote:
>> 
>>> On Fri, Oct 07, 2011 at 03:38:37AM +0100, gdavis at mvista.com wrote:
>>>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>>>> index 594d677..3c8253f 100644
>>>> --- a/arch/arm/mm/mmu.c
>>>> +++ b/arch/arm/mm/mmu.c
>>>> @@ -567,12 +567,24 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
>>>> 		if (addr & SECTION_SIZE)
>>>> 			pmd++;
>>>> 
>>>> +		if (cache_ops_need_broadcast())
>>>> +			preempt_disable();
>>>> 		do {
>>>> 			*pmd = __pmd(phys | type->prot_sect);
>>>> 			phys += SECTION_SIZE;
>>>> 		} while (pmd++, addr += SECTION_SIZE, addr != end);
>>>> 
>>>> +		/* FIXME: Multiple PMD entries may be written above
>>>> +		 * but only one cache line, up to 8 PMDs depending
>>>> +		 * on the alignment of this mapping, is flushed below.
>>>> +		 * IFF this mapping spans >8MiB, then only the first
>>>> +		 * 8MiB worth of entries will be flushed.  Entries
>>>> +		 * above the 8MiB limit will not be flushed if I
>>>> +		 * read this correctly.
>>>> +		 */
>>>> 		flush_pmd_entry(p);
>>>> +		if (cache_ops_need_broadcast())
>>>> +			preempt_enable();
>>> 
>>> My reading of the create_mapping() code is that alloc_init_pud() and
>>> alloc_init_section() are called with a 2MB range only (that's 2
>>> entries) given by pgd_addr_end().
>> 
>> You're correct of course.  I initially stuck that FIXME note in there more as
>> a reminder to review it more carefully.  Alas, in my haste, I submitted as-is
>> w/o checking parameter bounds passed from callers which do as you
>> say, so it's never more than 2MiB in size.  Thanks for the feedback.
>> I'll remove that note.
> 
> Note also that the early bringup of MMU mappings (alloc_init_section
> etc) are running before SMP bringup, so they're already bound to a
> single CPU.  So these shouldn't need 'fixing'.

Yep, I knew that and just went ahead and used a shot gun anyway.  I'll
drop cases which are already safe.

Thanks!

--
Regards,
George

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[George G. Davis]

On Oct 13, 2011, at 10:34 AM, Russell King - ARM Linux wrote:

> On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
>> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
>> index 78397d0..eaf4939 100644
>> --- a/arch/arm/include/asm/assembler.h
>> +++ b/arch/arm/include/asm/assembler.h
>> @@ -36,6 +36,20 @@
>> 	.endm
>> #endif  /* !CONFIG_THUMB2_KERNEL */
>> 
>> +	.macro	preempt_disable, tsk, cnt
>> +	get_thread_info \tsk
>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>> +	add	\cnt, \cnt, #1
>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>> +	.endm
>> +
>> +	.macro	preempt_enable, tsk, cnt
>> +	get_thread_info \tsk
>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>> +	sub	\cnt, \cnt, #1
>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>> +	.endm
>> +
>> /*
>>  * Endian independent macros for shifting bytes within registers.
>>  */
>> 
>> 
>> Not as efficient as it could be but I imagine the macros could
>> be written to support optional load of \tsk and/or optional \tmp
>> parameters to cover other common cases.
> 
> It's actually not that simple either: if you disable preemption, then you
> need to check for a preempt event after re-enabling preemption.  The
> C level code does this:
> 
> #define preempt_enable_no_resched() \
> do { \
>        barrier(); \
>        dec_preempt_count(); \
> } while (0)
> 
> #define preempt_check_resched() \
> do { \
>        if (unlikely(test_thread_flag(TIF_NEED_RESCHED))) \
>                preempt_schedule(); \
> } while (0)
> 
> #define preempt_enable() \
> do { \
>        preempt_enable_no_resched(); \
>        barrier(); \
>        preempt_check_resched(); \
> } while (0)
> 
> Note that preempt_schedule() will check the preempt count itself and
> return immediately if non-zero or irqs are disabled.

It would be easier to just insert preempt_disable/preempt_enable in
the cache and proc function call macros.  I did that in an earlier test
patch but moved it into the cache-v6.S and proc-v6.S files instead
since it only affects ARM11 MPCore as far as I'm aware.

--
Regards,
George

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[George G. Davis]

On Oct 13, 2011, at 10:49 AM, Catalin Marinas wrote:

> On Thu, Oct 13, 2011 at 03:34:20PM +0100, Russell King - ARM Linux wrote:
>> On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
>>> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
>>> index 78397d0..eaf4939 100644
>>> --- a/arch/arm/include/asm/assembler.h
>>> +++ b/arch/arm/include/asm/assembler.h
>>> @@ -36,6 +36,20 @@
>>> 	.endm
>>> #endif  /* !CONFIG_THUMB2_KERNEL */
>>> 
>>> +	.macro	preempt_disable, tsk, cnt
>>> +	get_thread_info \tsk
>>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>>> +	add	\cnt, \cnt, #1
>>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>>> +	.endm
>>> +
>>> +	.macro	preempt_enable, tsk, cnt
>>> +	get_thread_info \tsk
>>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>>> +	sub	\cnt, \cnt, #1
>>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>>> +	.endm
>>> +
>>> /*
>>>  * Endian independent macros for shifting bytes within registers.
>>>  */
>>> 
>>> 
>>> Not as efficient as it could be but I imagine the macros could
>>> be written to support optional load of \tsk and/or optional \tmp
>>> parameters to cover other common cases.
>> 
>> It's actually not that simple either: if you disable preemption, then you
>> need to check for a preempt event after re-enabling preemption.
> 
> That's not easily possible in assembly as calling a function would
> corrupt some registers.

Since the preempt_enable is done at the end of all current v6 funcs
in this series, it was easy to implement.  I'll include those changes
in an update.


Thanks!

--
Regards,
George

> Is there any disadvantage with just doing the
> equivalent of preempt_enable_no_resched() for a few asm cases where this
> is needed?
> 
> -- 
> Catalin

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[George G. Davis]

On Oct 13, 2011, at 10:53 AM, Russell King - ARM Linux wrote:

> On Thu, Oct 13, 2011 at 03:49:23PM +0100, Catalin Marinas wrote:
>> On Thu, Oct 13, 2011 at 03:34:20PM +0100, Russell King - ARM Linux wrote:
>>> On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
>>>> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
>>>> index 78397d0..eaf4939 100644
>>>> --- a/arch/arm/include/asm/assembler.h
>>>> +++ b/arch/arm/include/asm/assembler.h
>>>> @@ -36,6 +36,20 @@
>>>> 	.endm
>>>> #endif  /* !CONFIG_THUMB2_KERNEL */
>>>> 
>>>> +	.macro	preempt_disable, tsk, cnt
>>>> +	get_thread_info \tsk
>>>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	add	\cnt, \cnt, #1
>>>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	.endm
>>>> +
>>>> +	.macro	preempt_enable, tsk, cnt
>>>> +	get_thread_info \tsk
>>>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	sub	\cnt, \cnt, #1
>>>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	.endm
>>>> +
>>>> /*
>>>>  * Endian independent macros for shifting bytes within registers.
>>>>  */
>>>> 
>>>> 
>>>> Not as efficient as it could be but I imagine the macros could
>>>> be written to support optional load of \tsk and/or optional \tmp
>>>> parameters to cover other common cases.
>>> 
>>> It's actually not that simple either: if you disable preemption, then you
>>> need to check for a preempt event after re-enabling preemption.
>> 
>> That's not easily possible in assembly as calling a function would
>> corrupt some registers. Is there any disadvantage with just doing the
>> equivalent of preempt_enable_no_resched() for a few asm cases where this
>> is needed?
> 
> It means you drop preemption reschedule events, which means that you can
> no longer do hard-RT in any shape or form (because you're dropping the
> reschedule for an indefinite amount of time.)

I ignored that due to laziness in this series.  I figured it was a small window
and another interrupt will come along soon enough but yes, it does impact
preempt latency.

> preempt_enable_no_resched() is fine if you're going to also put an
> explicit check a bit later in the code path when it is convenient to
> do so.

I implemented it for all cases here and will submit an update.

Thanks!

--
Regards,
George

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[Nicolas Pitre]

On Thu, 13 Oct 2011, George G. Davis wrote:

> 
> On Oct 13, 2011, at 10:34 AM, Russell King - ARM Linux wrote:
> 
> > On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
> >> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> >> index 78397d0..eaf4939 100644
> >> --- a/arch/arm/include/asm/assembler.h
> >> +++ b/arch/arm/include/asm/assembler.h
> >> @@ -36,6 +36,20 @@
> >> 	.endm
> >> #endif  /* !CONFIG_THUMB2_KERNEL */
> >> 
> >> +	.macro	preempt_disable, tsk, cnt
> >> +	get_thread_info \tsk
> >> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> >> +	add	\cnt, \cnt, #1
> >> +	str	\cnt, [\tsk, #TI_PREEMPT]
> >> +	.endm
> >> +
> >> +	.macro	preempt_enable, tsk, cnt
> >> +	get_thread_info \tsk
> >> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
> >> +	sub	\cnt, \cnt, #1
> >> +	str	\cnt, [\tsk, #TI_PREEMPT]
> >> +	.endm
> >> +
> >> /*
> >>  * Endian independent macros for shifting bytes within registers.
> >>  */
> >> 
> >> 
> >> Not as efficient as it could be but I imagine the macros could
> >> be written to support optional load of \tsk and/or optional \tmp
> >> parameters to cover other common cases.
> > 
> > It's actually not that simple either: if you disable preemption, then you
> > need to check for a preempt event after re-enabling preemption.  The
> > C level code does this:
> > 
> > #define preempt_enable_no_resched() \
> > do { \
> >        barrier(); \
> >        dec_preempt_count(); \
> > } while (0)
> > 
> > #define preempt_check_resched() \
> > do { \
> >        if (unlikely(test_thread_flag(TIF_NEED_RESCHED))) \
> >                preempt_schedule(); \
> > } while (0)
> > 
> > #define preempt_enable() \
> > do { \
> >        preempt_enable_no_resched(); \
> >        barrier(); \
> >        preempt_check_resched(); \
> > } while (0)
> > 
> > Note that preempt_schedule() will check the preempt count itself and
> > return immediately if non-zero or irqs are disabled.
> 
> It would be easier to just insert preempt_disable/preempt_enable in
> the cache and proc function call macros.  I did that in an earlier test
> patch but moved it into the cache-v6.S and proc-v6.S files instead
> since it only affects ARM11 MPCore as far as I'm aware.

Alternately, why don't you simply disable IRQs locally?  No preemption 
can happen until enabled again, just like preempt_disable() would do, 
and the IRQ off period should be short enough not to visibly affect IRQ 
latency.  And that is easily done in assembly too, without preventing 
rescheduling if needed afterwards.


Nicolas

[RFC/PATCH 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[George G. Davis]

On Oct 13, 2011, at 10:54 PM, Nicolas Pitre wrote:

> On Thu, 13 Oct 2011, George G. Davis wrote:
> 
>> 
>> On Oct 13, 2011, at 10:34 AM, Russell King - ARM Linux wrote:
>> 
>>> On Wed, Oct 12, 2011 at 02:04:33AM -0400, gdavis at mvista.com wrote:
>>>> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
>>>> index 78397d0..eaf4939 100644
>>>> --- a/arch/arm/include/asm/assembler.h
>>>> +++ b/arch/arm/include/asm/assembler.h
>>>> @@ -36,6 +36,20 @@
>>>> 	.endm
>>>> #endif  /* !CONFIG_THUMB2_KERNEL */
>>>> 
>>>> +	.macro	preempt_disable, tsk, cnt
>>>> +	get_thread_info \tsk
>>>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	add	\cnt, \cnt, #1
>>>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	.endm
>>>> +
>>>> +	.macro	preempt_enable, tsk, cnt
>>>> +	get_thread_info \tsk
>>>> +	ldr	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	sub	\cnt, \cnt, #1
>>>> +	str	\cnt, [\tsk, #TI_PREEMPT]
>>>> +	.endm
>>>> +
>>>> /*
>>>> * Endian independent macros for shifting bytes within registers.
>>>> */
>>>> 
>>>> 
>>>> Not as efficient as it could be but I imagine the macros could
>>>> be written to support optional load of \tsk and/or optional \tmp
>>>> parameters to cover other common cases.
>>> 
>>> It's actually not that simple either: if you disable preemption, then you
>>> need to check for a preempt event after re-enabling preemption.  The
>>> C level code does this:
>>> 
>>> #define preempt_enable_no_resched() \
>>> do { \
>>>       barrier(); \
>>>       dec_preempt_count(); \
>>> } while (0)
>>> 
>>> #define preempt_check_resched() \
>>> do { \
>>>       if (unlikely(test_thread_flag(TIF_NEED_RESCHED))) \
>>>               preempt_schedule(); \
>>> } while (0)
>>> 
>>> #define preempt_enable() \
>>> do { \
>>>       preempt_enable_no_resched(); \
>>>       barrier(); \
>>>       preempt_check_resched(); \
>>> } while (0)
>>> 
>>> Note that preempt_schedule() will check the preempt count itself and
>>> return immediately if non-zero or irqs are disabled.
>> 
>> It would be easier to just insert preempt_disable/preempt_enable in
>> the cache and proc function call macros.  I did that in an earlier test
>> patch but moved it into the cache-v6.S and proc-v6.S files instead
>> since it only affects ARM11 MPCore as far as I'm aware.
> 
> Alternately, why don't you simply disable IRQs locally?  No preemption 
> can happen until enabled again, just like preempt_disable() would do, 
> and the IRQ off period should be short enough not to visibly affect IRQ 
> latency.  And that is easily done in assembly too, without preventing 
> rescheduling if needed afterwards.

I didn't want to impact interrupt latency but yes, that would work as well
and is easier to implement.  : )

--
Regards,
George

> 
> 
> Nicolas

[RFC/PATCH v4 0/7] ARM11 MPCore: preemption/task migration cache coherency fixups

[gdavis at mvista.com]

Greetings,

On ARM11 MPCore, the "SCU does not handle coherency consequences of CP15
cache operations" [1]. ?So cache maintenance functions have to insure
that memory is globally consistent. ?Although the current Linux kernel
works reasonably well on ARM11 MPCore machines, PREEMPT stress testing,
e.g. parallel module loading and hackbench, results in crashes which
exhibit non-sense oops traces where machine state does not make sense
relative to the code executing at the time of the oops.

Review and analysis of the various ARM11 MPCore cache maintenance
functions reveal that there are a number critical sections in which
ARM11 MPCore caches and/or memory may become inconsistent, i.e. a
cache line on CPU A contains a modified entry but preemption and task
migration occurs after which the same cache line is cleaned/flushed
on CPU B. ?This can obviously lead to inconsistent memory and/or
cache state as cache ops on ARM11 MPCore are non-coherent.

The following is a partial series of ARM11 MPCore preemption/task
migration fixes to resolve cache coherency problems on these machines:

George G. Davis (6):
      ARM: ARM11 MPCore: pte_alloc_one{,_kernel} are not preempt safe
      ARM: ARM11 MPCore: {clean,flush}_pmd_entry are not preempt safe
      ARM: ARM11 MPCore: clean_dcache_area is not preempt safe
      ARM: Move get_thread_info macro definition to <asm/assembler.h>
      ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe
      ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

Konstantin Baidarov (1):
      ARM: ARM11 MPCore: pgd_alloc is not preempt safe

 arch/arm/include/asm/assembler.h |   13 ++++++++++++
 arch/arm/include/asm/pgalloc.h   |   28 ++++++++++++++++++++++----
 arch/arm/include/asm/pgtable.h   |    9 ++++++++
 arch/arm/include/asm/smp_plat.h  |    2 +
 arch/arm/kernel/entry-header.S   |   11 ----------
 arch/arm/mm/cache-v6.S           |   39 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/idmap.c              |    5 ++++
 arch/arm/mm/pgd.c                |    7 ++++++
 arch/arm/mm/proc-v6.S            |   15 ++++++++++++++
 arch/arm/plat-omap/iommu.c       |   10 +++++++++
 arch/arm/vfp/entry.S             |    5 +++-
 arch/arm/vfp/vfphw.S             |    5 +++-
 12 files changed, 131 insertions(+), 18 deletions(-)

ChangeLog:

V2:
- Substitute {get,put}_cpu() with preempt_{disable,enable}().
- Fixed preempt {dis,en}able assembler code sequences to not
  use r11 since it is reserved for frame pointer use.  Also
  optimised these sequences to use r2, r3; ip scratch registers
  in most cases to eliminate stack push/pop.  In one case,
  cpu_v6_set_pte_ext, there are only two scratch registers
  available, r3 and ip.  However, both of these are used within
  the armv6_set_pte_ext macro.  So for this case, r3 is used
  as a temporary scratch when disabling preemption and r4 and
  r5 are pushed/popped as needed for other uses to avoid
  conflict with scratch register usage in armv6_set_pte_ext.
- Remove incorrect use of ALT_SMP macros in cpu_v6_set_pte_ext,
  making the preempt {dis,en}able assembler code sequences
  compile time dependent upon CONFIG_SMP instead.  This code
  is safe on UP machines anyway.
V3:
- Fix HIGHMEM breakage.
V4:
- Drop preempt_{disable,enable}() around calls to flush_pmd_entry()
  in remap_area_sections() and remap_area_supersections() in
  file arch/arm/mm/ioremap.c since these functions are not used
  on SMP machines (they're enclosed within #ifndef SMP/#endif)
- Drop preempt_{disable,enable}() around call to flush_pmd_entry()
  in alloc_init_section() in file arch/arm/mm/mmu.c since this
  function is called during early kernel initialization during
  which time preemption and task migration is not possible.  Also
  removed bogus FIXME comment as part of this change.
- Added calls to preempt_schedule when re-enabling preemption
  in various arch/arm/mm/{cache,proc}-v6.S functions which need
  preemption disabled due task migration issues.

The above changes are among the "low hanging fruit" where the
workarounds are relatively low impact from a performance and/or
implementation effort perspective. ?I'm still working on fixes
for the harder problems. ?Also, I believe that Catalin's "ARM: Allow
lazy cache flushing on ARM11MPCore" [2][3] is required for ARM11
MPCore machines and would like to see that or similar/alternative
solution applied.

Comments/feedback greatly appreciated.

TIA!

--
Regards,
George

[1] http://infocenter.arm.com/help/topic/com.arm.doc.dai0228a/index.html#arm_toc9
[2] http://www.spinics.net/lists/arm-kernel/msg129403.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-May/014990.html

[RFC/PATCH v4 1/7] ARM: ARM11 MPCore: pgd_alloc is not preempt safe

[gdavis at mvista.com]

From: Konstantin Baidarov <kbaidarov@mvista.com>

If preemption and subsequent task migration occurs during a call to
pgd_alloc on ARM11 MPCore machines, global memory state can become
inconsistent.  To prevent inconsistent memory state on these
machines, disable preemption around initialization and flushing
of new PGDs.

Signed-off-by: Konstantin Baidarov <kbaidarov@mvista.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/pgd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index b2027c1..e608981 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -14,6 +14,7 @@
 #include <asm/pgalloc.h>
 #include <asm/page.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #include "mm.h"
 
@@ -31,6 +32,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 	if (!new_pgd)
 		goto no_pgd;
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+
 	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
 
 	/*
@@ -42,6 +46,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 
 	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
 
+	if (cache_ops_need_broadcast())
+		preempt_enable();
+
 	if (!vectors_high()) {
 		/*
 		 * On ARM, first page must always be allocated since it
-- 
1.7.4.4

[RFC/PATCH v4 2/7] ARM: ARM11 MPCore: pte_alloc_one{, _kernel} are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during a call to
pte_alloc_one{,_kernel} on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption around initialization and flushing
of new PTEs.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 22de005..92d3f27 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -17,6 +17,7 @@
 #include <asm/processor.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #define check_pgt_cache()		do { } while (0)
 
@@ -35,7 +36,7 @@
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
 extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
 
-#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
+#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
 
 static inline void clean_pte_table(pte_t *pte)
 {
@@ -64,8 +65,14 @@ pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
 	pte_t *pte;
 
 	pte = (pte_t *)__get_free_page(PGALLOC_GFP);
-	if (pte)
+	if (pte) {
+		if (cache_ops_need_broadcast())
+			preempt_disable();
+		memset(pte, 0, PAGE_SIZE);
 		clean_pte_table(pte);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
+	}
 
 	return pte;
 }
@@ -76,13 +83,20 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 	struct page *pte;
 
 #ifdef CONFIG_HIGHPTE
-	pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
+	pte = alloc_pages(PGALLOC_GFP | __GFP_ZERO | __GFP_HIGHMEM, 0);
 #else
 	pte = alloc_pages(PGALLOC_GFP, 0);
 #endif
 	if (pte) {
-		if (!PageHighMem(pte))
-			clean_pte_table(page_address(pte));
+		if (!PageHighMem(pte)) {
+			void *page = page_address(pte);
+			if (cache_ops_need_broadcast())
+				preempt_disable();
+			memset(page, 0, PAGE_SIZE);
+			clean_pte_table(page);
+			if (cache_ops_need_broadcast())
+				preempt_enable();
+		}
 		pgtable_page_ctor(pte);
 	}
 
-- 
1.7.4.4

[RFC/PATCH v4 3/7] ARM: ARM11 MPCore: {clean, flush}_pmd_entry are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
{clean,flush}_pmd_entry on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions around
PMD modifications and subsequent {clean,flush}_pmd_entry calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h  |    4 ++++
 arch/arm/include/asm/pgtable.h  |    9 +++++++++
 arch/arm/include/asm/smp_plat.h |    2 ++
 arch/arm/mm/idmap.c             |    5 +++++
 4 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 92d3f27..ba8cf6a 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -122,9 +122,13 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
 	unsigned long prot)
 {
 	unsigned long pmdval = (pte + PTE_HWTABLE_OFF) | prot;
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	pmdp[0] = __pmd(pmdval);
 	pmdp[1] = __pmd(pmdval + 256 * sizeof(pte_t));
 	flush_pmd_entry(pmdp);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 /*
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 5750704..00068dc 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -13,6 +13,7 @@
 #include <linux/const.h>
 #include <asm-generic/4level-fixup.h>
 #include <asm/proc-fns.h>
+#include <asm/smp_plat.h>
 
 #ifndef CONFIG_MMU
 
@@ -313,16 +314,24 @@ extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
 
 #define copy_pmd(pmdpd,pmdps)		\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdpd[0] = pmdps[0];	\
 		pmdpd[1] = pmdps[1];	\
 		flush_pmd_entry(pmdpd);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 #define pmd_clear(pmdp)			\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdp[0] = __pmd(0);	\
 		pmdp[1] = __pmd(0);	\
 		clean_pmd_entry(pmdp);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 static inline pte_t *pmd_page_vaddr(pmd_t pmd)
diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index f24c1b9..5a8d3df 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -5,6 +5,7 @@
 #ifndef __ASMARM_SMP_PLAT_H
 #define __ASMARM_SMP_PLAT_H
 
+#ifndef	__ASSEMBLY__
 #include <asm/cputype.h>
 
 /*
@@ -42,5 +43,6 @@ static inline int cache_ops_need_broadcast(void)
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 1;
 }
 #endif
+#endif
 
 #endif
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 2be9139..c04face 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -3,17 +3,22 @@
 #include <asm/cputype.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include <asm/smp_plat.h>
 
 static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	unsigned long prot)
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	addr = (addr & PMD_MASK) | prot;
 	pmd[0] = __pmd(addr);
 	addr += SECTION_SIZE;
 	pmd[1] = __pmd(addr);
 	flush_pmd_entry(pmd);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 static void idmap_add_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
-- 
1.7.4.4

[RFC/PATCH v4 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
clean_dcache_area on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions
around memory modifications and subsequent clean_dcache_area calls.

Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/plat-omap/iommu.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 34fc31e..59836d1 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -1014,8 +1014,12 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto err_pgd;
 	}
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	memset(p, 0, IOPGD_TABLE_SIZE);
 	clean_dcache_area(p, IOPGD_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 	obj->iopgd = p;
 
 	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
@@ -1069,7 +1073,13 @@ static struct platform_driver omap_iommu_driver = {
 
 static void iopte_cachep_ctor(void *iopte)
 {
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+	/* FIXME: This will not work on ARM11 MPCore.
+	 */
 	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 static int __init omap_iommu_init(void)
-- 
1.7.4.4

[RFC/PATCH v4 5/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The get_thread_info assembler macro is currently defined in
arch/arm/kernel/entry-header.S and used in the following
files:

	arch/arm/kernel/entry-armv.S
	arch/arm/kernel/entry-common.S
	arch/arm/vfp/vfphw.S
	arch/arm/vfp/entry.S

The first two cases above use #include "entry-header.S" while
the later two cases use #include "../kernel/entry-header.S"
to obtain the get_thread_info assembler macro definition.

Move the get_thread_info assembler macro definition into
arch/arm/include/asm/assember.h and clean up inclusion
of this macro definition to use #include <asm/assember.h>.

This change facilitates use of the get_thread_info assembler
macro in other assembler functions without more of the same
path relative include directives.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 29035e8..78397d0 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -23,6 +23,19 @@
 #include <asm/ptrace.h>
 #include <asm/domain.h>
 
+#ifndef CONFIG_THUMB2_KERNEL
+	.macro	get_thread_info, rd
+	mov	\rd, sp, lsr #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#else   /* CONFIG_THUMB2_KERNEL */
+	.macro	get_thread_info, rd
+	mov	\rd, sp
+	lsr	\rd, \rd, #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#endif  /* !CONFIG_THUMB2_KERNEL */
+
 /*
  * Endian independent macros for shifting bytes within registers.
  */
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..d03b6a9 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -108,11 +108,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp, lsr #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
@@ -148,12 +143,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp
-	lsr	\rd, \rd, #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 4fa9903..e475a5a 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -15,9 +15,12 @@
  *  r10 = thread_info structure
  *  lr  = failure return
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 ENTRY(do_vfp)
 #ifdef CONFIG_PREEMPT
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 2d30c7f..68ca5af 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -14,9 +14,12 @@
  * r10 points at the start of the private FP workspace in the thread structure
  * sp points to a struct pt_regs (as defined in include/asm/proc/ptrace.h)
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 	.macro	DBGSTR, str
 #ifdef DEBUG
-- 
1.7.4.4

[RFC/PATCH v4 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The DMA_CACHE_RWFO operations are not preempt safe.  If preemption
occurs immediately following a RWFO operation on a cache line of
CPU A, it is possible for task migration to occur on resume at
which point the subsequent cache maintenance operation on the
same cache line of CPU B will have no affect on the CPU A cache
line leading to inconsistent memory and/or cache state.  To prevent
this, disable preemption during RWFO operations.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/cache-v6.S |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 74c2e5a..a5cc9a0 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -205,6 +205,12 @@ ENTRY(v6_flush_kern_dcache_area)
  */
 v6_dma_inv_range:
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]			@ read for ownership
 	strb	r2, [r0]			@ write for ownership
 #endif
@@ -241,6 +247,13 @@ v6_dma_inv_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	teq	r3, #0				@ preempt count == 0?
+	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ ret via preempt_schedule
+#endif
 	mov	pc, lr
 
 /*
@@ -249,6 +262,12 @@ v6_dma_inv_range:
  *	- end     - virtual end address of region
  */
 v6_dma_clean_range:
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
 #ifdef CONFIG_DMA_CACHE_RWFO
@@ -264,6 +283,13 @@ v6_dma_clean_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	teq	r3, #0				@ preempt count == 0?
+	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ ret via preempt_schedule
+#endif
 	mov	pc, lr
 
 /*
@@ -273,6 +299,12 @@ v6_dma_clean_range:
  */
 ENTRY(v6_dma_flush_range)
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]		@ read for ownership
 	strb	r2, [r0]		@ write for ownership
 #endif
@@ -292,6 +324,13 @@ ENTRY(v6_dma_flush_range)
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	teq	r3, #0				@ preempt count == 0?
+	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ ret via preempt_schedule
+#endif
 	mov	pc, lr
 
 /*
-- 
1.7.4.4

[RFC/PATCH v4 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in cpu_v6_set_pte_ext.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/proc-v6.S |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
index a923aa0..be0ab8d 100644
--- a/arch/arm/mm/proc-v6.S
+++ b/arch/arm/mm/proc-v6.S
@@ -122,7 +122,22 @@ ENTRY(cpu_v6_switch_mm)
 
 ENTRY(cpu_v6_set_pte_ext)
 #ifdef CONFIG_MMU
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	stmdb	sp!, {r4, r5}
+	get_thread_info r5
+	ldr	r4, [r5, #TI_PREEMPT]	@ get preempt count
+	add	r3, r4, #1		@ increment it
+	str	r3, [r5, #TI_PREEMPT]	@ disable preempt
+#endif
 	armv6_set_pte_ext cpu_v6
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	str	r4, [r5, #TI_PREEMPT]	@ restore preempt count
+	teq	r4, #0			@ preempt count == 0?
+	ldreq	r4, [r5, #TI_FLAGS]	@ load flags if yes
+	tst	r4, #_TIF_NEED_RESCHED	@ need resched?
+	ldmia	sp!, {r4, r5}
+	bne	preempt_schedule	@ ret via preempt_schedule
+#endif
 #endif
 	mov	pc, lr
 
-- 
1.7.4.4

[RFC/PATCH v4 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[Tony Lindgren]

Hi,

* gdavis at mvista.com <gdavis@mvista.com> [111018 06:13]:
> From: George G. Davis <gdavis@mvista.com>
> 
> If preemption and subsequent task migration occurs during calls to
> clean_dcache_area on ARM11 MPCore machines, global memory state
> can become inconsistent.  To prevent inconsistent memory state on
> these machines, disable preemption in callers of these functions
> around memory modifications and subsequent clean_dcache_area calls.

This one needs to be refreshed against what's queued in for-next as
Ohad has moved most of the iommu code into drivers. I've added
Ohad to Cc as well.

Regards,

Tony

> 
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: George G. Davis <gdavis@mvista.com>
> ---
>  arch/arm/plat-omap/iommu.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
> index 34fc31e..59836d1 100644
> --- a/arch/arm/plat-omap/iommu.c
> +++ b/arch/arm/plat-omap/iommu.c
> @@ -1014,8 +1014,12 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
>  		err = -ENOMEM;
>  		goto err_pgd;
>  	}
> +	if (cache_ops_need_broadcast())
> +		preempt_disable();
>  	memset(p, 0, IOPGD_TABLE_SIZE);
>  	clean_dcache_area(p, IOPGD_TABLE_SIZE);
> +	if (cache_ops_need_broadcast())
> +		preempt_enable();
>  	obj->iopgd = p;
>  
>  	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
> @@ -1069,7 +1073,13 @@ static struct platform_driver omap_iommu_driver = {
>  
>  static void iopte_cachep_ctor(void *iopte)
>  {
> +	if (cache_ops_need_broadcast())
> +		preempt_disable();
> +	/* FIXME: This will not work on ARM11 MPCore.
> +	 */
>  	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
> +	if (cache_ops_need_broadcast())
> +		preempt_enable();
>  }
>  
>  static int __init omap_iommu_init(void)
> -- 
> 1.7.4.4
> 

[RFC/PATCH v4 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[George G. Davis]

Hello Tony/Ohad,

On Oct 18, 2011, at 1:08 PM, Tony Lindgren wrote:

> Hi,
> 
> * gdavis at mvista.com <gdavis@mvista.com> [111018 06:13]:
>> From: George G. Davis <gdavis@mvista.com>
>> 
>> If preemption and subsequent task migration occurs during calls to
>> clean_dcache_area on ARM11 MPCore machines, global memory state
>> can become inconsistent.  To prevent inconsistent memory state on
>> these machines, disable preemption in callers of these functions
>> around memory modifications and subsequent clean_dcache_area calls.
> 
> This one needs to be refreshed against what's queued in for-next as
> Ohad has moved most of the iommu code into drivers. I've added
> Ohad to Cc as well.

If you do not use ARM11 MPCore with those drivers, then we can drop this altogether.

Thanks!

--
Regards,
George
 
> 
> Regards,
> 
> Tony
> 
>> 
>> Cc: Tony Lindgren <tony@atomide.com>
>> Signed-off-by: George G. Davis <gdavis@mvista.com>
>> ---
>> arch/arm/plat-omap/iommu.c |   10 ++++++++++
>> 1 files changed, 10 insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
>> index 34fc31e..59836d1 100644
>> --- a/arch/arm/plat-omap/iommu.c
>> +++ b/arch/arm/plat-omap/iommu.c
>> @@ -1014,8 +1014,12 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
>> 		err = -ENOMEM;
>> 		goto err_pgd;
>> 	}
>> +	if (cache_ops_need_broadcast())
>> +		preempt_disable();
>> 	memset(p, 0, IOPGD_TABLE_SIZE);
>> 	clean_dcache_area(p, IOPGD_TABLE_SIZE);
>> +	if (cache_ops_need_broadcast())
>> +		preempt_enable();
>> 	obj->iopgd = p;
>> 
>> 	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
>> @@ -1069,7 +1073,13 @@ static struct platform_driver omap_iommu_driver = {
>> 
>> static void iopte_cachep_ctor(void *iopte)
>> {
>> +	if (cache_ops_need_broadcast())
>> +		preempt_disable();
>> +	/* FIXME: This will not work on ARM11 MPCore.
>> +	 */
>> 	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
>> +	if (cache_ops_need_broadcast())
>> +		preempt_enable();
>> }
>> 
>> static int __init omap_iommu_init(void)
>> -- 
>> 1.7.4.4
>> 

[RFC/PATCH v4 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[Tony Lindgren]

* George G. Davis <gdavis@mvista.com> [111018 09:56]:
> Hello Tony/Ohad,
> 
> On Oct 18, 2011, at 1:08 PM, Tony Lindgren wrote:
> 
> > Hi,
> > 
> > * gdavis at mvista.com <gdavis@mvista.com> [111018 06:13]:
> >> From: George G. Davis <gdavis@mvista.com>
> >> 
> >> If preemption and subsequent task migration occurs during calls to
> >> clean_dcache_area on ARM11 MPCore machines, global memory state
> >> can become inconsistent.  To prevent inconsistent memory state on
> >> these machines, disable preemption in callers of these functions
> >> around memory modifications and subsequent clean_dcache_area calls.
> > 
> > This one needs to be refreshed against what's queued in for-next as
> > Ohad has moved most of the iommu code into drivers. I've added
> > Ohad to Cc as well.
> 
> If you do not use ARM11 MPCore with those drivers, then we can drop this altogether.

No MPCore omaps available AFAIK, so that sounds OK to me.

Tony

[RFC/PATCH v4 4/7] ARM: ARM11 MPCore: clean_dcache_area is not preempt safe

[George G. Davis]

On Oct 18, 2011, at 1:43 PM, Tony Lindgren wrote:

> * George G. Davis <gdavis@mvista.com> [111018 09:56]:
>> Hello Tony/Ohad,
>> 
>> On Oct 18, 2011, at 1:08 PM, Tony Lindgren wrote:
>> 
>>> Hi,
>>> 
>>> * gdavis at mvista.com <gdavis@mvista.com> [111018 06:13]:
>>>> From: George G. Davis <gdavis@mvista.com>
>>>> 
>>>> If preemption and subsequent task migration occurs during calls to
>>>> clean_dcache_area on ARM11 MPCore machines, global memory state
>>>> can become inconsistent.  To prevent inconsistent memory state on
>>>> these machines, disable preemption in callers of these functions
>>>> around memory modifications and subsequent clean_dcache_area calls.
>>> 
>>> This one needs to be refreshed against what's queued in for-next as
>>> Ohad has moved most of the iommu code into drivers. I've added
>>> Ohad to Cc as well.
>> 
>> If you do not use ARM11 MPCore with those drivers, then we can drop this altogether.
> 
> No MPCore omaps available AFAIK, so that sounds OK to me.

Good, I'll drop it then.

Thanks!

--
Regards,
George

> 
> Tony

[RFC/PATCH v4 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[Nicolas Pitre]

On Tue, 18 Oct 2011, gdavis at mvista.com wrote:

> @@ -241,6 +247,13 @@ v6_dma_inv_range:
>  	blo	1b
>  	mov	r0, #0
>  	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
> +#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
> +	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
> +	teq	r3, #0				@ preempt count == 0?
> +	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
> +	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
> +	bne	preempt_schedule		@ ret via preempt_schedule

This is buggy.  If the preempt count is _not_ zero, you end up not 
loading the TI_FLAGS bits and testing _TIF_NEED_RESCHED against that 
non-zero preempt count.


Nicolas

[RFC/PATCH v4 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[Nicolas Pitre]

On Tue, 18 Oct 2011, gdavis at mvista.com wrote:

> @@ -122,7 +122,22 @@ ENTRY(cpu_v6_switch_mm)
>  
>  ENTRY(cpu_v6_set_pte_ext)
>  #ifdef CONFIG_MMU
> +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> +	stmdb	sp!, {r4, r5}
> +	get_thread_info r5
> +	ldr	r4, [r5, #TI_PREEMPT]	@ get preempt count
> +	add	r3, r4, #1		@ increment it
> +	str	r3, [r5, #TI_PREEMPT]	@ disable preempt
> +#endif
>  	armv6_set_pte_ext cpu_v6
> +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> +	str	r4, [r5, #TI_PREEMPT]	@ restore preempt count
> +	teq	r4, #0			@ preempt count == 0?
> +	ldreq	r4, [r5, #TI_FLAGS]	@ load flags if yes
> +	tst	r4, #_TIF_NEED_RESCHED	@ need resched?
> +	ldmia	sp!, {r4, r5}
> +	bne	preempt_schedule	@ ret via preempt_schedule
> +#endif

Same issue as previous patch.

Wouldn't it be better if you disabled interrupts around the small 
critical region within armv6_set_pte_ext instead?  That would certainly 
be a lighter solution.  Something like:

diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 307a4def8d..9bc2dbfe4e 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -167,8 +167,11 @@
 	tstne	r1, #L_PTE_PRESENT
 	moveq	r3, #0
 
+	mrs	r2, cpsr
+	cpsid	i
 	str	r3, [r0]
 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
+	msr	cpsr_c, r2
 	.endm


Nicolas

[RFC/PATCH v4 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[George G. Davis]

Hello,

On Tue, Oct 18, 2011 at 05:28:45PM -0400, Nicolas Pitre wrote:
> On Tue, 18 Oct 2011, gdavis at mvista.com wrote:
> 
> > @@ -241,6 +247,13 @@ v6_dma_inv_range:
> >  	blo	1b
> >  	mov	r0, #0
> >  	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
> > +#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
> > +	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
> > +	teq	r3, #0				@ preempt count == 0?
> > +	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
> > +	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
> > +	bne	preempt_schedule		@ ret via preempt_schedule
> 
> This is buggy.  If the preempt count is _not_ zero, you end up not 
> loading the TI_FLAGS bits and testing _TIF_NEED_RESCHED against that 
> non-zero preempt count.

Sigh, yep.  FWIW, I had the following before submitting this but dropped
it due to brain failure rereading the code - convinced myself that the
movne was redundant:


diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index af055dc..5c6ce38 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -285,6 +287,7 @@ v6_dma_inv_range:
 	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
 	teq	r3, #0				@ preempt count == 0?
 	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
+	movne	r3, #0				@ force flags to 0
 	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
 	bne	preempt_schedule		@ ret via preempt_schedule
 #endif
@@ -321,6 +324,7 @@ v6_dma_clean_range:
 	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
 	teq	r3, #0				@ preempt count == 0?
 	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
+	movne	r3, #0				@ force flags to 0
 	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
 	bne	preempt_schedule		@ ret via preempt_schedule
 #endif
@@ -362,6 +366,7 @@ ENTRY(v6_dma_flush_range)
 	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
 	teq	r3, #0				@ preempt count == 0?
 	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
+	movne	r3, #0				@ force flags to 0
 	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
 	bne	preempt_schedule		@ ret via preempt_schedule
 #endif

In this case, I think preempt_{disable,enable} is the better option
(over interrupt disable/enable) due to variable DMA buffer sizes.

Thanks!

--
Regards,
George

> 
> 
> Nicolas

[RFC/PATCH v4 7/7] ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe

[George G. Davis]

On Tue, Oct 18, 2011 at 05:52:37PM -0400, Nicolas Pitre wrote:
> On Tue, 18 Oct 2011, gdavis at mvista.com wrote:
> 
> > @@ -122,7 +122,22 @@ ENTRY(cpu_v6_switch_mm)
> >  
> >  ENTRY(cpu_v6_set_pte_ext)
> >  #ifdef CONFIG_MMU
> > +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> > +	stmdb	sp!, {r4, r5}
> > +	get_thread_info r5
> > +	ldr	r4, [r5, #TI_PREEMPT]	@ get preempt count
> > +	add	r3, r4, #1		@ increment it
> > +	str	r3, [r5, #TI_PREEMPT]	@ disable preempt
> > +#endif
> >  	armv6_set_pte_ext cpu_v6
> > +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> > +	str	r4, [r5, #TI_PREEMPT]	@ restore preempt count
> > +	teq	r4, #0			@ preempt count == 0?
> > +	ldreq	r4, [r5, #TI_FLAGS]	@ load flags if yes
> > +	tst	r4, #_TIF_NEED_RESCHED	@ need resched?
> > +	ldmia	sp!, {r4, r5}
> > +	bne	preempt_schedule	@ ret via preempt_schedule
> > +#endif
> 
> Same issue as previous patch.

Same mistake, I second guessed myself and removed the movne r4, #0
before submitting.  : /

> Wouldn't it be better if you disabled interrupts around the small 
> critical region within armv6_set_pte_ext instead?  That would certainly 
> be a lighter solution.  Something like:
> 
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 307a4def8d..9bc2dbfe4e 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -167,8 +167,11 @@
>  	tstne	r1, #L_PTE_PRESENT
>  	moveq	r3, #0
>  
> +	mrs	r2, cpsr
> +	cpsid	i
>  	str	r3, [r0]
>  	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
> +	msr	cpsr_c, r2
>  	.endm

I do like this better, less overhead than the preempt_{disable,enable}
case.

Thanks!

--
Regards,
George

> 
> 
> Nicolas

[RFC/PATCH v4 6/7] ARM: ARM11 MPCore: DMA_CACHE_RWFO operations are not preempt safe

[Nicolas Pitre]

On Tue, 18 Oct 2011, George G. Davis wrote:
> On Tue, Oct 18, 2011 at 05:28:45PM -0400, Nicolas Pitre wrote:
> > This is buggy.  If the preempt count is _not_ zero, you end up not 
> > loading the TI_FLAGS bits and testing _TIF_NEED_RESCHED against that 
> > non-zero preempt count.
> 
> Sigh, yep.  FWIW, I had the following before submitting this but dropped
> it due to brain failure rereading the code - convinced myself that the
> movne was redundant:
> 
> 
> diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
> index af055dc..5c6ce38 100644
> --- a/arch/arm/mm/cache-v6.S
> +++ b/arch/arm/mm/cache-v6.S
> @@ -285,6 +287,7 @@ v6_dma_inv_range:
>  	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
>  	teq	r3, #0				@ preempt count == 0?
>  	ldreq	r3, [ip, #TI_FLAGS]		@ load flags if yes
> +	movne	r3, #0				@ force flags to 0
>  	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
>  	bne	preempt_schedule		@ ret via preempt_schedule
>  #endif

At this point, especially on ARMv6+, I think it is more efficient to
simply skip over the code:

	teq	r3, #0
	str	r3, [ip, #TI_PREEMPT]
	bne	99f
	ldr	r3, [ip, #TI_FLAGS]
	tst	r3, #_TIF_NEED_RESCHED
	bne	preempt_schedule
99:

> In this case, I think preempt_{disable,enable} is the better option
> (over interrupt disable/enable) due to variable DMA buffer sizes.

Agreed.


Nicolas

parallel load of modules on an ARM multicore

[George G. Davis]

Hello,

I'd like to resurrect this thread in order to get the "ARM: Allow lazy
cache flushing on ARM11MPCore" or some variant pushed upstream.

On Tue, Jun 21, 2011 at 04:50:30PM +0100, Catalin Marinas wrote:
> On Mon, Jun 20, 2011 at 03:43:27PM +0200, EXTERNAL Waechtler Peter (Fa. TCP, CM-AI/PJ-CF31) wrote:
> > I'm getting unexpected results from loading several modules - some
> > of them in parallel - on an ARM11 MPcore system.
> > 
> > The system does not use "single sequential" modprobe commands - instead it
> > starts several shells doing insmod sequences like this:
> > 
> > shell A:
> > insmod a
> > insmod ab
> > insmod abc
> > 
> > shell B:
> > insmod b
> > insmod bc
> > insmod bcd
> > 
> > shell C:
> > insmod c
> > insmod cd
> > insmod cde
> > 
> > This is done to crash^H^H^H^H^Hboot faster ;)
> > 
> > While one insmod operation is protected via the module_mutex - I'm wondering
> > what happens with the instruction cache invalidation.
> > AFAICT the flush_icache_range only invalidates the ICache on the running cpu.
> > The module_mutex is unlocked after _loading_ the module, do_mod_ctors() and
> > do_one_initcall() are called without that lock - can they run on a different cpu?
> > It's an preemptible system (SMP PREEMPT armv6l).
> > 
> > Wouldn't it be required to flush the icache on _all_ cpus?
> 
> I theory, you would need to flush both the I and D cache on all the CPUs
> but that's not easily possible (well, I don't think there are many users
> of flush_icache_range(), so we could make this do an
> smp_call_function().
> 
> I think what happens (I'm looking at a 3.0-rc3 kernel) is that the
> module code is copied into RAM and the process could migrate to another
> CPU before flush_icache_range() gets called (this function is called
> outside the mutex_lock regions). We therefore don't flush the data out
> of the D-cache that was allocated during copy_from_user().
> 
> You can try the patch below (I haven't tested it for some time), it may
> fix the issue.
> 
> 8<-----------------------------------------------
> 
> ARM: Allow lazy cache flushing on ARM11MPCore
> 
> From: Catalin Marinas <catalin.marinas@arm.com>
> 
> The ARM11MPCore doesn't broadcast the cache maintenance operations in
> hardware, therefore the flush_dcache_page() currently performs the cache
> flushing non-lazily. But since not all drivers call this function after
> writing to a page cache page, the kernel needs a different approach like
> using read-for-ownership on the CPU flushing the cache to force the
> dirty cache lines migration from other CPUs. This way the cache flushing
> operation can be done lazily via __sync_icache_dcache().
> 
> Since we cannot force read-for-ownership on the I-cache, the patch
> invalidates the whole I-cache when a thread migrates to another CPU.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

Tested-by: George G. Davis <gdavis@mvista.com>

On an ARM11 MPCore w/3-cores w/o this change, parallel module
loading stress testing with CONFIG_PREEMPT enabled falls over
due to cache incoherency.  Applying this change resolves parallel
module load oopses due to cache coherency issues.

> ---
>  arch/arm/include/asm/mmu_context.h |    3 ++-
>  arch/arm/mm/cache-v6.S             |    8 ++++++++
>  arch/arm/mm/flush.c                |    3 +--
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h
> index 71605d9..d116a23 100644
> --- a/arch/arm/include/asm/mmu_context.h
> +++ b/arch/arm/include/asm/mmu_context.h
> @@ -114,7 +114,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  #ifdef CONFIG_SMP
>  	/* check for possible thread migration */
>  	if (!cpumask_empty(mm_cpumask(next)) &&
> -	    !cpumask_test_cpu(cpu, mm_cpumask(next)))
> +	    (cache_ops_need_broadcast() ||
> +	     !cpumask_test_cpu(cpu, mm_cpumask(next))))
>  		__flush_icache_all();

Dropped this hunk based on feedback in this thread that it is not
required and merely results in __flush_icache_all() on every
context switch.

>  #endif
>  	if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next)) || prev != next) {
> diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
> index 73b4a8b..bdc1cc1 100644
> --- a/arch/arm/mm/cache-v6.S
> +++ b/arch/arm/mm/cache-v6.S
> @@ -133,6 +133,10 @@ ENTRY(v6_coherent_user_range)
>  #ifdef HARVARD_CACHE
>  	bic	r0, r0, #CACHE_LINE_SIZE - 1
>  1:
> +#ifdef CONFIG_SMP
> +	/* no cache maintenance broadcasting on ARM11MPCore */
> + USER(	ldr	r2, [r0]		)	@ read for ownership
> +#endif
>   USER(	mcr	p15, 0, r0, c7, c10, 1	)	@ clean D line
>  	add	r0, r0, #CACHE_LINE_SIZE
>  2:
> @@ -178,6 +182,10 @@ ENTRY(v6_flush_kern_dcache_area)
>  	add	r1, r0, r1
>  	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
>  1:
> +#ifdef CONFIG_SMP
> +	/* no cache maintenance broadcasting on ARM11MPCore */
> +	ldr	r2, [r0]			@ read for ownership
> +#endif
>  #ifdef HARVARD_CACHE
>  	mcr	p15, 0, r0, c7, c14, 1		@ clean & invalidate D line
>  #else

I also added the following around the above RFOs to disable
preemption:

From: George G. Davis <gdavis@mvista.com>
Date: Tue, 4 Oct 2011 13:44:37 -0400
Subject: [PATCH] ARM: ARM11 MPCore: RFO in v6_coherent_{kern,user}_range and v6_flush_kern_dcache_area are not preempt safe

The RFO operations added to v6_coherent_{kern,user}_range and
v6_flush_kern_dcache_area via the "ARM: Allow lazy cache flushing on
ARM11MPCore" commit are not preempt safe.  If preemption
occurs immediately following a RFO operation on a cache line of
CPU A, it is possible for task migration to occur on resume at
which point the subsequent cache maintenance operation on the
same cache line of CPU B will have no affect on the CPU A cache
line leading to inconsistent memory and/or cache state.  To prevent
this, disable preemption during RFO operations in these functions.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 301ed8f..932143f 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -123,6 +123,12 @@ ENTRY(v6_coherent_kern_range)
  */
 ENTRY(v6_coherent_user_range)
  UNWIND(.fnstart		)
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 #ifdef HARVARD_CACHE
 	bic	r0, r0, #CACHE_LINE_SIZE - 1
 1:
@@ -147,6 +153,15 @@ ENTRY(v6_coherent_user_range)
 #else
 	mcr	p15, 0, r0, c7, c5, 6		@ invalidate BTB
 #endif
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	teq	r3, #0				@ preempt count == 0?
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	bne	99f				@ done if non-zero
+	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ yes, do preempt_schedule
+99:
+#endif
 	mov	pc, lr
 
 /*
@@ -172,6 +187,12 @@ ENDPROC(v6_coherent_kern_range)
  *	- size	- region size
  */
 ENTRY(v6_flush_kern_dcache_area)
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	add	r1, r0, r1
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
@@ -191,6 +212,15 @@ ENTRY(v6_flush_kern_dcache_area)
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4
 #endif
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	teq	r3, #0				@ preempt count == 0?
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	bne	99f				@ done if non-zero
+	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ yes, do preempt_schedule
+99:
+#endif
 	mov	pc, lr
 

Thanks!

--
Regards,
George
 
> diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> index 1a8d4aa..72f9333 100644
> --- a/arch/arm/mm/flush.c
> +++ b/arch/arm/mm/flush.c
> @@ -291,8 +291,7 @@ void flush_dcache_page(struct page *page)
>  
>  	mapping = page_mapping(page);
>  
> -	if (!cache_ops_need_broadcast() &&
> -	    mapping && !mapping_mapped(mapping))
> +	if (mapping && !mapping_mapped(mapping))
>  		clear_bit(PG_dcache_clean, &page->flags);
>  	else {
>  		__flush_dcache_page(mapping, page);
> 
> -- 
> Catalin
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC/PATCH v5 0/7] ARM11 MPCore: preemption/task migration cache coherency fixups

[gdavis at mvista.com]

Greetings,

Apologies for the long delay following up on this...

On ARM11 MPCore, the "SCU does not handle coherency consequences of CP15
cache operations" [1].  So cache maintenance functions have to insure
that memory is globally consistent.  Although the current Linux kernel
works reasonably well on ARM11 MPCore machines, PREEMPT stress testing,
e.g. parallel module loading, hackbench and LTP stress, results in
crashes which exhibit non-sense oops traces where machine state does
not make sense relative to the code executing at the time of the oops.

Review and analysis of the various ARM11 MPCore cache maintenance
functions reveal that there are a number critical sections in which
ARM11 MPCore caches and/or memory may become inconsistent, i.e. a
cache line on CPU A contains a modified entry but preemption and task
migration occurs after which the same cache line is cleaned/flushed
on CPU B.  This can obviously lead to inconsistent memory and/or
cache state as cache ops on ARM11 MPCore are non-coherent.

The following is a series of ARM11 MPCore preemption/task migration
fixes to resolve cache coherency problems on these machines:

George G. Davis (6):
      ARM: ARM11 MPCore: Make pte_alloc_one{,_kernel} preempt safe
      ARM: ARM11 MPCore: Make {clean,flush}_pmd_entry preempt safe
      ARM: Move get_thread_info macro definition to <asm/assembler.h>
      ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO
      ARM: ARM11 MPCore: Make DMA_CACHE_RWFO operations preempt safe
      ARM: ARM11 MPCore: Make cpu_v6_set_pte_ext preempt safe

Konstantin Baidarov (1):
      ARM: ARM11 MPCore: Make pgd_alloc preempt safe

 arch/arm/include/asm/assembler.h      |   13 +++++++++
 arch/arm/include/asm/pgalloc.h        |   28 +++++++++++++++++---
 arch/arm/include/asm/pgtable-2level.h |    8 ++++++
 arch/arm/include/asm/pgtable.h        |    1 +
 arch/arm/include/asm/smp_plat.h       |    2 +
 arch/arm/kernel/entry-header.S        |   11 --------
 arch/arm/mm/cache-v6.S                |   45 +++++++++++++++++++++++++++++++++
 arch/arm/mm/idmap.c                   |    4 +++
 arch/arm/mm/pgd.c                     |    7 +++++
 arch/arm/mm/proc-macros.S             |    3 ++
 arch/arm/mm/proc-v6.S                 |   22 +++++++++++++++-
 arch/arm/vfp/entry.S                  |    5 +++-
 arch/arm/vfp/vfphw.S                  |    5 +++-
 13 files changed, 135 insertions(+), 19 deletions(-)

The above changes were tested on:

1. ARM Ltd RealView ARM11 MPCore
2. NEC NE1 TB
3. NEC NEmid TB

Test cases used:

1. Continuous parallel modprobe stress test
2. 24 hour LTP stress test

When these fixes are not applied, both test cases fail often and early where
the failure symptoms are random hard/soft lockups and/or non-sense oopses.

When these fixes are applied, no test case failures are observed.

Note that the following change is an alternative implementation to
fix all callers of clean_dcache_area:

      ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

If the above change is not an acceptable fix, then preemption/
task migration issues in callers of clean_dcache_area still
need to be fixed in some way.  On the other hand, if the above
change is acceptable, then the following changes can be dropped
since they call clean_dcache_area which is fixed by the above
change:

      ARM: ARM11 MPCore: Make pgd_alloc preempt safe
      ARM: ARM11 MPCore: Make pte_alloc_one{,_kernel} preempt safe


I further believe that Catalin's "ARM: Allow lazy cache flushing on
ARM11MPCore" [2][3] is required for ARM11 MPCore machines and would
like to see that or similar/alternative solution applied.  I have
an additional RWFO preempt fix which must be applied on top of
Catalin's patch [2][3] to resolve preeption/task migration issues
when his patch is applied.  I'll submit that patch as a follow up
to that thread.

Comments/feedback greatly appreciated.

TIA!

ChangeLog:

V2:
- Substitute {get,put}_cpu() with preempt_{disable,enable}().
- Fixed preempt {dis,en}able assembler code sequences to not
 use r11 since it is reserved for frame pointer use.  Also
 optimised these sequences to use r2, r3; ip scratch registers
 in most cases to eliminate stack push/pop.  In one case,
 cpu_v6_set_pte_ext, there are only two scratch registers
 available, r3 and ip.  However, both of these are used within
 the armv6_set_pte_ext macro.  So for this case, r3 is used
 as a temporary scratch when disabling preemption and r4 and
 r5 are pushed/popped as needed for other uses to avoid
 conflict with scratch register usage in armv6_set_pte_ext.
- Remove incorrect use of ALT_SMP macros in cpu_v6_set_pte_ext,
 making the preempt {dis,en}able assembler code sequences
 compile time dependent upon CONFIG_SMP instead.  This code
 is safe on UP machines anyway.
V3:
- Fix HIGHMEM breakage.
V4:
- Drop preempt_{disable,enable}() around calls to flush_pmd_entry()
 in remap_area_sections() and remap_area_supersections() in
 file arch/arm/mm/ioremap.c since these functions are not used
 on SMP machines (they're enclosed within #ifndef SMP/#endif)
- Drop preempt_{disable,enable}() around call to flush_pmd_entry()
 in alloc_init_section() in file arch/arm/mm/mmu.c since this
 function is called during early kernel initialization during
 which time preemption and task migration is not possible.  Also
 removed bogus FIXME comment as part of this change.
- Added calls to preempt_schedule when re-enabling preemption
 in various arch/arm/mm/{cache,proc}-v6.S functions which need
 preemption disabled due task migration issues.
V5:
- Drop OMAP specific changes in "ARM: ARM11 MPCore: clean_dcache_area
  is not preempt safe" based on feed back from Tony Lindgren [4]
  indicating that the affected code is not used on ARM11 MPCore
  machines.
- Fix bug in implementation of conditional preempt_schedule calls
  pointed out by Nicolas Pitre [5].
- Reimplement "ARM: ARM11 MPCore: cpu_v6_set_pte_ext is not preempt safe"
  to simply disable/enable interrupts around critical section based on
  feedback from Nicolas Pitre [6].
- Improve (ARMv6+ instruction cycle) efficiency of preempt_enable
  assembler code sequences as suggested by Nicolas Pitre [7]. 
- Implement alternative implementation for fixup of clean_dcache_area
  callers by implementing RFO and preempt disable in
  cpu_v6_dcache_clean_area.  If this change is acceptable, the
  "ARM: ARM11 MPCore: Make pgd_alloc preempt safe" and "ARM: ARM11
  MPCore: Make pte_alloc_one{,_kernel} preempt safe" commits can be
  dropped. Else, if unacceptable, some callers of clean_dcache_area
  may be missing preempt/migration fixups which are not otherwise
  fixed in this series.

--
Regards,
George

References:

[1] http://infocenter.arm.com/help/topic/com.arm.doc.dai0228a/index.html#arm_toc9
[2] http://www.spinics.net/lists/arm-kernel/msg129403.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-May/014990.html
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-October/069912.html
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-October/069942.html
[6] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-October/069943.html
[7] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-October/069953.html

[RFC/PATCH v5 1/7] ARM: ARM11 MPCore: Make pgd_alloc preempt safe

[gdavis at mvista.com]

From: Konstantin Baidarov <kbaidarov@mvista.com>

If preemption and subsequent task migration occurs during a call to
pgd_alloc on ARM11 MPCore machines, global memory state can become
inconsistent.  To prevent inconsistent memory state on these
machines, disable preemption around initialization and flushing
of new PGDs.

Signed-off-by: Konstantin Baidarov <kbaidarov@mvista.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/pgd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index 0acb089..174b387 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -16,6 +16,7 @@
 #include <asm/pgalloc.h>
 #include <asm/page.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #include "mm.h"
 
@@ -41,6 +42,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 	if (!new_pgd)
 		goto no_pgd;
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
+
 	memset(new_pgd, 0, USER_PTRS_PER_PGD * sizeof(pgd_t));
 
 	/*
@@ -52,6 +56,9 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 
 	clean_dcache_area(new_pgd, PTRS_PER_PGD * sizeof(pgd_t));
 
+	if (cache_ops_need_broadcast())
+		preempt_enable();
+
 #ifdef CONFIG_ARM_LPAE
 	/*
 	 * Allocate PMD table for modules and pkmap mappings.
-- 
1.7.4.4

[RFC/PATCH v5 2/7] ARM: ARM11 MPCore: Make pte_alloc_one{, _kernel} preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during a call to
pte_alloc_one{,_kernel} on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption around initialization and flushing
of new PTEs.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 943504f..c290b42 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -17,6 +17,7 @@
 #include <asm/processor.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #define check_pgt_cache()		do { } while (0)
 
@@ -57,7 +58,7 @@ static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
 extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
 
-#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
+#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT)
 
 static inline void clean_pte_table(pte_t *pte)
 {
@@ -86,8 +87,14 @@ pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
 	pte_t *pte;
 
 	pte = (pte_t *)__get_free_page(PGALLOC_GFP);
-	if (pte)
+	if (pte) {
+		if (cache_ops_need_broadcast())
+			preempt_disable();
+		memset(pte, 0, PAGE_SIZE);
 		clean_pte_table(pte);
+		if (cache_ops_need_broadcast())
+			preempt_enable();
+	}
 
 	return pte;
 }
@@ -98,13 +105,20 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 	struct page *pte;
 
 #ifdef CONFIG_HIGHPTE
-	pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
+	pte = alloc_pages(PGALLOC_GFP | __GFP_ZERO | __GFP_HIGHMEM, 0);
 #else
 	pte = alloc_pages(PGALLOC_GFP, 0);
 #endif
 	if (pte) {
-		if (!PageHighMem(pte))
-			clean_pte_table(page_address(pte));
+		if (!PageHighMem(pte)) {
+			void *page = page_address(pte);
+			if (cache_ops_need_broadcast())
+				preempt_disable();
+			memset(page, 0, PAGE_SIZE);
+			clean_pte_table(page);
+			if (cache_ops_need_broadcast())
+				preempt_enable();
+		}
 		pgtable_page_ctor(pte);
 	}
 
-- 
1.7.4.4

[RFC/PATCH v5 3/7] ARM: ARM11 MPCore: Make {clean, flush}_pmd_entry preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
{clean,flush}_pmd_entry on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable preemption in callers of these functions around
PMD modifications and subsequent {clean,flush}_pmd_entry calls.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/pgalloc.h        |    4 ++++
 arch/arm/include/asm/pgtable-2level.h |    8 ++++++++
 arch/arm/include/asm/pgtable.h        |    1 +
 arch/arm/include/asm/smp_plat.h       |    2 ++
 arch/arm/mm/idmap.c                   |    4 ++++
 5 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index c290b42..0dc9a81 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -144,11 +144,15 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
 				  pmdval_t prot)
 {
 	pmdval_t pmdval = (pte + PTE_HWTABLE_OFF) | prot;
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	pmdp[0] = __pmd(pmdval);
 #ifndef CONFIG_ARM_LPAE
 	pmdp[1] = __pmd(pmdval + 256 * sizeof(pte_t));
 #endif
 	flush_pmd_entry(pmdp);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 
 /*
diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
index 2317a71..93ad948 100644
--- a/arch/arm/include/asm/pgtable-2level.h
+++ b/arch/arm/include/asm/pgtable-2level.h
@@ -162,16 +162,24 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)
 
 #define copy_pmd(pmdpd,pmdps)		\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdpd[0] = pmdps[0];	\
 		pmdpd[1] = pmdps[1];	\
 		flush_pmd_entry(pmdpd);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 #define pmd_clear(pmdp)			\
 	do {				\
+		if (cache_ops_need_broadcast())	\
+			preempt_disable();	\
 		pmdp[0] = __pmd(0);	\
 		pmdp[1] = __pmd(0);	\
 		clean_pmd_entry(pmdp);	\
+		if (cache_ops_need_broadcast())	\
+			preempt_enable();	\
 	} while (0)
 
 /* we don't need complex calculations here as the pmd is folded into the pgd */
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index f66626d..b511fcf 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -12,6 +12,7 @@
 
 #include <linux/const.h>
 #include <asm/proc-fns.h>
+#include <asm/smp_plat.h>
 
 #ifndef CONFIG_MMU
 
diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index 558d6c8..bd419d5 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -5,6 +5,7 @@
 #ifndef __ASMARM_SMP_PLAT_H
 #define __ASMARM_SMP_PLAT_H
 
+#ifndef	__ASSEMBLY__
 #include <asm/cputype.h>
 
 /*
@@ -50,3 +51,4 @@ extern int __cpu_logical_map[];
 #define cpu_logical_map(cpu)	__cpu_logical_map[cpu]
 
 #endif
+#endif
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index ab88ed4..2846ce7 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -39,11 +39,15 @@ static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
 
+	if (cache_ops_need_broadcast())
+		preempt_disable();
 	addr = (addr & PMD_MASK) | prot;
 	pmd[0] = __pmd(addr);
 	addr += SECTION_SIZE;
 	pmd[1] = __pmd(addr);
 	flush_pmd_entry(pmd);
+	if (cache_ops_need_broadcast())
+		preempt_enable();
 }
 #endif	/* CONFIG_ARM_LPAE */
 
-- 
1.7.4.4

[RFC/PATCH v5 4/7] ARM: Move get_thread_info macro definition to <asm/assembler.h>

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The get_thread_info assembler macro is currently defined in
arch/arm/kernel/entry-header.S and used in the following
files:

	arch/arm/kernel/entry-armv.S
	arch/arm/kernel/entry-common.S
	arch/arm/vfp/vfphw.S
	arch/arm/vfp/entry.S

The first two cases above use #include "entry-header.S" while
the later two cases use #include "../kernel/entry-header.S"
to obtain the get_thread_info assembler macro definition.

Move the get_thread_info assembler macro definition into
arch/arm/include/asm/assember.h and clean up inclusion
of this macro definition to use #include <asm/assember.h>.

This change facilitates use of the get_thread_info assembler
macro in other assembler functions without more of the same
path relative include directives.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/include/asm/assembler.h |   13 +++++++++++++
 arch/arm/kernel/entry-header.S   |   11 -----------
 arch/arm/vfp/entry.S             |    5 ++++-
 arch/arm/vfp/vfphw.S             |    5 ++++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 03fb936..5fd570d 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -23,6 +23,19 @@
 #include <asm/ptrace.h>
 #include <asm/domain.h>
 
+#ifndef CONFIG_THUMB2_KERNEL
+	.macro	get_thread_info, rd
+	mov	\rd, sp, lsr #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#else   /* CONFIG_THUMB2_KERNEL */
+	.macro	get_thread_info, rd
+	mov	\rd, sp
+	lsr	\rd, \rd, #13
+	mov	\rd, \rd, lsl #13
+	.endm
+#endif  /* !CONFIG_THUMB2_KERNEL */
+
 #define IOMEM(x)	(x)
 
 /*
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..d03b6a9 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -108,11 +108,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp, lsr #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
@@ -148,12 +143,6 @@
 	movs	pc, lr				@ return & move spsr_svc into cpsr
 	.endm
 
-	.macro	get_thread_info, rd
-	mov	\rd, sp
-	lsr	\rd, \rd, #13
-	mov	\rd, \rd, lsl #13
-	.endm
-
 	@
 	@ 32-bit wide "mov pc, reg"
 	@
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 4fa9903..e475a5a 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -15,9 +15,12 @@
  *  r10 = thread_info structure
  *  lr  = failure return
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 ENTRY(do_vfp)
 #ifdef CONFIG_PREEMPT
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 2d30c7f..68ca5af 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -14,9 +14,12 @@
  * r10 points at the start of the private FP workspace in the thread structure
  * sp points to a struct pt_regs (as defined in include/asm/proc/ptrace.h)
  */
+#include <linux/init.h>
+#include <linux/linkage.h>
 #include <asm/thread_info.h>
 #include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
 
 	.macro	DBGSTR, str
 #ifdef DEBUG
-- 
1.7.4.4

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

Implement Request-For-Ownership in cpu_v6_dcache_clean_area and
disable preemption in same to insure that memory is consistent
in cases of preemption and subsequent task migration just before
and during calls to this function.  This is an alternative
implementation to other workarounds which disable preemption
in callers of this function while updating PTEs.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/proc-v6.S |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
index 5900cd5..de8b3a6 100644
--- a/arch/arm/mm/proc-v6.S
+++ b/arch/arm/mm/proc-v6.S
@@ -81,10 +81,30 @@ ENTRY(cpu_v6_do_idle)
 
 ENTRY(cpu_v6_dcache_clean_area)
 #ifndef TLB_CAN_READ_FROM_L1_CACHE
-1:	mcr	p15, 0, r0, c7, c10, 1		@ clean D entry
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preemption
+#endif
+1:
+#ifdef CONFIG_SMP
+	/* no cache maintenance broadcasting on ARM11MPCore */
+	ldr	r2, [r0]			@ read for ownership
+#endif
+	mcr	p15, 0, r0, c7, c10, 1		@ clean D entry
 	add	r0, r0, #D_CACHE_LINE_SIZE
 	subs	r1, r1, #D_CACHE_LINE_SIZE
 	bhi	1b
+#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+	teq	r3, #0				@ preempt count == 0?
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	bne	99f				@ done if non-zero
+	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ yes, do preempt_schedule
+99:
+#endif
 #endif
 	mov	pc, lr
 
-- 
1.7.4.4

[RFC/PATCH v5 6/7] ARM: ARM11 MPCore: Make DMA_CACHE_RWFO operations preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

The DMA_CACHE_RWFO operations are not preempt safe.  If preemption
occurs immediately following a RWFO operation on a cache line of
CPU A, it is possible for task migration to occur on resume at
which point the subsequent cache maintenance operation on the
same cache line of CPU B will have no affect on the CPU A cache
line leading to inconsistent memory and/or cache state.  To prevent
this, disable preemption during RWFO operations.

This change depends on "ARM: Move get_thread_info macro definition
to <asm/assembler.h>".

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/cache-v6.S |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 4b10760..ee36b6c 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -203,6 +203,12 @@ ENTRY(v6_flush_kern_dcache_area)
  */
 v6_dma_inv_range:
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]			@ read for ownership
 	strb	r2, [r0]			@ write for ownership
 #endif
@@ -239,6 +245,15 @@ v6_dma_inv_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	teq	r3, #0				@ preempt count == 0?
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	bne	99f				@ done if non-zero
+	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ yes, do preempt_schedule
+99:
+#endif
 	mov	pc, lr
 
 /*
@@ -247,6 +262,12 @@ v6_dma_inv_range:
  *	- end     - virtual end address of region
  */
 v6_dma_clean_range:
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
 #ifdef CONFIG_DMA_CACHE_RWFO
@@ -262,6 +283,15 @@ v6_dma_clean_range:
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	teq	r3, #0				@ preempt count == 0?
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	bne	99f				@ done if non-zero
+	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ yes, do preempt_schedule
+99:
+#endif
 	mov	pc, lr
 
 /*
@@ -271,6 +301,12 @@ v6_dma_clean_range:
  */
 ENTRY(v6_dma_flush_range)
 #ifdef CONFIG_DMA_CACHE_RWFO
+#ifdef CONFIG_PREEMPT
+	get_thread_info ip
+	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
+	add	r2, r3, #1			@ increment it
+	str	r2, [ip, #TI_PREEMPT]		@ disable preempt
+#endif
 	ldrb	r2, [r0]		@ read for ownership
 	strb	r2, [r0]		@ write for ownership
 #endif
@@ -290,6 +326,15 @@ ENTRY(v6_dma_flush_range)
 	blo	1b
 	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+#if	defined(CONFIG_DMA_CACHE_RWFO) && defined(CONFIG_PREEMPT)
+	teq	r3, #0				@ preempt count == 0?
+	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
+	bne	99f				@ done if non-zero
+	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
+	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
+	bne	preempt_schedule		@ yes, do preempt_schedule
+99:
+#endif
 	mov	pc, lr
 
 /*
-- 
1.7.4.4

[RFC/PATCH v5 7/7] ARM: ARM11 MPCore: Make cpu_v6_set_pte_ext preempt safe

[gdavis at mvista.com]

From: George G. Davis <gdavis@mvista.com>

If preemption and subsequent task migration occurs during calls to
cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
can become inconsistent.  To prevent inconsistent memory state on
these machines, disable interrupts in cpu_v6_set_pte_ext.

Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 arch/arm/mm/proc-macros.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 2d8ff3a..7e32603 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -168,8 +168,11 @@
 	tstne	r1, #L_PTE_PRESENT
 	moveq	r3, #0
 
+	mrs	r2, cpsr
+	cpsid	i
 	str	r3, [r0]
 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
+	msr	cpsr_c, r2
 	.endm
 
 
-- 
1.7.4.4

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[Catalin Marinas]

On Tue, Jun 12, 2012 at 09:40:16PM +0100, gdavis at mvista.com wrote:
> From: George G. Davis <gdavis@mvista.com>
> 
> Implement Request-For-Ownership in cpu_v6_dcache_clean_area and
> disable preemption in same to insure that memory is consistent
> in cases of preemption and subsequent task migration just before
> and during calls to this function.  This is an alternative
> implementation to other workarounds which disable preemption
> in callers of this function while updating PTEs.
> 
> This change depends on "ARM: Move get_thread_info macro definition
> to <asm/assembler.h>".
> 
> Signed-off-by: George G. Davis <gdavis@mvista.com>
> ---
>  arch/arm/mm/proc-v6.S |   22 +++++++++++++++++++++-
>  1 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
> index 5900cd5..de8b3a6 100644
> --- a/arch/arm/mm/proc-v6.S
> +++ b/arch/arm/mm/proc-v6.S
> @@ -81,10 +81,30 @@ ENTRY(cpu_v6_do_idle)
>  
>  ENTRY(cpu_v6_dcache_clean_area)
>  #ifndef TLB_CAN_READ_FROM_L1_CACHE
> -1:	mcr	p15, 0, r0, c7, c10, 1		@ clean D entry
> +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> +	get_thread_info ip
> +	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
> +	add	r2, r3, #1			@ increment it
> +	str	r2, [ip, #TI_PREEMPT]		@ disable preemption
> +#endif
> +1:
> +#ifdef CONFIG_SMP
> +	/* no cache maintenance broadcasting on ARM11MPCore */
> +	ldr	r2, [r0]			@ read for ownership
> +#endif
> +	mcr	p15, 0, r0, c7, c10, 1		@ clean D entry
>  	add	r0, r0, #D_CACHE_LINE_SIZE
>  	subs	r1, r1, #D_CACHE_LINE_SIZE
>  	bhi	1b
> +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> +	teq	r3, #0				@ preempt count == 0?
> +	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
> +	bne	99f				@ done if non-zero
> +	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
> +	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
> +	bne	preempt_schedule		@ yes, do preempt_schedule

Maybe we should get some asm macros for preempt disable/enable as they
are used in more than one place.

Alternatively, we could disable the interrupts around RFO and D-cache
cleaning (inside the loop), though I think the preempt disabling is
faster.

-- 
Catalin

[RFC/PATCH v5 7/7] ARM: ARM11 MPCore: Make cpu_v6_set_pte_ext preempt safe

[Catalin Marinas]

On Tue, Jun 12, 2012 at 09:40:18PM +0100, gdavis at mvista.com wrote:
> From: George G. Davis <gdavis@mvista.com>
> 
> If preemption and subsequent task migration occurs during calls to
> cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
> can become inconsistent.  To prevent inconsistent memory state on
> these machines, disable interrupts in cpu_v6_set_pte_ext.
> 
> Signed-off-by: George G. Davis <gdavis@mvista.com>
> ---
>  arch/arm/mm/proc-macros.S |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 2d8ff3a..7e32603 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -168,8 +168,11 @@
>  	tstne	r1, #L_PTE_PRESENT
>  	moveq	r3, #0
>  
> +	mrs	r2, cpsr
> +	cpsid	i
>  	str	r3, [r0]
>  	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
> +	msr	cpsr_c, r2
>  	.endm

We have save_and_disable_irqs and restore_irqs (and _notrace variants)
macros in asm/assembler.h.

-- 
Catalin

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[Russell King - ARM Linux]

On Wed, Jun 13, 2012 at 10:32:18AM +0100, Catalin Marinas wrote:
> Alternatively, we could disable the interrupts around RFO and D-cache
> cleaning (inside the loop), though I think the preempt disabling is
> faster.

It also has the *important* effect that it doesn't cause problems with
interrupt latency, which is a real concern if you have peripherals
requiring quick servicing.

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[Catalin Marinas]

On Wed, Jun 13, 2012 at 10:36:34AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 13, 2012 at 10:32:18AM +0100, Catalin Marinas wrote:
> > Alternatively, we could disable the interrupts around RFO and D-cache
> > cleaning (inside the loop), though I think the preempt disabling is
> > faster.
> 
> It also has the *important* effect that it doesn't cause problems with
> interrupt latency, which is a real concern if you have peripherals
> requiring quick servicing.

I was suggesting just disabling the interrupts inside the loop around
RFO and D-cache cleaning, so only for a single cache line (and not the
whole range) which would not introduce noticeable interrupt latency.

-- 
Catalin

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[Russell King - ARM Linux]

On Wed, Jun 13, 2012 at 10:41:27AM +0100, Catalin Marinas wrote:
> On Wed, Jun 13, 2012 at 10:36:34AM +0100, Russell King - ARM Linux wrote:
> > On Wed, Jun 13, 2012 at 10:32:18AM +0100, Catalin Marinas wrote:
> > > Alternatively, we could disable the interrupts around RFO and D-cache
> > > cleaning (inside the loop), though I think the preempt disabling is
> > > faster.
> > 
> > It also has the *important* effect that it doesn't cause problems with
> > interrupt latency, which is a real concern if you have peripherals
> > requiring quick servicing.
> 
> I was suggesting just disabling the interrupts inside the loop around
> RFO and D-cache cleaning, so only for a single cache line (and not the
> whole range) which would not introduce noticeable interrupt latency.

But would slow down the loop, probably significantly.

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[Catalin Marinas]

On Wed, Jun 13, 2012 at 10:45:28AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 13, 2012 at 10:41:27AM +0100, Catalin Marinas wrote:
> > On Wed, Jun 13, 2012 at 10:36:34AM +0100, Russell King - ARM Linux wrote:
> > > On Wed, Jun 13, 2012 at 10:32:18AM +0100, Catalin Marinas wrote:
> > > > Alternatively, we could disable the interrupts around RFO and D-cache
> > > > cleaning (inside the loop), though I think the preempt disabling is
> > > > faster.
> > > 
> > > It also has the *important* effect that it doesn't cause problems with
> > > interrupt latency, which is a real concern if you have peripherals
> > > requiring quick servicing.
> > 
> > I was suggesting just disabling the interrupts inside the loop around
> > RFO and D-cache cleaning, so only for a single cache line (and not the
> > whole range) which would not introduce noticeable interrupt latency.
> 
> But would slow down the loop, probably significantly.

Yes, indeed.

-- 
Catalin

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[George G. Davis]

Hello,

On Jun 13, 2012, at 5:32 AM, Catalin Marinas wrote:

> On Tue, Jun 12, 2012 at 09:40:16PM +0100, gdavis at mvista.com wrote:
>> From: George G. Davis <gdavis@mvista.com>
>> 
>> Implement Request-For-Ownership in cpu_v6_dcache_clean_area and
>> disable preemption in same to insure that memory is consistent
>> in cases of preemption and subsequent task migration just before
>> and during calls to this function.  This is an alternative
>> implementation to other workarounds which disable preemption
>> in callers of this function while updating PTEs.
>> 
>> This change depends on "ARM: Move get_thread_info macro definition
>> to <asm/assembler.h>".
>> 
>> Signed-off-by: George G. Davis <gdavis@mvista.com>
>> ---
>> arch/arm/mm/proc-v6.S |   22 +++++++++++++++++++++-
>> 1 files changed, 21 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
>> index 5900cd5..de8b3a6 100644
>> --- a/arch/arm/mm/proc-v6.S
>> +++ b/arch/arm/mm/proc-v6.S
>> @@ -81,10 +81,30 @@ ENTRY(cpu_v6_do_idle)
>> 
>> ENTRY(cpu_v6_dcache_clean_area)
>> #ifndef TLB_CAN_READ_FROM_L1_CACHE
>> -1:	mcr	p15, 0, r0, c7, c10, 1		@ clean D entry
>> +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
>> +	get_thread_info ip
>> +	ldr	r3, [ip, #TI_PREEMPT]		@ get preempt count
>> +	add	r2, r3, #1			@ increment it
>> +	str	r2, [ip, #TI_PREEMPT]		@ disable preemption
>> +#endif
>> +1:
>> +#ifdef CONFIG_SMP
>> +	/* no cache maintenance broadcasting on ARM11MPCore */
>> +	ldr	r2, [r0]			@ read for ownership
>> +#endif
>> +	mcr	p15, 0, r0, c7, c10, 1		@ clean D entry
>> 	add	r0, r0, #D_CACHE_LINE_SIZE
>> 	subs	r1, r1, #D_CACHE_LINE_SIZE
>> 	bhi	1b
>> +#if	defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
>> +	teq	r3, #0				@ preempt count == 0?
>> +	str	r3, [ip, #TI_PREEMPT]		@ restore preempt count
>> +	bne	99f				@ done if non-zero
>> +	ldr	r3, [ip, #TI_FLAGS]		@ else check flags
>> +	tst	r3, #_TIF_NEED_RESCHED		@ need resched?
>> +	bne	preempt_schedule		@ yes, do preempt_schedule
> 
> Maybe we should get some asm macros for preempt disable/enable as they
> are used in more than one place.

I've pondered that a bit, it's not easy,  the conditional call to preempt_schedule
in particular is non-trivial, you have to save/restore lr somewhere.  In the open
coded cases in these patches, it was easy to simply branch to preempt_schedule
and return from there but I suspect it won't be so trivial in all cases.

> Alternatively, we could disable the interrupts around RFO and D-cache
> cleaning (inside the loop), though I think the preempt disabling is
> faster.

I've had reports that disabling preemption is impacting performance
in some of these cases (requiring nasty hacks in upper level macros
to break up blocks into smaller pieces).  So I don't even want to
consider disabling interrupts for these cases.  : )

Thanks!

--
Regards,
George

> 
> -- 
> Catalin

[RFC/PATCH v5 7/7] ARM: ARM11 MPCore: Make cpu_v6_set_pte_ext preempt safe

[George G. Davis]

On Jun 13, 2012, at 5:34 AM, Catalin Marinas wrote:

> On Tue, Jun 12, 2012 at 09:40:18PM +0100, gdavis at mvista.com wrote:
>> From: George G. Davis <gdavis@mvista.com>
>> 
>> If preemption and subsequent task migration occurs during calls to
>> cpu_v6_set_pte_ext on ARM11 MPCore machines, global memory state
>> can become inconsistent.  To prevent inconsistent memory state on
>> these machines, disable interrupts in cpu_v6_set_pte_ext.
>> 
>> Signed-off-by: George G. Davis <gdavis@mvista.com>
>> ---
>> arch/arm/mm/proc-macros.S |    3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
>> index 2d8ff3a..7e32603 100644
>> --- a/arch/arm/mm/proc-macros.S
>> +++ b/arch/arm/mm/proc-macros.S
>> @@ -168,8 +168,11 @@
>> 	tstne	r1, #L_PTE_PRESENT
>> 	moveq	r3, #0
>> 
>> +	mrs	r2, cpsr
>> +	cpsid	i
>> 	str	r3, [r0]
>> 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
>> +	msr	cpsr_c, r2
>> 	.endm
> 
> We have save_and_disable_irqs and restore_irqs (and _notrace variants)
> macros in asm/assembler.h.

I updated this patch to use save_and_disable_irqs and restore_irqs.

Thanks!

--
Regards,
George

> 
> -- 
> Catalin

[RFC/PATCH v5 5/7] ARM: ARM11 MPCore: cpu_v6_dcache_clean_area needs RFO

[George G. Davis]

On Jun 13, 2012, at 5:36 AM, Russell King - ARM Linux wrote:

> On Wed, Jun 13, 2012 at 10:32:18AM +0100, Catalin Marinas wrote:
>> Alternatively, we could disable the interrupts around RFO and D-cache
>> cleaning (inside the loop), though I think the preempt disabling is
>> faster.
> 
> It also has the *important* effect that it doesn't cause problems with
> interrupt latency, which is a real concern if you have peripherals
> requiring quick servicing.

Yep, even disabling preemption is not acceptable for some work loads but this
is the least painful option here.

Thanks!

--
Regards,
George