Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965292AbbHKOlH (ORCPT ); Tue, 11 Aug 2015 10:41:07 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:41954 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964946AbbHKOlF (ORCPT ); Tue, 11 Aug 2015 10:41:05 -0400 Date: Tue, 11 Aug 2015 15:41:01 +0100 From: Markos Chandras To: Leonid Yegoshin CC: , , , , , , Subject: Re: [PATCH] MIPS: R6: emulation of PC-relative instructions Message-ID: <20150811144101.GA25145@mchandras-linux.le.imgtec.org> References: <20150805235343.21126.29589.stgit@ubuntu-yegoshin> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline In-Reply-To: <20150805235343.21126.29589.stgit@ubuntu-yegoshin> User-Agent: Mutt/1.5.23 (2014-03-12) X-Originating-IP: [192.168.154.168] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3893 Lines: 113 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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/