All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4]: bugfix
@ 2021-11-25 10:59 Huang Pei
  2021-11-25 10:59 ` [PATCH 1/4] MIPS: rework local_t operation on MIPS64 Huang Pei
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Huang Pei @ 2021-11-25 10:59 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen

V4:

+. add more info about TX39 core, and it is safe to remove checking
for cpu_has_dc_alias

+. improve commit message



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] MIPS: rework local_t operation on MIPS64
  2021-11-25 10:59 [PATCH V4]: bugfix Huang Pei
@ 2021-11-25 10:59 ` Huang Pei
  2021-11-25 10:59 ` [PATCH 2/4] MIPS: tx39: fix tx39_flush_cache_page Huang Pei
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Huang Pei @ 2021-11-25 10:59 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Maciej W . Rozycki

+. use "daddu/dsubu" for long int on MIPS64 instead of "addu/subu"

+. 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 ecda7295ddcd..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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] MIPS: tx39: fix tx39_flush_cache_page
  2021-11-25 10:59 [PATCH V4]: bugfix Huang Pei
  2021-11-25 10:59 ` [PATCH 1/4] MIPS: rework local_t operation on MIPS64 Huang Pei
@ 2021-11-25 10:59 ` Huang Pei
  2021-11-25 10:59 ` [PATCH 3/4] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48 Huang Pei
  2021-11-25 10:59 ` [PATCH 4/4] MIPS: loongson64: fix FTLB configuration Huang Pei
  3 siblings, 0 replies; 10+ messages in thread
From: Huang Pei @ 2021-11-25 10:59 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen

Indexed cache operation needs KSEG0 address for safety and assumes
no dcache alias nor high memory, since indexed cache instrcution
CAN NOT handle cache alias

The TX39 core based on MIPS R3000A has 4KB Icache(direct mapped) and
1KB Dcache(2-way associate), and TX39 has no High Memory support till
now,  so it is safe to use Index cache instuction with KSEG0 address
assuming no cache alias nor High Memory under 4KB page size on MIPS32

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

diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c
index 03dfbb40ec73..c7c3dbfe7756 100644
--- a/arch/mips/mm/c-tx39.c
+++ b/arch/mips/mm/c-tx39.c
@@ -170,6 +170,7 @@ static void tx39_flush_cache_page(struct vm_area_struct *vma, unsigned long page
 	struct mm_struct *mm = vma->vm_mm;
 	pmd_t *pmdp;
 	pte_t *ptep;
+	unsigned long vaddr = phys_to_virt(pfn_to_phys(pfn));
 
 	/*
 	 * If ownes no valid ASID yet, cannot possibly have gotten
@@ -207,11 +208,14 @@ 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.
+	 *
+	 * Assuming that tx39 family do not support high memory, nor has
+	 * dcache alias, vaddr can index dcache directly and correctly
 	 */
-	if (cpu_has_dc_aliases || exec)
-		tx39_blast_dcache_page_indexed(page);
-	if (exec)
-		tx39_blast_icache_page_indexed(page);
+	if (exec) {
+		tx39_blast_dcache_page_indexed(vaddr);
+		tx39_blast_icache_page_indexed(vaddr);
+	}
 }
 
 static void local_tx39_flush_data_cache_page(void * addr)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48
  2021-11-25 10:59 [PATCH V4]: bugfix Huang Pei
  2021-11-25 10:59 ` [PATCH 1/4] MIPS: rework local_t operation on MIPS64 Huang Pei
  2021-11-25 10:59 ` [PATCH 2/4] MIPS: tx39: fix tx39_flush_cache_page Huang Pei
@ 2021-11-25 10:59 ` Huang Pei
  2021-11-25 15:55   ` Thomas Bogendoerfer
  2021-11-25 10:59 ` [PATCH 4/4] MIPS: loongson64: fix FTLB configuration Huang Pei
  3 siblings, 1 reply; 10+ messages in thread
From: Huang Pei @ 2021-11-25 10:59 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen

