Re: armv7 _dcache_wbinv_all, or _dcache_wb_all ?

2017-07-17 Thread Artturi Alm
On Fri, Jul 14, 2017 at 03:33:32AM +0300, Artturi Alm wrote:
> On Fri, Jul 14, 2017 at 03:28:25AM +0300, Artturi Alm wrote:
> > Hi,
> > 
> > i'm having hard time choosing, between this diff, and the one that
> > renames it to for what it does, which would likely incur cleanup
> > elsewhere. current is just wrong.. but i'm open to be taught on this
> > matter anyway.
> > 
> > -Artturi
> > 
> 
> Wonders of manually recreating a quick diff by hand above.. fixed:
> 
> diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
> index c91108e7066..fd5385043fc 100644
> --- a/sys/arch/arm/arm/cpufunc.c
> +++ b/sys/arch/arm/arm/cpufunc.c
> @@ -291,8 +291,8 @@ armv7_dcache_wbinv_all()
>   for (ways = 0; ways < nways; ways++) {
>   word = wayval | setval | lvl;
>  
> - /* Clean D cache SE with Set/Index */
> - __asm volatile("mcr p15, 0, %0, c7, c10, 2"
> + /* Data cache clean and invalidate by set/way */
> + __asm volatile("mcr p15, 0, %0, c7, c14, 2\n"
>   : : "r" (word));
>   wayval += wayincr;
>   }

Guessing now, that this is just yet another case still prevalent in cvs
tree, where things are the way they are, and need no cleaning/fixing,
because it was written for !MULTIPROCESSOR + does work as is for GENERIC.

As a side note w/some relevancy to this, and more to the "sysreg.h use
in C". This is what i ended up using in some of my branches where i
don't restrict myself from anything, and i find it useful while cleaning:

diff --git a/sys/arch/arm/include/sysreg.h b/sys/arch/arm/include/sysreg.h
index c2aab7d6667..5d48912494a 100644
--- a/sys/arch/arm/include/sysreg.h
+++ b/sys/arch/arm/include/sysreg.h
@@ -269,4 +269,8 @@
  */
 #define CP15_CBAR(rr)  p15, 4, rr, c15, c0, 0 /* Configuration Base 
Address Register */
 
+#define_CP_REG_STR(...)#__VA_ARGS__
+#define_IA_MRC_RD(x)   "mrc" _CP_REG_STR(x) "\n"
+#define_IA_MCR_WR(x)   "mcr" _CP_REG_STR(x) "\n"
+
 #endif /* !MACHINE_SYSREG_H */


that does look like below in use, i think the mcr/mrc was not enough, so
went w/longer and more verbose name with some duplicity being _X_RealOP_OP,
where RealOP is enough to reveal _OP already...

u_int
cpu_id(void)
{
u_int _midr;
asm volatile(_IA_MRC_RD(CP15_MIDR(%0)) : "=r"(_midr));
return _midr;
}

void
cpu_set_tpidrprw(void *_tpidrprw)
{
asm volatile(_IA_MCR_WR(CP15_TPIDRPRW(%0)) :: "r"(_tpidrprw));
}

now im still not suggesting this anymore, just wanted to note why i hit
the naming issue of non-invalidating _wbinv_all() again.
-Artturi



Re: armv7 _dcache_wbinv_all, or _dcache_wb_all ?

2017-07-13 Thread Artturi Alm
On Fri, Jul 14, 2017 at 03:28:25AM +0300, Artturi Alm wrote:
> Hi,
> 
> i'm having hard time choosing, between this diff, and the one that
> renames it to for what it does, which would likely incur cleanup
> elsewhere. current is just wrong.. but i'm open to be taught on this
> matter anyway.
> 
> -Artturi
> 

Wonders of manually recreating a quick diff by hand above.. fixed:

diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
index c91108e7066..fd5385043fc 100644
--- a/sys/arch/arm/arm/cpufunc.c
+++ b/sys/arch/arm/arm/cpufunc.c
@@ -291,8 +291,8 @@ armv7_dcache_wbinv_all()
for (ways = 0; ways < nways; ways++) {
word = wayval | setval | lvl;
 
-   /* Clean D cache SE with Set/Index */
-   __asm volatile("mcr p15, 0, %0, c7, c10, 2"
+   /* Data cache clean and invalidate by set/way */
+   __asm volatile("mcr p15, 0, %0, c7, c14, 2\n"
: : "r" (word));
wayval += wayincr;
}



armv7 _dcache_wbinv_all, or _dcache_wb_all ?

2017-07-13 Thread Artturi Alm
Hi,

i'm having hard time choosing, between this diff, and the one that
renames it to for what it does, which would likely incur cleanup
elsewhere. current is just wrong.. but i'm open to be taught on this
matter anyway.

-Artturi


diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
index c91108e7066..0631ba8fa78 100644
--- a/sys/arch/arm/arm/cpufunc.c
+++ b/sys/arch/arm/arm/cpufunc.c
@@ -291,8 +291,8 @@ armv7_dcache_wbinv_all()
for (ways = 0; ways < nways; ways++) {
word = wayval | setval | lvl;
 
-   /* Clean D cache SE with Set/Index */
-   __asm volatile("mcr p15, 0, %0, c7, c10, 2"
+   /* Data cache clean and invalidate by set/way */
+   __asm volatile("mcr p15, 0, rr, c7, c14, 2\n"
: : "r" (word));
wayval += wayincr;
}