SMP/TLB problems with 64-bit kernels

A 64-bit SB1 SMP kernel built from current 2.6 CVS sources will hang when trying to run init. This is probably the same as the bug reported here:

http://www.linux-mips.org/archives/linux-mips/2005-01/msg00086.html

The patch below fixes things for me. There were three separate problems:

- The CONFIG_BUILD_ELF64 version of get_saved_sp miscalculated
kernel_sp: it failed to shift the address left 16 bits after adding in %hi(kernel_sp).

- build_get_pmde64 says:

        /*
         * 64 bit SMP has the lower part of &pgd_current[smp_processor_id()]
         * stored in CONTEXT.
         */
        if (in_compat_space_p(pgdc)) {
                i_dmfc0(p, ptr, C0_CONTEXT);
                i_dsra(p, ptr, ptr, 23);
                i_ld(p, ptr, 0, ptr);
        } else {
#ifdef CONFIG_BUILD_ELF64
                ...
#else
                ...

#endif
}

but for CONFIG_BUILD_ELF64, the context register contains smp_processor_id(), not &pgd_current[smp_processor_id()]. The patch rearranges the code as follows:

#ifdef CONFIG_BUILD_ELF64
        ...
#else
        /*
         * 64 bit SMP has the lower part of &pgd_current[smp_processor_id()]
         * stored in CONTEXT.
         */
        if (in_compat_space_p(pgdc)) {
                i_dmfc0(p, ptr, C0_CONTEXT);
                i_dsra(p, ptr, ptr, 23);
                i_ld(p, ptr, 0, ptr);
        } else {
                ...
        }

#endif

- The final "..." above is:

i_dmfc0(p, ptr, C0_CONTEXT); i_lui(p, tmp, rel_highest(pgdc)); i_dsll(p, ptr, ptr, 9); i_daddiu(p, tmp, tmp, rel_higher(pgdc));
---> i_dsrl32(p, ptr, ptr, 0);
---> i_and(p, ptr, ptr, tmp);
i_dmfc0(p, tmp, C0_BADVADDR); i_ld(p, ptr, 0, ptr);

There are three problems here:

* the high 32 bits of the address (tmp) aren't shifted left 32 places.

* If you calculate foo as:

(%highest(foo) << 48) + (%higher(foo) << 32) + x

then "x" is a signed number, so the shift should be dsra32, not dsrl32. This is the only use of dsrl32 in the file, and nothing uses dsra32 at the moment, so the patch replaces the dsrl32 definitions with dsra32 definitions.

* The "and" should be a "daddu".

(I'm not sure how this case triggers. Won't pgd_current always be in the compat region if !CONFIG_BUILD_ELF64?)

Also, whenever the CONFIG_BUILD_ELF64 code reads smp_processor_id() from the context register, it uses the result to index arrays of 8-byte values. We can avoid a shift in both the TLB code and in get_saved_sp if we store smp_processor_id() * 8 in the context register (i.e. if TLBMISS_HANDLER_SETUP shifts the value left by 26 rather than 23).

I've tested the patch with both CONFIG_BUILD_ELF64 and !CONFIG_BUILD_ELF64, and in both cases it fixes the hang and seems to give a stable kernel. I also tried a !CONFIG_BUILD_ELF64 with the in_compat_space_p() check above disabled, forcing the longer version to be used. Please apply if OK.

Richard

Index: arch/mips/mm/tlbex.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/mm/tlbex.c,v
retrieving revision 1.16

diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.16 tlbex.c
@@ -91,7 +91,7 @@ enum opcode {
 	insn_addu, insn_addiu, insn_and, insn_andi, insn_beq,
 	insn_beql, insn_bgez, insn_bgezl, insn_bltz, insn_bltzl,
 	insn_bne, insn_daddu, insn_daddiu, insn_dmfc0, insn_dmtc0,
-	insn_dsll, insn_dsll32, insn_dsra, insn_dsrl, insn_dsrl32,
+	insn_dsll, insn_dsll32, insn_dsra, insn_dsrl, insn_dsra32,
 	insn_dsubu, insn_eret, insn_j, insn_jal, insn_jr, insn_ld,
 	insn_ll, insn_lld, insn_lui, insn_lw, insn_mfc0, insn_mtc0,
 	insn_ori, insn_rfe, insn_sc, insn_scd, insn_sd, insn_sll,

@@ -134,7 +134,7 @@ static __initdata struct insn insn_table
 	{ insn_dsll32, M(spec_op,0,0,0,0,dsll32_op), RT | RD | RE },
 	{ insn_dsra, M(spec_op,0,0,0,0,dsra_op), RT | RD | RE },
 	{ insn_dsrl, M(spec_op,0,0,0,0,dsrl_op), RT | RD | RE },
-	{ insn_dsrl32, M(spec_op,0,0,0,0,dsrl32_op), RT | RD | RE },
+	{ insn_dsra32, M(spec_op,0,0,0,0,dsra32_op), RT | RD | RE },
 	{ insn_dsubu, M(spec_op,0,0,0,0,dsubu_op), RS | RT | RD },
 	{ insn_eret, M(cop0_op,cop_op,0,0,0,eret_op), 0 },
 	{ insn_j, M(j_op,0,0,0,0,0), JIMM },

@@ -366,7 +366,7 @@ I_u2u1u3(_dsll);
 I_u2u1u3(_dsll32);
 I_u2u1u3(_dsra);
 I_u2u1u3(_dsrl);
-I_u2u1u3(_dsrl32);
+I_u2u1u3(_dsra32);
 I_u3u1u2(_dsubu);
 I_0(_eret);
 I_u1(_j);

@@ -942,6 +942,14 @@ build_get_pmde64(u32 **p, struct label *
 	/* No i_nop needed here, since the next insn doesn't touch TMP. */
 
 #ifdef CONFIG_SMP
+#ifdef CONFIG_BUILD_ELF64
+	i_dmfc0(p, ptr, C0_CONTEXT);
+	i_dsrl(p, ptr, ptr, 23);
+	i_LA_mostly(p, tmp, pgdc);
+	i_daddu(p, ptr, ptr, tmp);
+	i_dmfc0(p, tmp, C0_BADVADDR);
+	i_ld(p, ptr, rel_lo(pgdc), ptr);
+#else
 	/*
 	 * 64 bit SMP has the lower part of &pgd_current[smp_processor_id()]
 	 * stored in CONTEXT.

@@ -951,25 +959,17 @@ build_get_pmde64(u32 **p, struct label *
 		i_dsra(p, ptr, ptr, 23);
 		i_ld(p, ptr, 0, ptr);
 	} else {
-#ifdef CONFIG_BUILD_ELF64
-		i_dmfc0(p, ptr, C0_CONTEXT);
-		i_dsrl(p, ptr, ptr, 23);
-		i_dsll(p, ptr, ptr, 3);
-		i_LA_mostly(p, tmp, pgdc);
-		i_daddu(p, ptr, ptr, tmp);
-		i_dmfc0(p, tmp, C0_BADVADDR);
-		i_ld(p, ptr, rel_lo(pgdc), ptr);
-#else
 		i_dmfc0(p, ptr, C0_CONTEXT);
 		i_lui(p, tmp, rel_highest(pgdc));
 		i_dsll(p, ptr, ptr, 9);
 		i_daddiu(p, tmp, tmp, rel_higher(pgdc));
-		i_dsrl32(p, ptr, ptr, 0);
-		i_and(p, ptr, ptr, tmp);
+		i_dsra32(p, ptr, ptr, 0);
+		i_dsll32(p, tmp, tmp, 0);
+		i_daddu(p, ptr, ptr, tmp);
 		i_dmfc0(p, tmp, C0_BADVADDR);
 		i_ld(p, ptr, 0, ptr);
-#endif
 	}
+#endif
 #else
 	i_LA_mostly(p, ptr, pgdc);
 	i_ld(p, ptr, rel_lo(pgdc), ptr);

Index: include/asm-mips/mmu_context.h
===================================================================
RCS file: /home/cvs/linux/include/asm-mips/mmu_context.h,v retrieving revision 1.48

diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.48 mmu_context.h
@@ -40,7 +40,7 @@ #define TLBMISS_HANDLER_SETUP()						\
 #endif
 #if defined(CONFIG_MIPS64) && defined(CONFIG_BUILD_ELF64)
 #define TLBMISS_HANDLER_SETUP()						\
-	write_c0_context((unsigned long) smp_processor_id() << 23);	\
+	write_c0_context((unsigned long) smp_processor_id() << 26);	\
 	TLBMISS_HANDLER_SETUP_PGD(swapper_pg_dir)
 #endif
 

Index: include/asm-mips/stackframe.h
===================================================================
RCS file: /home/cvs/linux/include/asm-mips/stackframe.h,v retrieving revision 1.36

diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.36 stackframe.h
@@ -76,12 +76,12 @@ #define _ASM_STACKFRAME_H
 #endif
 #if defined(CONFIG_MIPS64) && defined(CONFIG_BUILD_ELF64)
 		MFC0	k1, CP0_CONTEXT
-		dsrl	k1, 23
-		dsll	k1, k1, 3
 		lui	k0, %highest(kernelsp)
 		daddiu	k0, %higher(kernelsp)
 		dsll	k0, k0, 16
 		daddiu	k0, %hi(kernelsp)
+		dsrl	k1, 23
+		dsll	k0, k0, 16
 		daddu	k1, k1, k0
 		LONG_L	k1, %lo(kernelsp)(k1)
 #endif

It is indeed.

I second that. I have tested the patch on my Yosemite board and it makes a 64-bit SMP kernel work again (as long as I do not have more than 512 MB of memory).

Thanks
Rojhalat Ibrahim