[PATCH V6]: bugfix

[PATCH V6]: bugfix

[Huang Pei]

V6:

+. V5's PATCH 1/2 split into two part, PATCH 1/4 and PATCH 3/4.

+. PATCH 1/4 is the bugfix, PATCH 3/4 is the cleanup.

+. PATCH 4/4 is further improvement.

+. add url of TX3911/TX3912 spec into PATCH 2/4.

[PATCH 1/4] MIPS: fix local_{add,sub}_return on MIPS64

[Huang Pei]

Use "daddu/dsubu" for long int on MIPS64 instead of "addu/subu"

Fixes: 7232311ef14c ("local_t: mips extension")
Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/include/asm/llsc.h  | 4 ++++
 arch/mips/include/asm/local.h | 8 ++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/llsc.h b/arch/mips/include/asm/llsc.h
index ec09fe5d6d6c..8cc28177c37f 100644
--- a/arch/mips/include/asm/llsc.h
+++ b/arch/mips/include/asm/llsc.h
@@ -14,10 +14,14 @@
 #if _MIPS_SZLONG == 32
 #define __LL		"ll	"
 #define __SC		"sc	"
+#define __ADDU		"addu	"
+#define __SUBU		"subu	"
 #define __INS		"ins	"
 #define __EXT		"ext	"
 #elif _MIPS_SZLONG == 64
 #define __LL		"lld	"
+#define __ADDU		"daddu	"
+#define __SUBU		"dsubu	"
 #define __SC		"scd	"
 #define __INS		"dins	"
 #define __EXT		"dext	"
diff --git a/arch/mips/include/asm/local.h b/arch/mips/include/asm/local.h
index ecda7295ddcd..608651d9affe 100644
--- a/arch/mips/include/asm/local.h
+++ b/arch/mips/include/asm/local.h
@@ -39,7 +39,7 @@ static __inline__ long local_add_return(long i, local_t * l)
 		"	.set	arch=r4000				\n"
 			__SYNC(full, loongson3_war) "			\n"
 		"1:"	__LL	"%1, %2		# local_add_return	\n"
-		"	addu	%0, %1, %3				\n"
+			__ADDU	"%0, %1, %3				\n"
 			__SC	"%0, %2					\n"
 		"	beqzl	%0, 1b					\n"
 		"	addu	%0, %1, %3				\n"
@@ -55,7 +55,7 @@ static __inline__ long local_add_return(long i, local_t * l)
 		"	.set	"MIPS_ISA_ARCH_LEVEL"			\n"
 			__SYNC(full, loongson3_war) "			\n"
 		"1:"	__LL	"%1, %2		# local_add_return	\n"
-		"	addu	%0, %1, %3				\n"
+			__ADDU	"%0, %1, %3				\n"
 			__SC	"%0, %2					\n"
 		"	beqz	%0, 1b					\n"
 		"	addu	%0, %1, %3				\n"
@@ -88,7 +88,7 @@ static __inline__ long local_sub_return(long i, local_t * l)
 		"	.set	arch=r4000				\n"
 			__SYNC(full, loongson3_war) "			\n"
 		"1:"	__LL	"%1, %2		# local_sub_return	\n"
-		"	subu	%0, %1, %3				\n"
+			__SUBU	"%0, %1, %3				\n"
 			__SC	"%0, %2					\n"
 		"	beqzl	%0, 1b					\n"
 		"	subu	%0, %1, %3				\n"
@@ -104,7 +104,7 @@ static __inline__ long local_sub_return(long i, local_t * l)
 		"	.set	"MIPS_ISA_ARCH_LEVEL"			\n"
 			__SYNC(full, loongson3_war) "			\n"
 		"1:"	__LL	"%1, %2		# local_sub_return	\n"
-		"	subu	%0, %1, %3				\n"
+			__SUBU	"%0, %1, %3				\n"
 			__SC	"%0, %2					\n"
 		"	beqz	%0, 1b					\n"
 		"	subu	%0, %1, %3				\n"
-- 
2.20.1

Re: [PATCH 1/4] MIPS: fix local_{add,sub}_return on MIPS64

[Thomas Bogendoerfer]

On Wed, Dec 15, 2021 at 04:44:57PM +0800, Huang Pei wrote:
> Use "daddu/dsubu" for long int on MIPS64 instead of "addu/subu"
> 
> Fixes: 7232311ef14c ("local_t: mips extension")
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/include/asm/llsc.h  | 4 ++++
>  arch/mips/include/asm/local.h | 8 ++++----
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/mips/include/asm/llsc.h b/arch/mips/include/asm/llsc.h
> index ec09fe5d6d6c..8cc28177c37f 100644
> --- a/arch/mips/include/asm/llsc.h
> +++ b/arch/mips/include/asm/llsc.h
> @@ -14,10 +14,14 @@
>  #if _MIPS_SZLONG == 32
>  #define __LL		"ll	"
>  #define __SC		"sc	"
> +#define __ADDU		"addu	"
> +#define __SUBU		"subu	"
>  #define __INS		"ins	"
>  #define __EXT		"ext	"
>  #elif _MIPS_SZLONG == 64
>  #define __LL		"lld	"
> +#define __ADDU		"daddu	"
> +#define __SUBU		"dsubu	"
>  #define __SC		"scd	"
>  #define __INS		"dins	"
>  #define __EXT		"dext	"

maybe I wasn't clear enough, I don't want your orginal fix, but use
fix patch using __stringify(LONG_ADDU)/__stringify(LONG_SUBU).

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH 1/4] MIPS: fix local_{add,sub}_return on MIPS64

[Huang Pei]

