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@xxxxxxxxxxx>
> > > > ---
> > > > 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 ]
