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:

quoted

On Thu, Nov 25, 2021 at 04:55:28PM +0100, Thomas Bogendoerfer wrote:

quoted

On Thu, Nov 25, 2021 at 06:59:49PM +0800, Huang Pei wrote:

quoted

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 <redacted>
---
 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;

quoted

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 ]