On Thu, Dec 16, 2021 at 01:49:48PM +0100, Thomas Bogendoerfer wrote:
> On Wed, Dec 15, 2021 at 04:44:57PM +0800, Huang Pei wrote:
> > Use "daddu/dsubu" for long int on MIPS64 instead of "addu/subu"
> > 
> > Fixes: 7232311ef14c ("local_t: mips extension")
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > ---
> >  arch/mips/include/asm/llsc.h  | 4 ++++
> >  arch/mips/include/asm/local.h | 8 ++++----
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/mips/include/asm/llsc.h b/arch/mips/include/asm/llsc.h
> > index ec09fe5d6d6c..8cc28177c37f 100644
> > --- a/arch/mips/include/asm/llsc.h
> > +++ b/arch/mips/include/asm/llsc.h
> > @@ -14,10 +14,14 @@
> >  #if _MIPS_SZLONG == 32
> >  #define __LL		"ll	"
> >  #define __SC		"sc	"
> > +#define __ADDU		"addu	"
> > +#define __SUBU		"subu	"
> >  #define __INS		"ins	"
> >  #define __EXT		"ext	"
> >  #elif _MIPS_SZLONG == 64
> >  #define __LL		"lld	"
> > +#define __ADDU		"daddu	"
> > +#define __SUBU		"dsubu	"
> >  #define __SC		"scd	"
> >  #define __INS		"dins	"
> >  #define __EXT		"dext	"
> 
> maybe I wasn't clear enough, I don't want your orginal fix, but use
> fix patch using __stringify(LONG_ADDU)/__stringify(LONG_SUBU).
> 
> Thomas.
> 
My point is to keep code style in consistency. If you insist, you can
fix it by yourself. It is ok, I don't mind.
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH 1/4] MIPS: fix local_{add,sub}_return on MIPS64

[Thomas Bogendoerfer]

On Sat, Dec 18, 2021 at 11:23:12AM +0800, Huang Pei wrote:
> On Thu, Dec 16, 2021 at 01:49:48PM +0100, Thomas Bogendoerfer wrote:
> > On Wed, Dec 15, 2021 at 04:44:57PM +0800, Huang Pei wrote:
> > > Use "daddu/dsubu" for long int on MIPS64 instead of "addu/subu"
> > > 
> > > Fixes: 7232311ef14c ("local_t: mips extension")
> > > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > > ---
> > >  arch/mips/include/asm/llsc.h  | 4 ++++
> > >  arch/mips/include/asm/local.h | 8 ++++----
> > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/mips/include/asm/llsc.h b/arch/mips/include/asm/llsc.h
> > > index ec09fe5d6d6c..8cc28177c37f 100644
> > > --- a/arch/mips/include/asm/llsc.h
> > > +++ b/arch/mips/include/asm/llsc.h
> > > @@ -14,10 +14,14 @@
> > >  #if _MIPS_SZLONG == 32
> > >  #define __LL		"ll	"
> > >  #define __SC		"sc	"
> > > +#define __ADDU		"addu	"
> > > +#define __SUBU		"subu	"
> > >  #define __INS		"ins	"
> > >  #define __EXT		"ext	"
> > >  #elif _MIPS_SZLONG == 64
> > >  #define __LL		"lld	"
> > > +#define __ADDU		"daddu	"
> > > +#define __SUBU		"dsubu	"
> > >  #define __SC		"scd	"
> > >  #define __INS		"dins	"
> > >  #define __EXT		"dext	"
> > 
> > maybe I wasn't clear enough, I don't want your orginal fix, but use
> > fix patch using __stringify(LONG_ADDU)/__stringify(LONG_SUBU).
> > 
> > Thomas.
> > 
> My point is to keep code style in consistency. If you insist, you can
> fix it by yourself. It is ok, I don't mind.

sorry for causing such frustration on your side.

I've applied

diff --git a/arch/mips/include/asm/local.h b/arch/mips/include/asm/local.h
index ecda7295ddcd..3fa634090388 100644
--- a/arch/mips/include/asm/local.h
+++ b/arch/mips/include/asm/local.h
@@ -5,6 +5,7 @@
 #include <linux/percpu.h>
 #include <linux/bitops.h>
 #include <linux/atomic.h>
+#include <asm/asm.h>
 #include <asm/cmpxchg.h>
 #include <asm/compiler.h>
 #include <asm/war.h>
@@ -39,7 +40,7 @@ static __inline__ long local_add_return(long i, local_t * l)
                "       .set    arch=r4000                              \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_add_return      \n"