It hangup when booting Loongson 3A1000 with BOTH
CONFIG_PAGE_SIZE_64KB and CONFIG_MIPS_VA_BITS_48, that it turn
out to use 2-level pgtable instead of 3-level. 64KB page size
with 2-level pgtable only cover 42 bits VA, use 3-level pgtable
to cover all 48 bits VA(55 bits)

Fixes: 1e321fa917fb ("MIPS64: Support of at least 48 bits of SEGBITS)
Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index de60ad190057..0215dc1529e9 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3097,7 +3097,7 @@ config STACKTRACE_SUPPORT
 config PGTABLE_LEVELS
 	int
 	default 4 if PAGE_SIZE_4KB && MIPS_VA_BITS_48
-	default 3 if 64BIT && !PAGE_SIZE_64KB
+	default 3 if 64BIT && (!PAGE_SIZE_64KB || MIPS_VA_BITS_48)
 	default 2
 
 config MIPS_AUTO_PFN_OFFSET
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] MIPS: loongson64: fix FTLB configuration
  2021-11-25 10:59 [PATCH V4]: bugfix Huang Pei
                   ` (2 preceding siblings ...)
  2021-11-25 10:59 ` [PATCH 3/4] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48 Huang Pei
@ 2021-11-25 10:59 ` Huang Pei
  2021-11-25 15:55   ` Thomas Bogendoerfer
  3 siblings, 1 reply; 10+ messages in thread
From: Huang Pei @ 2021-11-25 10:59 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen

It turns out that 'decode_configs' -> 'set_ftlb_enable' is called under
c->cputype unset, which leaves FTLB disabled on BOTH 3A2000 and 3A3000

Fix it by calling "decode_configs" after c->cputype is initialized

Fixes: da1bd29742b1 ("MIPS: Loongson64: Probe CPU features via CPUCFG")
Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/kernel/cpu-probe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index ac0e2cfc6d57..24a529c6c4be 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1734,8 +1734,6 @@ static inline void decode_cpucfg(struct cpuinfo_mips *c)
 
 static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
 {
-	decode_configs(c);
-
 	/* All Loongson processors covered here define ExcCode 16 as GSExc. */
 	c->options |= MIPS_CPU_GSEXCEX;
 
@@ -1796,6 +1794,8 @@ static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
 		panic("Unknown Loongson Processor ID!");
 		break;
 	}
+
+	decode_configs(c);
 }
 #else
 static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu) { }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48
  2021-11-25 10:59 ` [PATCH 3/4] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48 Huang Pei
@ 2021-11-25 15:55   ` Thomas Bogendoerfer
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2021-11-25 15:55 UTC (permalink / raw)
  To: Huang Pei
  Cc: ambrosehua, Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton,
	Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen

