Re: [PATCH] MIPS: R6: emulation of PC-relative instructions

[Markos Chandras <markos.chandras@imgtec.com>]

Hi,

On Wed, Aug 05, 2015 at 04:53:43PM -0700, Leonid Yegoshin wrote:
> MIPS R6 has 6 new PC-relative instructions: LWUPC, LWPC, LDPC, ADDIUPC, ALUIPC
> and AUIPC. These instructions can be placed in BD-slot of BC1* branch
> instruction and FPU may be not available, which requires emulation of these
> instructions.
> 
> However, the traditional way to emulate that is via filling some emulation block
> in stack or special area and jump to it. This is not suitable for PC-relative
> instructions.
> 
> So, this patch introduces a universal emulation of that instructions directly by
> kernel emulator.
> 
> Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
> ---
>  arch/mips/include/uapi/asm/inst.h     |   42 ++++++++++++++-
>  arch/mips/kernel/mips-r2-to-r6-emul.c |    3 +
>  arch/mips/math-emu/dsemul.c           |   94 +++++++++++++++++++++++++++++++++
>  3 files changed, 138 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/include/uapi/asm/inst.h b/arch/mips/include/uapi/asm/inst.h
> index 3dce80e67948..6253197d4908 100644
> --- a/arch/mips/include/uapi/asm/inst.h
> +++ b/arch/mips/include/uapi/asm/inst.h
> @@ -33,7 +33,7 @@ enum major_op {
>  	sdl_op, sdr_op, swr_op, cache_op,
>  	ll_op, lwc1_op, lwc2_op, bc6_op = lwc2_op, pref_op,
>  	lld_op, ldc1_op, ldc2_op, beqzcjic_op = ldc2_op, ld_op,
> -	sc_op, swc1_op, swc2_op, balc6_op = swc2_op, major_3b_op,
> +	sc_op, swc1_op, swc2_op, balc6_op = swc2_op, pcrel_op,
>  	scd_op, sdc1_op, sdc2_op, bnezcjialc_op = sdc2_op, sd_op
>  };
>  
>  			if (nir) {
>  				err = mipsr6_emul(regs, nir);
>  				if (err > 0) {
> +					regs->cp0_epc = nepc;

Does this change belog to this patch? If so why? Maybe a comment would help?
It does feel like it fixes a different problem but I haven't read your patch in depth.

>  					err = mips_dsemul(regs, nir, cpc, epc, r31);
>  					if (err == SIGILL)
>  						err = SIGEMT;
> @@ -1082,6 +1083,7 @@ repeat:
>  			if (nir) {
>  				err = mipsr6_emul(regs, nir);
>  				if (err > 0) {
> +					regs->cp0_epc = nepc;
likewise

>  					err = mips_dsemul(regs, nir, cpc, epc, r31);
>  					if (err == SIGILL)
>  						err = SIGEMT;
> @@ -1149,6 +1151,7 @@ repeat:
>  		if (nir) {
>  			err = mipsr6_emul(regs, nir);
>  			if (err > 0) {
> +				regs->cp0_epc = nepc;
likewise

>  				err = mips_dsemul(regs, nir, cpc, epc, r31);
>  				if (err == SIGILL)
>  					err = SIGEMT;
> diff --git a/arch/mips/math-emu/dsemul.c b/arch/mips/math-emu/dsemul.c
> index eac76a09d822..9b388aaf594f 100644
> --- a/arch/mips/math-emu/dsemul.c
> +++ b/arch/mips/math-emu/dsemul.c
> @@ -8,6 +8,95 @@
>  
>  #include "ieee754.h"
>  
> +#ifdef CONFIG_CPU_MIPSR6

Can we simply avoid the if/def for R6 please? Just leave this function as is and
use if(cpu_has_mips_r6) when calling it. If you can't do that, please explain
why.

> +
> +static int mipsr6_pc(struct pt_regs *regs, mips_instruction inst, unsigned long cpc,
> +		    unsigned long bpc, unsigned long r31)
> +{
> +	union mips_instruction ir = (union mips_instruction)inst;
> +	register unsigned long vaddr;
> +	unsigned int val;
> +	int err = SIGILL;
> +
> +	if (ir.rel_format.opcode != pcrel_op)
> +		return SIGILL;
> +
> +	switch (ir.rel_format.op) {
> +	case addiupc_op:
> +		vaddr = regs->cp0_epc + (ir.rel_format.simmediate << 2);
> +		if (config_enabled(CONFIG_64BIT) && !(regs->cp0_status & ST0_UX))
> +			__asm__ __volatile__("sll %0, %0, 0":"+&r"(vaddr)::);
> +		regs->regs[ir.rel_format.rs] = vaddr;
> +		return 0;
> +#ifdef CONFIG_CPU_MIPS64

Could you use cpu_has_mips64 and avoid the if/def and return SIGILL if it is not
true?

Same thing for the rest of this patch.

-- 
markos