-               "       addu    %0, %1, %3                              \n"
+                       __stringify(LONG_ADDU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqzl   %0, 1b                                  \n"
                "       addu    %0, %1, %3                              \n"
@@ -55,7 +56,7 @@ static __inline__ long local_add_return(long i, local_t * l)
                "       .set    "MIPS_ISA_ARCH_LEVEL"                   \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_add_return      \n"
-               "       addu    %0, %1, %3                              \n"
+                       __stringify(LONG_ADDU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqz    %0, 1b                                  \n"
                "       addu    %0, %1, %3                              \n"
@@ -88,7 +89,7 @@ static __inline__ long local_sub_return(long i, local_t * l)
                "       .set    arch=r4000                              \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_sub_return      \n"
-               "       subu    %0, %1, %3                              \n"
+                       __stringify(LONG_SUBU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqzl   %0, 1b                                  \n"
                "       subu    %0, %1, %3                              \n"
@@ -104,7 +105,7 @@ static __inline__ long local_sub_return(long i, local_t * l)
                "       .set    "MIPS_ISA_ARCH_LEVEL"                   \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_sub_return      \n"
-               "       subu    %0, %1, %3                              \n"
+                       __stringify(LONG_SUBU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqz    %0, 1b                                  \n"
                "       subu    %0, %1, %3                              \n"

and the reason is I prefer to keep the changes as small and local 
as possible. This makes the reviews and applying to older trees easier.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

[PATCH 2/4] MIPS: tx39: adjust tx39_flush_cache_page

[Huang Pei]

Indexed cache operation actually uses KSEG0/CKSEG0 (AKA physical
address, see INDEX_BASE in arch/mips/include/asm/r4kcache.h) to
index cache line, so it CAN NOT handle cache alias(cache alias
is first introduced into MIPS by R4000, indexing cache line with
virtual address).

It is said, on "32-Bit TX System TX39 Family TMPR3911/3912", P86,

•Translation Look-aside Buffer (TLB) (4 Kbyte Page size, 32 Entries)
•4Kbyte instruction cache (I-cache)
	•16 bytes (4 words) per line (256 lines total)
	•physical address tag per cache line
	•single valid bit per cache line
	•direct-mapped
•1 Kbyte data cache (D-cache)
	•4bytes (1 word) per line (128 lines total)
	•physical address tag per cache line
	•write-through
	•two-way set associate

We can assume there is NO cache alias on TX39's R3900 core

Anyway, remove checking for cpu_has_dc_aliases, since tx39_*indexed
can not index cache alias, nor there is cache alias on R3900

More info about TX3911/3912, see
https://pdf1.alldatasheet.com/datasheet-pdf/view/211951/TOSHIBA/TMPR
3912.html

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/mm/c-tx39.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c
index 03dfbb40ec73..c2ecdde0371d 100644
--- a/arch/mips/mm/c-tx39.c
+++ b/arch/mips/mm/c-tx39.c
@@ -207,11 +207,12 @@ static void tx39_flush_cache_page(struct vm_area_struct *vma, unsigned long page
 	/*
 	 * Do indexed flush, too much work to get the (possible) TLB refills
 	 * to work correctly.
+	 *
 	 */
-	if (cpu_has_dc_aliases || exec)
+	if (exec) {
 		tx39_blast_dcache_page_indexed(page);
-	if (exec)
 		tx39_blast_icache_page_indexed(page);
+	}
 }
 
 static void local_tx39_flush_data_cache_page(void * addr)
-- 
2.20.1

Re: [PATCH 2/4] MIPS: tx39: adjust tx39_flush_cache_page

[Sergey Shtylyov]

Hello!

On 15.12.2021 11:44, Huang Pei wrote:

> Indexed cache operation actually uses KSEG0/CKSEG0 (AKA physical
> address, see INDEX_BASE in arch/mips/include/asm/r4kcache.h) to
> index cache line, so it CAN NOT handle cache alias(cache alias
> is first introduced into MIPS by R4000, indexing cache line with
> virtual address).
> 
> It is said, on "32-Bit TX System TX39 Family TMPR3911/3912", P86,
> 
> •Translation Look-aside Buffer (TLB) (4 Kbyte Page size, 32 Entries)
> •4Kbyte instruction cache (I-cache)
> 	•16 bytes (4 words) per line (256 lines total)
> 	•physical address tag per cache line
> 	•single valid bit per cache line
> 	•direct-mapped
> •1 Kbyte data cache (D-cache)
> 	•4bytes (1 word) per line (128 lines total)
> 	•physical address tag per cache line
> 	•write-through
> 	•two-way set associate
> 
> We can assume there is NO cache alias on TX39's R3900 core
> 
> Anyway, remove checking for cpu_has_dc_aliases, since tx39_*indexed
> can not index cache alias, nor there is cache alias on R3900
> 
> More info about TX3911/3912, see
> https://pdf1.alldatasheet.com/datasheet-pdf/view/211951/TOSHIBA/TMPR
> 3912.html
> 
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>   arch/mips/mm/c-tx39.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c
> index 03dfbb40ec73..c2ecdde0371d 100644
> --- a/arch/mips/mm/c-tx39.c
> +++ b/arch/mips/mm/c-tx39.c
> @@ -207,11 +207,12 @@ static void tx39_flush_cache_page(struct vm_area_struct *vma, unsigned long page
>   	/*
>   	 * Do indexed flush, too much work to get the (possible) TLB refills
>   	 * to work correctly.
> +	 *

    Why?

[...]

MBR, Sergey

Re: [PATCH 2/4] MIPS: tx39: adjust tx39_flush_cache_page

[Thomas Bogendoerfer]

On Wed, Dec 15, 2021 at 04:44:58PM +0800, Huang Pei wrote:
> Indexed cache operation actually uses KSEG0/CKSEG0 (AKA physical
> address, see INDEX_BASE in arch/mips/include/asm/r4kcache.h) to
> index cache line, so it CAN NOT handle cache alias(cache alias
> is first introduced into MIPS by R4000, indexing cache line with
> virtual address).
> 
> It is said, on "32-Bit TX System TX39 Family TMPR3911/3912", P86,
> 
> •Translation Look-aside Buffer (TLB) (4 Kbyte Page size, 32 Entries)
> •4Kbyte instruction cache (I-cache)
> 	•16 bytes (4 words) per line (256 lines total)
> 	•physical address tag per cache line
> 	•single valid bit per cache line
> 	•direct-mapped
> •1 Kbyte data cache (D-cache)
> 	•4bytes (1 word) per line (128 lines total)
> 	•physical address tag per cache line
> 	•write-through
> 	•two-way set associate
> 
> We can assume there is NO cache alias on TX39's R3900 core

in the same sense the whole cache flushing magic isn't needed and
we could to the same as for pure R3k CPUs. But this code is there
and none of the user manuals I found describe excat cache behaviour.

I've planned to retire the whole tx39 soon, so please no more patches
for it. 

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH 2/4] MIPS: tx39: adjust tx39_flush_cache_page

[Huang Pei]

On Thu, Dec 16, 2021 at 01:52:04PM +0100, Thomas Bogendoerfer wrote:
> On Wed, Dec 15, 2021 at 04:44:58PM +0800, Huang Pei wrote:
> > Indexed cache operation actually uses KSEG0/CKSEG0 (AKA physical
> > address, see INDEX_BASE in arch/mips/include/asm/r4kcache.h) to
> > index cache line, so it CAN NOT handle cache alias(cache alias
> > is first introduced into MIPS by R4000, indexing cache line with
> > virtual address).
> > 
> > It is said, on "32-Bit TX System TX39 Family TMPR3911/3912", P86,
> > 
> > •Translation Look-aside Buffer (TLB) (4 Kbyte Page size, 32 Entries)
> > •4Kbyte instruction cache (I-cache)
> > 	•16 bytes (4 words) per line (256 lines total)
> > 	•physical address tag per cache line
> > 	•single valid bit per cache line
> > 	•direct-mapped
> > •1 Kbyte data cache (D-cache)
> > 	•4bytes (1 word) per line (128 lines total)
> > 	•physical address tag per cache line
> > 	•write-through
> > 	•two-way set associate
> > 
> > We can assume there is NO cache alias on TX39's R3900 core
> 
> in the same sense the whole cache flushing magic isn't needed and
> we could to the same as for pure R3k CPUs. But this code is there
> and none of the user manuals I found describe excat cache behaviour.
> 
the original code is odd, I think this may be better, if we do not need
to consider cache alias

-       if (cpu_has_dc_aliases || exec)
-               tx39_blast_dcache_page_indexed(page);
+       tx39_blast_dcache_page_indexed(page);
        if (exec)
	        tx39_blast_icache_page_indexed(page);

> I've planned to retire the whole tx39 soon, so please no more patches
> for it. 
> 

Ok, I can wait. I need more test of my patch on reviving
f685a533a7f ("MIPS: make userspace mapping young by default")
> Thomas.

> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

[PATCH 3/4] MIPS: rework local_t operation on MIPS64

[Huang Pei]

+. remove "asm/war.h" since R10000_LLSC_WAR became a config option

+. clean up

Suggested-by:  Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/include/asm/asm.h   | 18 ++++++++++
 arch/mips/include/asm/local.h | 62 +++++++++--------------------------
 2 files changed, 33 insertions(+), 47 deletions(-)

diff --git a/arch/mips/include/asm/asm.h b/arch/mips/include/asm/asm.h
index 2f8ce94ebaaf..f3302b13d3e0 100644
--- a/arch/mips/include/asm/asm.h
+++ b/arch/mips/include/asm/asm.h
@@ -19,6 +19,7 @@
 
 #include <asm/sgidefs.h>
 #include <asm/asm-eva.h>
+#include <asm/isa-rev.h>
 
 #ifndef __VDSO__
 /*
@@ -211,6 +212,8 @@ symbol		=	value
 #define LONG_SUB	sub
 #define LONG_SUBU	subu
 #define LONG_L		lw
+#define LONG_LL		ll
+#define LONG_SC		sc
 #define LONG_S		sw
 #define LONG_SP		swp
 #define LONG_SLL	sll
@@ -236,6 +239,8 @@ symbol		=	value
 #define LONG_SUB	dsub
 #define LONG_SUBU	dsubu
 #define LONG_L		ld
+#define LONG_LL		lld
+#define LONG_SC		scd
 #define LONG_S		sd
 #define LONG_SP		sdp
 #define LONG_SLL	dsll
@@ -320,6 +325,19 @@ symbol		=	value
 
 #define SSNOP		sll zero, zero, 1
 
+/*
+ * Using a branch-likely instruction to check the result of an sc instruction
+ * works around a bug present in R10000 CPUs prior to revision 3.0 that could
+ * cause ll-sc sequences to execute non-atomically.
+ */
+#ifdef CONFIG_WAR_R10000_LLSC
+# define SC_BEQZ	beqzl
+#elif MIPS_ISA_REV >= 6
+# define SC_BEQZ	beqzc
+#else
+# define SC_BEQZ	beqz
+#endif
+
 #ifdef CONFIG_SGI_IP28
 /* Inhibit speculative stores to volatile (e.g.DMA) or invalid addresses. */
 #include <asm/cacheops.h>
diff --git a/arch/mips/include/asm/local.h b/arch/mips/include/asm/local.h
index 608651d9affe..c1e109357110 100644
--- a/arch/mips/include/asm/local.h
+++ b/arch/mips/include/asm/local.h
@@ -7,7 +7,7 @@
 #include <linux/atomic.h>
 #include <asm/cmpxchg.h>
 #include <asm/compiler.h>
-#include <asm/war.h>
+#include <asm/asm.h>
 
 typedef struct
 {
@@ -31,34 +31,18 @@ static __inline__ long local_add_return(long i, local_t * l)
 {
 	unsigned long result;
 
-	if (kernel_uses_llsc && IS_ENABLED(CONFIG_WAR_R10000_LLSC)) {
-		unsigned long temp;
-
-		__asm__ __volatile__(
-		"	.set	push					\n"
-		"	.set	arch=r4000				\n"
-			__SYNC(full, loongson3_war) "			\n"
-		"1:"	__LL	"%1, %2		# local_add_return	\n"
-			__ADDU	"%0, %1, %3				\n"
-			__SC	"%0, %2					\n"
-		"	beqzl	%0, 1b					\n"
-		"	addu	%0, %1, %3				\n"
-		"	.set	pop					\n"
-		: "=&r" (result), "=&r" (temp), "=m" (l->a.counter)
-		: "Ir" (i), "m" (l->a.counter)
-		: "memory");
-	} else if (kernel_uses_llsc) {
+	if (kernel_uses_llsc) {
 		unsigned long temp;
 
 		__asm__ __volatile__(
 		"	.set	push					\n"
 		"	.set	"MIPS_ISA_ARCH_LEVEL"			\n"
-			__SYNC(full, loongson3_war) "			\n"
-		"1:"	__LL	"%1, %2		# local_add_return	\n"
-			__ADDU	"%0, %1, %3				\n"
-			__SC	"%0, %2					\n"
-		"	beqz	%0, 1b					\n"
-		"	addu	%0, %1, %3				\n"
+			__SYNC(full, loongson3_war) "                   \n"
+		"1:"	__stringify(LONG_LL)	"	%1, %2		\n"
+		"	"__stringify(LONG_ADDU)	"	%0, %1, %3	\n"
+		"	"__stringify(LONG_SC)	"	%0, %2		\n"
+		"	"__stringify(SC_BEQZ)	"	%0, 1b		\n"
+		"	"__stringify(LONG_ADDU)	"	%0, %1, %3	\n"
 		"	.set	pop					\n"
 		: "=&r" (result), "=&r" (temp), "=m" (l->a.counter)
 		: "Ir" (i), "m" (l->a.counter)
@@ -80,34 +64,18 @@ static __inline__ long local_sub_return(long i, local_t * l)
 {
 	unsigned long result;
 
-	if (kernel_uses_llsc && IS_ENABLED(CONFIG_WAR_R10000_LLSC)) {
-		unsigned long temp;
-
-		__asm__ __volatile__(
-		"	.set	push					\n"
-		"	.set	arch=r4000				\n"
-			__SYNC(full, loongson3_war) "			\n"
-		"1:"	__LL	"%1, %2		# local_sub_return	\n"
-			__SUBU	"%0, %1, %3				\n"
-			__SC	"%0, %2					\n"
-		"	beqzl	%0, 1b					\n"
-		"	subu	%0, %1, %3				\n"
-		"	.set	pop					\n"
-		: "=&r" (result), "=&r" (temp), "=m" (l->a.counter)
-		: "Ir" (i), "m" (l->a.counter)
-		: "memory");
-	} else if (kernel_uses_llsc) {
+	if (kernel_uses_llsc) {
 		unsigned long temp;
 
 		__asm__ __volatile__(
 		"	.set	push					\n"
 		"	.set	"MIPS_ISA_ARCH_LEVEL"			\n"
-			__SYNC(full, loongson3_war) "			\n"
-		"1:"	__LL	"%1, %2		# local_sub_return	\n"
-			__SUBU	"%0, %1, %3				\n"
-			__SC	"%0, %2					\n"
-		"	beqz	%0, 1b					\n"
-		"	subu	%0, %1, %3				\n"
+			__SYNC(full, loongson3_war) "                   \n"
+		"1:"	__stringify(LONG_LL)	"	%1, %2		\n"
+		"	"__stringify(LONG_SUBU)	"	%0, %1, %3	\n"
+		"	"__stringify(LONG_SC)	"	%0, %2		\n"
+		"	"__stringify(SC_BEQZ)	"	%0, 1b		\n"
+		"	"__stringify(LONG_SUBU)	"	%0, %1, %3	\n"
 		"	.set	pop					\n"
 		: "=&r" (result), "=&r" (temp), "=m" (l->a.counter)
 		: "Ir" (i), "m" (l->a.counter)
-- 
2.20.1

Re: [PATCH 3/4] MIPS: rework local_t operation on MIPS64

[Thomas Bogendoerfer]

On Wed, Dec 15, 2021 at 04:44:59PM +0800, Huang Pei wrote:
> +. remove "asm/war.h" since R10000_LLSC_WAR became a config option
> 
> +. clean up
> 
> Suggested-by:  Maciej W. Rozycki <macro@orcam.me.uk>
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/include/asm/asm.h   | 18 ++++++++++
>  arch/mips/include/asm/local.h | 62 +++++++++--------------------------
>  2 files changed, 33 insertions(+), 47 deletions(-)

applied to mips-next adapted to the changed first patch.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

[PATCH 4/4] MIPS: retire "asm/llsc.h"

[Huang Pei]

all that "asm/llsc.h" does is just to help inline asm, which can be
stringifyed from "asm/asm.h"

+. Since "asm/asm.h" has all we need, retire "asm/llsc.h"

+. remove unused header file

Inspired-by:   Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/include/asm/asm.h      |  4 +++
 arch/mips/include/asm/atomic.h   | 10 +++-----
 arch/mips/include/asm/bitops.h   | 24 ++++++++----------
 arch/mips/include/asm/cmpxchg.h  |  8 +++---
 arch/mips/include/asm/kvm_host.h | 12 ++++-----
 arch/mips/include/asm/llsc.h     | 43 --------------------------------
 6 files changed, 28 insertions(+), 73 deletions(-)
 delete mode 100644 arch/mips/include/asm/llsc.h

diff --git a/arch/mips/include/asm/asm.h b/arch/mips/include/asm/asm.h
index f3302b13d3e0..ed74a6032ec8 100644
--- a/arch/mips/include/asm/asm.h
+++ b/arch/mips/include/asm/asm.h
@@ -182,6 +182,8 @@ symbol		=	value
 #define INT_SRLV	srlv
 #define INT_SRA		sra
 #define INT_SRAV	srav
+#define LONG_INS	ins
+#define LONG_EXT	ext
 #endif
 
 #if (_MIPS_SZINT == 64)
@@ -199,6 +201,8 @@ symbol		=	value
 #define INT_SRLV	dsrlv
 #define INT_SRA		dsra
 #define INT_SRAV	dsrav
+#define LONG_INS	dins
+#define LONG_EXT	dext
 #endif
 
 /*
diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index a0b9e7c1e4fc..77ba1e36701f 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -20,9 +20,7 @@
 #include <asm/compiler.h>
 #include <asm/cpu-features.h>
 #include <asm/cmpxchg.h>
-#include <asm/llsc.h>
 #include <asm/sync.h>
-#include <asm/war.h>
 
 #define ATOMIC_OPS(pfx, type)						\
 static __always_inline type arch_##pfx##_read(const pfx##_t *v)		\
@@ -74,7 +72,7 @@ static __inline__ void arch_##pfx##_##op(type i, pfx##_t * v)		\
 	"1:	" #ll "	%0, %1		# " #pfx "_" #op "	\n"	\
 	"	" #asm_op " %0, %2				\n"	\
 	"	" #sc "	%0, %1					\n"	\
-	"\t" __SC_BEQZ "%0, 1b					\n"	\
+	"\t" __stringify(SC_BEQZ) "	%0, 1b			\n"	\
 	"	.set	pop					\n"	\
 	: "=&r" (temp), "+" GCC_OFF_SMALL_ASM() (v->counter)		\
 	: "Ir" (i) : __LLSC_CLOBBER);					\
@@ -104,7 +102,7 @@ arch_##pfx##_##op##_return_relaxed(type i, pfx##_t * v)			\
 	"1:	" #ll "	%1, %2		# " #pfx "_" #op "_return\n"	\
 	"	" #asm_op " %0, %1, %3				\n"	\
 	"	" #sc "	%0, %2					\n"	\
-	"\t" __SC_BEQZ "%0, 1b					\n"	\
+	"\t" __stringify(SC_BEQZ) "	%0, 1b			\n"	\
 	"	" #asm_op " %0, %1, %3				\n"	\
 	"	.set	pop					\n"	\
 	: "=&r" (result), "=&r" (temp),					\
@@ -137,7 +135,7 @@ arch_##pfx##_fetch_##op##_relaxed(type i, pfx##_t * v)			\
 	"1:	" #ll "	%1, %2		# " #pfx "_fetch_" #op "\n"	\
 	"	" #asm_op " %0, %1, %3				\n"	\
 	"	" #sc "	%0, %2					\n"	\
-	"\t" __SC_BEQZ "%0, 1b					\n"	\
+	"\t" __stringify(SC_BEQZ) "	%0, 1b			\n"	\
 	"	.set	pop					\n"	\
 	"	move	%0, %1					\n"	\
 	: "=&r" (result), "=&r" (temp),					\
@@ -237,7 +235,7 @@ static __inline__ type arch_##pfx##_sub_if_positive(type i, pfx##_t * v)	\
 	"	.set	push					\n"	\
 	"	.set	" MIPS_ISA_LEVEL "			\n"	\
 	"	" #sc "	%1, %2					\n"	\
-	"	" __SC_BEQZ "%1, 1b				\n"	\
+	"	" __stringify(SC_BEQZ) "	%1, 1b		\n"	\
 	"2:	" __SYNC(full, loongson3_war) "			\n"	\
 	"	.set	pop					\n"	\
 	: "=&r" (result), "=&r" (temp),					\
diff --git a/arch/mips/include/asm/bitops.h b/arch/mips/include/asm/bitops.h
index dc2a6234dd3c..3812082b8295 100644
--- a/arch/mips/include/asm/bitops.h
+++ b/arch/mips/include/asm/bitops.h
@@ -16,14 +16,12 @@
 #include <linux/bits.h>
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <asm/asm.h>
 #include <asm/barrier.h>
 #include <asm/byteorder.h>		/* sigh ... */
 #include <asm/compiler.h>
 #include <asm/cpu-features.h>
-#include <asm/isa-rev.h>
-#include <asm/llsc.h>
 #include <asm/sgidefs.h>
-#include <asm/war.h>
 
 #define __bit_op(mem, insn, inputs...) do {			\
 	unsigned long __temp;					\
@@ -32,10 +30,10 @@
 	"	.set		push			\n"	\
 	"	.set		" MIPS_ISA_LEVEL "	\n"	\
 	"	" __SYNC(full, loongson3_war) "		\n"	\
-	"1:	" __LL		"%0, %1			\n"	\
+	"1:	" __stringify(LONG_LL)	"	%0, %1	\n"	\
 	"	" insn		"			\n"	\
-	"	" __SC		"%0, %1			\n"	\
-	"	" __SC_BEQZ	"%0, 1b			\n"	\
+	"	" __stringify(LONG_SC)	"	%0, %1	\n"	\
+	"	" __stringify(SC_BEQZ)	"	%0, 1b	\n"	\
 	"	.set		pop			\n"	\
 	: "=&r"(__temp), "+" GCC_OFF_SMALL_ASM()(mem)		\
 	: inputs						\
@@ -49,10 +47,10 @@
 	"	.set		push			\n"	\
 	"	.set		" MIPS_ISA_LEVEL "	\n"	\
 	"	" __SYNC(full, loongson3_war) "		\n"	\
-	"1:	" __LL		ll_dst ", %2		\n"	\
+	"1:	" __stringify(LONG_LL) " "	ll_dst ", %2\n"	\
 	"	" insn		"			\n"	\
-	"	" __SC		"%1, %2			\n"	\
-	"	" __SC_BEQZ	"%1, 1b			\n"	\
+	"	" __stringify(LONG_SC)	"	%1, %2	\n"	\
+	"	" __stringify(SC_BEQZ)	"	%1, 1b	\n"	\
 	"	.set		pop			\n"	\
 	: "=&r"(__orig), "=&r"(__temp),				\
 	  "+" GCC_OFF_SMALL_ASM()(mem)				\
@@ -98,7 +96,7 @@ static inline void set_bit(unsigned long nr, volatile unsigned long *addr)
 	}
 
 	if ((MIPS_ISA_REV >= 2) && __builtin_constant_p(bit) && (bit >= 16)) {
-		__bit_op(*m, __INS "%0, %3, %2, 1", "i"(bit), "r"(~0));
+		__bit_op(*m, __stringify(LONG_INS) " %0, %3, %2, 1", "i"(bit), "r"(~0));
 		return;
 	}
 
@@ -126,7 +124,7 @@ static inline void clear_bit(unsigned long nr, volatile unsigned long *addr)
 	}
 
 	if ((MIPS_ISA_REV >= 2) && __builtin_constant_p(bit)) {
-		__bit_op(*m, __INS "%0, $0, %2, 1", "i"(bit));
+		__bit_op(*m, __stringify(LONG_INS) " %0, $0, %2, 1", "i"(bit));
 		return;
 	}
 
@@ -234,8 +232,8 @@ static inline int test_and_clear_bit(unsigned long nr,
 		res = __mips_test_and_clear_bit(nr, addr);
 	} else if ((MIPS_ISA_REV >= 2) && __builtin_constant_p(nr)) {
 		res = __test_bit_op(*m, "%1",
-				    __EXT "%0, %1, %3, 1;"
-				    __INS "%1, $0, %3, 1",
+				    __stringify(LONG_EXT) " %0, %1, %3, 1;"
+				    __stringify(LONG_INS) " %1, $0, %3, 1",
 				    "i"(bit));
 	} else {
 		orig = __test_bit_op(*m, "%0",
diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 66a8b293fd80..b47a5e49f519 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -11,9 +11,7 @@
 #include <linux/bug.h>
 #include <linux/irqflags.h>
 #include <asm/compiler.h>
-#include <asm/llsc.h>
 #include <asm/sync.h>
-#include <asm/war.h>
 
 /*
  * These functions doesn't exist, so if they are called you'll either:
@@ -48,7 +46,7 @@ extern unsigned long __xchg_called_with_bad_pointer(void)
 		"	move	$1, %z3				\n"	\
 		"	.set	" MIPS_ISA_ARCH_LEVEL "		\n"	\
 		"	" st "	$1, %1				\n"	\
-		"\t" __SC_BEQZ	"$1, 1b				\n"	\
+		"\t" __stringify(SC_BEQZ)	"	$1, 1b	\n"	\
 		"	.set	pop				\n"	\
 		: "=&r" (__ret), "=" GCC_OFF_SMALL_ASM() (*m)		\
 		: GCC_OFF_SMALL_ASM() (*m), "Jr" (val)			\
@@ -127,7 +125,7 @@ unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
 		"	move	$1, %z4				\n"	\
 		"	.set	"MIPS_ISA_ARCH_LEVEL"		\n"	\
 		"	" st "	$1, %1				\n"	\
-		"\t" __SC_BEQZ	"$1, 1b				\n"	\
+		"\t" __stringify(SC_BEQZ)	"	$1, 1b	\n"	\
 		"	.set	pop				\n"	\
 		"2:	" __SYNC(full, loongson3_war) "		\n"	\
 		: "=&r" (__ret), "=" GCC_OFF_SMALL_ASM() (*m)		\
@@ -282,7 +280,7 @@ static inline unsigned long __cmpxchg64(volatile void *ptr,
 	/* Attempt to store new at ptr */
 	"	scd	%L1, %2				\n"
 	/* If we failed, loop! */
-	"\t" __SC_BEQZ "%L1, 1b				\n"
+	"\t" __stringify(SC_BEQZ) "	%L1, 1b		\n"
 	"2:	" __SYNC(full, loongson3_war) "		\n"
 	"	.set	pop				\n"
 	: "=&r"(ret),
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 696f6b009377..bfe6d9e48dbf 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -379,9 +379,9 @@ static inline void _kvm_atomic_set_c0_guest_reg(unsigned long *reg,
 		__asm__ __volatile__(
 		"	.set	push				\n"
 		"	.set	"MIPS_ISA_ARCH_LEVEL"		\n"
-		"	" __LL "%0, %1				\n"
+		"	"__stringify(LONG_LL)	" %0, %1	\n"
 		"	or	%0, %2				\n"
-		"	" __SC	"%0, %1				\n"
+		"	"__stringify(LONG_SC)	" %0, %1	\n"
 		"	.set	pop				\n"
 		: "=&r" (temp), "+m" (*reg)
 		: "r" (val));
@@ -396,9 +396,9 @@ static inline void _kvm_atomic_clear_c0_guest_reg(unsigned long *reg,
 		__asm__ __volatile__(
 		"	.set	push				\n"
 		"	.set	"MIPS_ISA_ARCH_LEVEL"		\n"
-		"	" __LL "%0, %1				\n"
+		"	"__stringify(LONG_LL)	" %0, %1	\n"
 		"	and	%0, %2				\n"
-		"	" __SC	"%0, %1				\n"
+		"	"__stringify(LONG_SC)	" %0, %1	\n"
 		"	.set	pop				\n"
 		: "=&r" (temp), "+m" (*reg)
 		: "r" (~val));
@@ -414,10 +414,10 @@ static inline void _kvm_atomic_change_c0_guest_reg(unsigned long *reg,
 		__asm__ __volatile__(
 		"	.set	push				\n"
 		"	.set	"MIPS_ISA_ARCH_LEVEL"		\n"
-		"	" __LL "%0, %1				\n"
+		"	"__stringify(LONG_LL)	" %0, %1	\n"
 		"	and	%0, %2				\n"
 		"	or	%0, %3				\n"
-		"	" __SC	"%0, %1				\n"
+		"	"__stringify(LONG_SC)	" %0, %1	\n"
 		"	.set	pop				\n"
 		: "=&r" (temp), "+m" (*reg)
 		: "r" (~change), "r" (val & change));
diff --git a/arch/mips/include/asm/llsc.h b/arch/mips/include/asm/llsc.h
deleted file mode 100644
index 8cc28177c37f..000000000000
--- a/arch/mips/include/asm/llsc.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
- *
- * Macros for 32/64-bit neutral inline assembler
- */
-
-#ifndef __ASM_LLSC_H
-#define __ASM_LLSC_H
-
-#include <asm/isa-rev.h>
-
-#if _MIPS_SZLONG == 32
-#define __LL		"ll	"
-#define __SC		"sc	"
-#define __ADDU		"addu	"
-#define __SUBU		"subu	"
-#define __INS		"ins	"
-#define __EXT		"ext	"
-#elif _MIPS_SZLONG == 64
-#define __LL		"lld	"
-#define __ADDU		"daddu	"
-#define __SUBU		"dsubu	"
-#define __SC		"scd	"
-#define __INS		"dins	"
-#define __EXT		"dext	"
-#endif
-
-/*
- * Using a branch-likely instruction to check the result of an sc instruction
- * works around a bug present in R10000 CPUs prior to revision 3.0 that could
- * cause ll-sc sequences to execute non-atomically.
- */
-#ifdef CONFIG_WAR_R10000_LLSC
-# define __SC_BEQZ "beqzl	"
-#elif MIPS_ISA_REV >= 6
-# define __SC_BEQZ "beqzc	"
-#else
-# define __SC_BEQZ "beqz	"
-#endif
-
-#endif /* __ASM_LLSC_H  */
-- 
2.20.1

Re: [PATCH 4/4] MIPS: retire "asm/llsc.h"

[Thomas Bogendoerfer]

On Wed, Dec 15, 2021 at 04:45:00PM +0800, Huang Pei wrote:
> all that "asm/llsc.h" does is just to help inline asm, which can be
> stringifyed from "asm/asm.h"
> 
> +. Since "asm/asm.h" has all we need, retire "asm/llsc.h"
> 
> +. remove unused header file
> 
> Inspired-by:   Maciej W. Rozycki <macro@orcam.me.uk>
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/include/asm/asm.h      |  4 +++
>  arch/mips/include/asm/atomic.h   | 10 +++-----
>  arch/mips/include/asm/bitops.h   | 24 ++++++++----------
>  arch/mips/include/asm/cmpxchg.h  |  8 +++---
>  arch/mips/include/asm/kvm_host.h | 12 ++++-----
>  arch/mips/include/asm/llsc.h     | 43 --------------------------------
>  6 files changed, 28 insertions(+), 73 deletions(-)
>  delete mode 100644 arch/mips/include/asm/llsc.h
> 
> diff --git a/arch/mips/include/asm/asm.h b/arch/mips/include/asm/asm.h
> index f3302b13d3e0..ed74a6032ec8 100644
> --- a/arch/mips/include/asm/asm.h
> +++ b/arch/mips/include/asm/asm.h
> @@ -182,6 +182,8 @@ symbol		=	value
>  #define INT_SRLV	srlv
>  #define INT_SRA		sra
>  #define INT_SRAV	srav
> +#define LONG_INS	ins
> +#define LONG_EXT	ext
>  #endif
>  
>  #if (_MIPS_SZINT == 64)
> @@ -199,6 +201,8 @@ symbol		=	value
>  #define INT_SRLV	dsrlv
>  #define INT_SRA		dsra
>  #define INT_SRAV	dsrav
> +#define LONG_INS	dins
> +#define LONG_EXT	dext
>  #endif

this is the wrong place for the defines. I wonder how you compiled it
as it blew up a loongson64 build.

> diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
> index a0b9e7c1e4fc..77ba1e36701f 100644
> --- a/arch/mips/include/asm/atomic.h
> +++ b/arch/mips/include/asm/atomic.h
> @@ -20,9 +20,7 @@
>  #include <asm/compiler.h>
>  #include <asm/cpu-features.h>
>  #include <asm/cmpxchg.h>
> -#include <asm/llsc.h>
>  #include <asm/sync.h>
> -#include <asm/war.h>

I've added an #include <asm/asm.h> as this file is now using defines
out of it.

> diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
> index 66a8b293fd80..b47a5e49f519 100644
> --- a/arch/mips/include/asm/cmpxchg.h
> +++ b/arch/mips/include/asm/cmpxchg.h
> @@ -11,9 +11,7 @@
>  #include <linux/bug.h>
>  #include <linux/irqflags.h>
>  #include <asm/compiler.h>
> -#include <asm/llsc.h>
>  #include <asm/sync.h>
> -#include <asm/war.h>

same here. This also caused compile errors...

applied to mips-next with the compile fixes.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]