On Thu, Nov 25, 2021 at 06:59:48PM +0800, Huang Pei wrote:
> It hangup when booting Loongson 3A1000 with BOTH
> CONFIG_PAGE_SIZE_64KB and CONFIG_MIPS_VA_BITS_48, that it turn
> out to use 2-level pgtable instead of 3-level. 64KB page size
> with 2-level pgtable only cover 42 bits VA, use 3-level pgtable
> to cover all 48 bits VA(55 bits)
> 
> Fixes: 1e321fa917fb ("MIPS64: Support of at least 48 bits of SEGBITS)
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index de60ad190057..0215dc1529e9 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3097,7 +3097,7 @@ config STACKTRACE_SUPPORT
>  config PGTABLE_LEVELS
>  	int
>  	default 4 if PAGE_SIZE_4KB && MIPS_VA_BITS_48
> -	default 3 if 64BIT && !PAGE_SIZE_64KB
> +	default 3 if 64BIT && (!PAGE_SIZE_64KB || MIPS_VA_BITS_48)
>  	default 2
>  
>  config MIPS_AUTO_PFN_OFFSET
> -- 
> 2.20.1

applied to mips-fixes.

Thomas.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] MIPS: loongson64: fix FTLB configuration
  2021-11-25 10:59 ` [PATCH 4/4] MIPS: loongson64: fix FTLB configuration Huang Pei
@ 2021-11-25 15:55   ` Thomas Bogendoerfer
  2021-11-26  3:13     ` Huang Pei
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Bogendoerfer @ 2021-11-25 15:55 UTC (permalink / raw)
  To: Huang Pei
  Cc: ambrosehua, Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton,
	Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen

On Thu, Nov 25, 2021 at 06:59:49PM +0800, Huang Pei wrote:
> It turns out that 'decode_configs' -> 'set_ftlb_enable' is called under
> c->cputype unset, which leaves FTLB disabled on BOTH 3A2000 and 3A3000
> 
> Fix it by calling "decode_configs" after c->cputype is initialized
> 
> Fixes: da1bd29742b1 ("MIPS: Loongson64: Probe CPU features via CPUCFG")
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/kernel/cpu-probe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index ac0e2cfc6d57..24a529c6c4be 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -1734,8 +1734,6 @@ static inline void decode_cpucfg(struct cpuinfo_mips *c)
>  
>  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
>  {
> -	decode_configs(c);
> -
>  	/* All Loongson processors covered here define ExcCode 16 as GSExc. */
>  	c->options |= MIPS_CPU_GSEXCEX;
>  
> @@ -1796,6 +1794,8 @@ static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
>  		panic("Unknown Loongson Processor ID!");
>  		break;
>  	}
> +
> +	decode_configs(c);
>  }
>  #else
>  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu) { }
> -- 
> 2.20.1

applied to mips-fixes.

Thomas.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] MIPS: loongson64: fix FTLB configuration
  2021-11-25 15:55   ` Thomas Bogendoerfer
@ 2021-11-26  3:13     ` Huang Pei
  2021-11-30 10:34       ` Thomas Bogendoerfer
  0 siblings, 1 reply; 10+ messages in thread
From: Huang Pei @ 2021-11-26  3:13 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: ambrosehua, Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton,
	Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen

On Thu, Nov 25, 2021 at 04:55:28PM +0100, Thomas Bogendoerfer wrote:
> On Thu, Nov 25, 2021 at 06:59:49PM +0800, Huang Pei wrote:
> > It turns out that 'decode_configs' -> 'set_ftlb_enable' is called under
> > c->cputype unset, which leaves FTLB disabled on BOTH 3A2000 and 3A3000
> > 
> > Fix it by calling "decode_configs" after c->cputype is initialized
> > 
> > Fixes: da1bd29742b1 ("MIPS: Loongson64: Probe CPU features via CPUCFG")
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > ---
> >  arch/mips/kernel/cpu-probe.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> > index ac0e2cfc6d57..24a529c6c4be 100644
> > --- a/arch/mips/kernel/cpu-probe.c
> > +++ b/arch/mips/kernel/cpu-probe.c
> > @@ -1734,8 +1734,6 @@ static inline void decode_cpucfg(struct cpuinfo_mips *c)
> >  
> >  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
> >  {
> > -	decode_configs(c);
> > -
> >  	/* All Loongson processors covered here define ExcCode 16 as GSExc. */
> >  	c->options |= MIPS_CPU_GSEXCEX;
> >  
> > @@ -1796,6 +1794,8 @@ static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
> >  		panic("Unknown Loongson Processor ID!");
> >  		break;
> >  	}
> > +
> > +	decode_configs(c);
> >  }
> >  #else
> >  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu) { }
> > -- 
> > 2.20.1
> 
> applied to mips-fixes.
> 
> Thomas.
> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]
Hi, Thomas,

