Re: [PATCH 5/6] mips: use per-mm page to execute FP branch delay slots

[David Daney <ddaney.cavm@gmail.com>]

Nice work...

On 11/07/2013 04:48 AM, Paul Burton wrote:
[...]
> -	 * Algorithmics used a system call instruction, and
> -	 * borrowed that vector.  MIPS/Linux version is a bit
> -	 * more heavyweight in the interests of portability and
> -	 * multiprocessor support.  For Linux we generate a
> -	 * an unaligned access and force an address error exception.
> +	 *  1) Be a bug in the userland code, because it has a branch/jump in
> +	 *     a branch delay slot. So if we run out of emuframes and the
> +	 *     userland code hangs it's not exactly the kernels fault.

s/kernels/kernel's/


>   	 *
> -	 * For embedded systems (stand-alone) we prefer to use a
> -	 * non-existing CP1 instruction. This prevents us from emulating
> -	 * branches, but gives us a cleaner interface to the exception
> -	 * handler (single entry point).
> +	 *  2) Only affect that userland process, since emuframes are allocated
> +	 *     per-mm and kernel threads don't use them at all.
>   	 */
> +	if (!get_isa16_mode(regs->cp0_epc)) {
> +		if (!ir) {
> +			/* typical NOP encoding: sll r0, r0, r0 */
> +is_nop:
> +			regs->cp0_epc = cpc;
> +			regs->cp0_cause &= ~CAUSEF_BD;
> +			return 0;
> +		}
>
> -	/* Ensure that the two instructions are in the same cache line */
> -	fr = (struct emuframe __user *)
> -		((regs->regs[29] - sizeof(struct emuframe)) & ~0x7);
> +		switch (inst.j_format.opcode) {
> +		case bcond_op:
> +			switch (inst.i_format.rt) {
> +			case bltz_op:
> +			case bgez_op:
> +			case bltzl_op:
> +			case bgezl_op:
> +			case bltzal_op:
> +			case bgezal_op:
> +			case bltzall_op:
> +			case bgezall_op:
> +				goto is_branch;
> +			}
> +			break;

Is there any way to use the support in arch/mips/kernel/branch.c instead 
of duplicating the code here?

It may require some refactoring to make it work, but I think it would be 
worth it.

>
> -	/* Verify that the stack pointer is not competely insane */
> -	if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe))))
> +		case cop1_op:
> +			switch (inst.i_format.rs) {
> +			case bc_op:
> +				goto is_branch;
> +			}
> +			break;
> +
> +		case j_op:
> +		case jal_op:
> +		case beq_op:
> +		case bne_op:
> +		case blez_op:
> +		case bgtz_op:
> +		case beql_op:
> +		case bnel_op:
> +		case blezl_op:
> +		case bgtzl_op:
> +		case jalx_op:
> +is_branch:
> +			pr_warn("PID %d has a branch in an FP branch delay slot at 0x%08lx\n",
> +				current->pid, regs->cp0_epc);
> +			goto is_nop;
> +		}
> +	} else {
> +		if ((ir >> 16) == MM_NOP16)
> +			goto is_nop;
> +
> +		switch (inst.mm_i_format.opcode) {
> +		case mm_beqz16_op:
> +		case mm_beq32_op:
> +		case mm_bnez16_op:
> +		case mm_bne32_op:
> +		case mm_b16_op:
> +		case mm_j32_op:
> +		case mm_jalx32_op:
> +		case mm_jal32_op:
> +			goto is_branch;
> +
> +		case mm_pool32i_op:
> +			switch (inst.mm_i_format.rt) {
> +			case mm_bltz_op:
> +			case mm_bltzal_op:
> +			case mm_bgez_op:
> +			case mm_bgezal_op:
> +			case mm_blez_op:
> +			case mm_bnezc_op:
> +			case mm_bgtz_op:
> +			case mm_beqzc_op:
> +			case mm_bltzals_op:
> +			case mm_bgezals_op:
> +			case mm_bc2f_op:
> +			case mm_bc2t_op:
> +			case mm_bc1f_op:
> +			case mm_bc1t_op:
> +				goto is_branch;
> +			}
> +			break;
> +
> +		case mm_pool16c_op:
> +			switch (inst.mm16_r5_format.rt) {
> +			case mm_jr16_op:
> +			case mm_jrc_op:
> +			case mm_jalr16_op:
> +			case mm_jalrs16_op:
> +			case mm_jraddiusp_op:
> +				goto is_branch;
> +			}
> +			break;
> +		}
> +	}
> +
>