[PATCH] SMP/TLB problems with 64-bit kernels

[Richard Sandiford <rsandifo@redhat.com>]

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
--- arch/mips/mm/tlbex.c	12 Jan 2005 16:38:50 -0000	1.16
+++ arch/mips/mm/tlbex.c	20 Feb 2005 18:58:35 -0000
@@ -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
--- include/asm-mips/mmu_context.h	8 Jan 2005 15:03:53 -0000	1.48
+++ include/asm-mips/mmu_context.h	20 Feb 2005 18:59:53 -0000
@@ -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
--- include/asm-mips/stackframe.h	13 Feb 2005 00:32:43 -0000	1.36
+++ include/asm-mips/stackframe.h	20 Feb 2005 18:59:54 -0000
@@ -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