What about PATCH 1/4, without it, kernel/trace/ring_buffer.i using
local_add_return, like this 
--------------------------------------------------------------------------------
    __asm__ __volatile__(
	"     .set    push                                    \n"
	"     .set    ""arch=r4000""                  \n"
	".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set mips64r2;.rept 1; sync 
	0x00; .endr; .set pop; .else; ; .endif" "\n"
	"1:" "lld     " "%1, %2               # local_add_return\n"
	"     addu    %0, %1, %3				\n"
	"scd " "%0, %2						\n"
	"     beqz    %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 (1) {
  unsigned long temp;

    __asm__ __volatile__(
	    "     .set    push                                    \n"
	    "     .set    ""arch=r4000""                  \n"
	    ".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set
	    mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ;
	    .endif" "                    \n"
	    "1:" "lld     " "%1, %2               # local_add_return
	    \n"
	    "     addu    %0, %1, %3                              \n"
	    "scd " "%0, %2
	    \n"
	    "     beqz    %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");

--------------------------------------------------------------------------------
it is wrong here, "lld" + "addu"

with it, like this 
--------------------------------------------------------------------------------
 if (1) {
  unsigned long temp;

  __asm__ __volatile__(
	  "     .set    push                                    \n"
	  "     .set    ""arch=r4000""                  \n"
	   ".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set mips64r2;
	.rept 1; sync 0x00; .endr; .set pop; .else; ; .endif" "
	\n"
	  "1:" "lld" "  %1, %2          \n"
	  "     ""daddu" "      %0, %1, %3      \n"
	  "     ""scd" "        %0, %2          \n"
	  "     ""beqz" "       %0, 1b          \n"
	  "     ""daddu" "      %0, %1, %3      \n"
	  "     .set    pop                                     \n"
	  : "=&r" (result), "=&r" (temp), "=m" (l->a.counter)
	  : "Ir" (i), "m" (l->a.counter)
	  : "memory");
 } else {

MIPS64 needs "lld + daddu"

and PATCH 2, any comment?

 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] MIPS: loongson64: fix FTLB configuration
  2021-11-26  3:13     ` Huang Pei
@ 2021-11-30 10:34       ` Thomas Bogendoerfer
  2021-11-30 11:42         ` Huang Pei
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Bogendoerfer @ 2021-11-30 10:34 UTC (permalink / raw)
  To: Huang Pei
  Cc: ambrosehua, Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton,
	Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen

On Fri, Nov 26, 2021 at 11:13:19AM +0800, Huang Pei wrote:
> On Thu, Nov 25, 2021 at 04:55:28PM +0100, Thomas Bogendoerfer wrote:
> > On Thu, Nov 25, 2021 at 06:59:49PM +0800, Huang Pei wrote:
> > > It turns out that 'decode_configs' -> 'set_ftlb_enable' is called under
> > > c->cputype unset, which leaves FTLB disabled on BOTH 3A2000 and 3A3000
> > > 
> > > Fix it by calling "decode_configs" after c->cputype is initialized
> > > 
> > > Fixes: da1bd29742b1 ("MIPS: Loongson64: Probe CPU features via CPUCFG")
> > > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > > ---
> > >  arch/mips/kernel/cpu-probe.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> > > index ac0e2cfc6d57..24a529c6c4be 100644
> > > --- a/arch/mips/kernel/cpu-probe.c
> > > +++ b/arch/mips/kernel/cpu-probe.c
> > > @@ -1734,8 +1734,6 @@ static inline void decode_cpucfg(struct cpuinfo_mips *c)
> > >  
> > >  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
> > >  {
> > > -	decode_configs(c);
> > > -
> > >  	/* All Loongson processors covered here define ExcCode 16 as GSExc. */
> > >  	c->options |= MIPS_CPU_GSEXCEX;
> > >  
> > > @@ -1796,6 +1794,8 @@ static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
> > >  		panic("Unknown Loongson Processor ID!");
> > >  		break;
> > >  	}
> > > +
> > > +	decode_configs(c);
> > >  }
> > >  #else
> > >  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu) { }
> > > -- 
> > > 2.20.1
> > 
> > applied to mips-fixes.
> > 
> > Thomas.
> > 
> > -- 
> > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> > good idea.                                                [ RFC1925, 2.3 ]
> Hi, Thomas,
> 
> What about PATCH 1/4, without it, kernel/trace/ring_buffer.i using
> local_add_return, like this 
> --------------------------------------------------------------------------------
>     __asm__ __volatile__(
> 	"     .set    push                                    \n"
> 	"     .set    ""arch=r4000""                  \n"
> 	".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set mips64r2;.rept 1; sync 
> 	0x00; .endr; .set pop; .else; ; .endif" "\n"
> 	"1:" "lld     " "%1, %2               # local_add_return\n"
> 	"     addu    %0, %1, %3				\n"
> 	"scd " "%0, %2						\n"
> 	"     beqz    %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 (1) {
>   unsigned long temp;
> 
>     __asm__ __volatile__(
> 	    "     .set    push                                    \n"
> 	    "     .set    ""arch=r4000""                  \n"
> 	    ".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set
> 	    mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ;
> 	    .endif" "                    \n"
> 	    "1:" "lld     " "%1, %2               # local_add_return
> 	    \n"
> 	    "     addu    %0, %1, %3                              \n"
> 	    "scd " "%0, %2
> 	    \n"
> 	    "     beqz    %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");
> 
> --------------------------------------------------------------------------------
> it is wrong here, "lld" + "addu"

it fixes something, but I didn't see the impact from the commit message.
Because there is no Fixes tag, I haven't applied it tomips-fixes.

And

+#elif MIPS_ISA_REV >= 6
+# define SC_BEQZ	beqzc

why are you doing this ?

> and PATCH 2, any comment?

parameter page already contains the virtual address of the page to
flush, so there is nothing to fix. I'm not 100% about the cache
nature of all TX39 core, so leaving the check for cpu_has_dc_aliases
in place is the safer bet. And this platform is nearly dead, so I'm
sure sure whether this sort of cosemtics is needed.

Thomas.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] MIPS: loongson64: fix FTLB configuration
  2021-11-30 10:34       ` Thomas Bogendoerfer
@ 2021-11-30 11:42         ` Huang Pei
  0 siblings, 0 replies; 10+ messages in thread
From: Huang Pei @ 2021-11-30 11:42 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: ambrosehua, Bibo Mao, linux-mips, Jiaxun Yang, Paul Burton,
	Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen

On Tue, Nov 30, 2021 at 11:34:48AM +0100, Thomas Bogendoerfer wrote:
> On Fri, Nov 26, 2021 at 11:13:19AM +0800, Huang Pei wrote:
> > On Thu, Nov 25, 2021 at 04:55:28PM +0100, Thomas Bogendoerfer wrote:
> > > On Thu, Nov 25, 2021 at 06:59:49PM +0800, Huang Pei wrote:
> > > > It turns out that 'decode_configs' -> 'set_ftlb_enable' is called under
> > > > c->cputype unset, which leaves FTLB disabled on BOTH 3A2000 and 3A3000
> > > > 
> > > > Fix it by calling "decode_configs" after c->cputype is initialized
> > > > 
> > > > Fixes: da1bd29742b1 ("MIPS: Loongson64: Probe CPU features via CPUCFG")
> > > > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > > > ---
> > > >  arch/mips/kernel/cpu-probe.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> > > > index ac0e2cfc6d57..24a529c6c4be 100644
> > > > --- a/arch/mips/kernel/cpu-probe.c
> > > > +++ b/arch/mips/kernel/cpu-probe.c
> > > > @@ -1734,8 +1734,6 @@ static inline void decode_cpucfg(struct cpuinfo_mips *c)
> > > >  
> > > >  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
> > > >  {
> > > > -	decode_configs(c);
> > > > -
> > > >  	/* All Loongson processors covered here define ExcCode 16 as GSExc. */
> > > >  	c->options |= MIPS_CPU_GSEXCEX;
> > > >  
> > > > @@ -1796,6 +1794,8 @@ static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu)
> > > >  		panic("Unknown Loongson Processor ID!");
> > > >  		break;
> > > >  	}
> > > > +
> > > > +	decode_configs(c);
> > > >  }
> > > >  #else
> > > >  static inline void cpu_probe_loongson(struct cpuinfo_mips *c, unsigned int cpu) { }
> > > > -- 
> > > > 2.20.1
> > > 
> > > applied to mips-fixes.
> > > 
> > > Thomas.
> > > 
> > > -- 
> > > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> > > good idea.                                                [ RFC1925, 2.3 ]
> > Hi, Thomas,
> > 
> > What about PATCH 1/4, without it, kernel/trace/ring_buffer.i using
> > local_add_return, like this 
> > --------------------------------------------------------------------------------
> >     __asm__ __volatile__(
> > 	"     .set    push                                    \n"
> > 	"     .set    ""arch=r4000""                  \n"
> > 	".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set mips64r2;.rept 1; sync 
> > 	0x00; .endr; .set pop; .else; ; .endif" "\n"
> > 	"1:" "lld     " "%1, %2               # local_add_return\n"
> > 	"     addu    %0, %1, %3				\n"
> > 	"scd " "%0, %2						\n"
> > 	"     beqz    %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 (1) {
> >   unsigned long temp;
> > 
> >     __asm__ __volatile__(
> > 	    "     .set    push                                    \n"
> > 	    "     .set    ""arch=r4000""                  \n"
> > 	    ".if (( 0x00 ) != -1) && ( (1 << 31) ); .set push; .set
> > 	    mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ;
> > 	    .endif" "                    \n"
> > 	    "1:" "lld     " "%1, %2               # local_add_return
> > 	    \n"
> > 	    "     addu    %0, %1, %3                              \n"
> > 	    "scd " "%0, %2
> > 	    \n"
> > 	    "     beqz    %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");
> > 
> > --------------------------------------------------------------------------------
> > it is wrong here, "lld" + "addu"
> 
> it fixes something, but I didn't see the impact from the commit message.
> Because there is no Fixes tag, I haven't applied it tomips-fixes.
> 
Sorry, this bug is introduced by 7232311ef14c274d88871212a07557f18f4140d1
> And
> 
> +#elif MIPS_ISA_REV >= 6
> +# define SC_BEQZ	beqzc
> 
> why are you doing this ?
This copies from arch/mips/include/asm/llsc.h, I had another patch which
remove llsc.h. I do not want add that patch into bugfix;

> > and PATCH 2, any comment?
> 
> parameter page already contains the virtual address of the page to
> flush, so there is nothing to fix. I'm not 100% about the cache
> nature of all TX39 core, so leaving the check for cpu_has_dc_aliases
> in place is the safer bet. And this platform is nearly dead, so I'm
> sure sure whether this sort of cosemtics is needed.
> 
Based on the comment, I can see TX39 want to bypass TLB by Indexed Cache
Operation,  because with KSEG0 address, Indexed Cache Operation does not 
pass through TLB; But there are two limitation:

+. KSEG0 only cover first 512MB physical address, so do not support 
High Memory, if not using KSEG0 address ,then it passes through TLB, which
is not intended;

+. Indexed Cache Operation can overkill cache line with same cache index
from different way, but it can not kill Cache Alias, since Cache Alias
actually has different cache index;

From datasheet of TX39xx, I know the R3900 core of TX39 series has only
4KB direct-mapped Icache and 2KB two-way Dcache, so it has no Cache 
Alias under 4KB page size, and I can assume it does not support memory
beyond 512MB based on Kconfig of TX39, AKA no High Memory;

Above all, I think it is OK to both use KSEG0 address and remove cpu_has_dc_alias 

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-11-30 11:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 10:59 [PATCH V4]: bugfix Huang Pei
2021-11-25 10:59 ` [PATCH 1/4] MIPS: rework local_t operation on MIPS64 Huang Pei
2021-11-25 10:59 ` [PATCH 2/4] MIPS: tx39: fix tx39_flush_cache_page Huang Pei
2021-11-25 10:59 ` [PATCH 3/4] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48 Huang Pei
2021-11-25 15:55   ` Thomas Bogendoerfer
2021-11-25 10:59 ` [PATCH 4/4] MIPS: loongson64: fix FTLB configuration Huang Pei
2021-11-25 15:55   ` Thomas Bogendoerfer
2021-11-26  3:13     ` Huang Pei
2021-11-30 10:34       ` Thomas Bogendoerfer
2021-11-30 11:42         ` Huang Pei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.