Hi Joachim,
On 3/7/19 5:43 am, Joachim Dietrich wrote:
thank you for your explanations.
From: Joachim Dietrich <jo.dietrich@xxxxxx>
There are no separate supervisor and user stack pointers on older
Coldfire CPUs such as the mcf5329.
We, therefore, must enable the use of software copies which is done by
selecting CONFIG_COLDFIRE_SW_A7, else the first user process has a wrong
stack pointer.
Signed-off-by: Joachim Dietrich <jo.dietrich@xxxxxx>
Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
---
I (Geert) am not that super familiar with the CONFIG_COLDFIRE_SW_A7
handling. According to Section 3.2.3 ("Supervisor/User Stack Pointers
(A7 and OTHER_A7)") of MCF5329RM.pdf[1], mcf5329 does have separate
stack pointers, but they're not USP and SSP, like on classic m68k and
newer Coldfire parts.
Yes, I know he's right. So my description is probably not correct. But
I'm a little bit confused about the stack pointer handling of the v3
The stack pointer handling (or really the presence of A7 and other-A7)
can't be determined only by knowing the version core. There are v3
cores that don't have both A7 registers and some that do.
coldfire, because in the reference manual of the mcf5329 stands: "To
support dual stack pointers, the following two supervisor instructions
are included in the ColdFire instruction set architecture to load/store
the USP:
move.l Ay,USP;move to USP
move.l USP,Ax;move from USP"
And that's what the CONFIG_COLDFIRE_SW_A7 code does in entry.h.
Furthermore, in earlier versions of uclinux, e.g kernel 2.6.26, this was
the default handling for the mcf5329.
That was certainly true of older kernels. I added support for using
both A7 registers in later kernels (I don't remember the exact version
I included it). The addition of 2 A7 registers and supporting instructions
was introduced in the ColdFire ISA_A+ version of the instruction set.
(So generally speaking old ColdFire parts don't have them, newer ones do).
That support introduced the CONFIG_COLDFIRE_SW_A7 define.
Hence mcf5329 differs from e.g. mcf5206[2], which has a single unified
stack pointer, which is what CONFIG_COLDFIRE_SW_A7 is designed for.
I don't know if this applies to all mcf532x variants.
It's quite possible CONFIG_COLDFIRE_SW_A7 is the correct solution for
mcf5329, or perhaps it needs some special handling for A7 vs. OTHER_A7?
Perhaps there's a better or more correct handling for the stack
pointers, but without CONFIG_COLDFIRE_SW_A7 my kernel (4.19.15) fails at
rdusp() and wdusp() in processor.h and my first user process has a
wrong sp.
The 5329 supports ColdFire ISA_A+ so it definitely has the A7
and other-A7 support. And is is implemented the same on all ColdFire
parts that support it.
The trick with the dual A7 support is that you have to enable it
in the Cache Control Register (CACR), the EUSP bit. Otherwise you
get traps on those move to and from USP instructions - like what
you are seeing.
So my guess is that CACR is not being setup properly. It is set via
the startup code in arch/m68k/coldfire/head.S - based on whether
CONFIG_COLDFIRE_SW_A7 is defined (see arch/m68k/include/asm/m53xxacr.h).
Can you double check that the CACR register is being set with
the EUSP bit (bit 4) set?
OK. I will do that. But at first glance everything looks right (in the
code). I'm afraid I'm going to need more analysis on the behavior.
I'll get back to you.
Looking into this further I think I can see at least one problem -
directly related to v3 core cache handling. And this could be
what you are seeing.
The CACR register also has bits to invalidate and flush the caches.
And we use that in the cache control functions in arch/m68k/include/asm/cacheflush_no.h
For the v3 cores with the definitions set the way they are we
will be over-writing the CACR value - loosing the EUSP bit setting.
Can you try the patch below?
It will maintain the CACR value while flushing caches.
The most widely used v3 core, the 5307, won't be effected since it
does not support separate A7 registers. The handling of this for
other version cores (v2 and v4) does it correctly already.
Regards
Greg
diff --git a/arch/m68k/include/asm/m53xxacr.h b/arch/m68k/include/asm/m53xxacr.h
index 9138a624c5c8..25b144495234 100644
--- a/arch/m68k/include/asm/m53xxacr.h
+++ b/arch/m68k/include/asm/m53xxacr.h
@@ -89,9 +89,9 @@
* coherency though in all cases. And for copyback caches we will need
* to push cached data as well.
*/
-#define CACHE_INIT CACR_CINVA
-#define CACHE_INVALIDATE CACR_CINVA
-#define CACHE_INVALIDATED CACR_CINVA
+#define CACHE_INIT (CACHE_MODE + CACR_CINVA)
+#define CACHE_INVALIDATE (CACHE_MODE + CACR_CINVA)
+#define CACHE_INVALIDATED (CACHE_MODE + CACR_CINVA)
#define ACR0_MODE ((CONFIG_RAMBASE & 0xff000000) + \
(0x000f0000) + \
