From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6IMf-0000Jb-8A for qemu-devel@nongnu.org; Mon, 05 Aug 2013 06:50:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6IMZ-0006tE-Lw for qemu-devel@nongnu.org; Mon, 05 Aug 2013 06:50:13 -0400 Received: from hall.aurel32.net ([2001:470:1f0b:4a8::1]:34368) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6IMZ-0006sM-Fv for qemu-devel@nongnu.org; Mon, 05 Aug 2013 06:50:07 -0400 Date: Mon, 5 Aug 2013 12:50:00 +0200 From: Aurelien Jarno Message-ID: <20130805105000.GC4193@ohm.aurel32.net> References: <1375351347-16604-1-git-send-email-leon.alrae@imgtec.com> <20130803220132.GA20566@ohm.aurel32.net> <51FF5740.4090504@imgtec.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <51FF5740.4090504@imgtec.com> Subject: Re: [Qemu-devel] [PATCH] target-mips: fix decoding of microMIPS POOL32Axf instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leon Alrae Cc: yongbok.kim@imgtec.com, cristian.cuna@imgtec.com, qemu-devel@nongnu.org On Mon, Aug 05, 2013 at 08:41:52AM +0100, Leon Alrae wrote: > On 03/08/13 23:01, Aurelien Jarno wrote: > > On Thu, Aug 01, 2013 at 11:02:27AM +0100, Leon Alrae wrote: > >> These are not DSP instructions, thus there is no "ac" field. > >> > >> For more details please refer to instruction encoding of > >> MULT, MULTU, MADD, MADDU, MSUB, MSUBU, MFHI, MFLO, MTHI, MTLO in > >> MIPS Architecture for Programmers Volume II-B: The microMIPS32 Instruction Set > > > > These instructions have both a non DSP version without the "ac" field, > > and a DSP version with the "ac" field. The encoding of the "ac" field is > > done in bits 14 and 15, so the current QEMU code is correct. > > > > Please refer to the MIPS32® Architecture Manual Volume IV-e: The > > MIPS® DSP Application-Specific Extension to the microMIPS32® > > Architecture (document MD00764). > > > > Please note that the patch modifies non-DSP version of listed > instructions, where "ac" field doesn't exist. In this case reading bits This is correct, except for MFHI, MFLO, MTHI, MTLO which share the same opcode for the DSP and non-DSP version. Theses instructions have bits 14 and 15 set to 0 for the non-DSP version, so they are working as expected and thus your patch should not modify them. > 14 and 15 and passing it as acc argument to gen_muldiv() or gen_HILO() > is not correct. If those bits are not 0 (and for most of the listed > instructions this is true) then check_dsp() is called which will cause > an exception if DSP is not supported / disabled. > This is correct. The current code assumes that all instructions using gen_muldiv() have the same opcode for non-DSP and DSP version (actually it's weird that they have a different encoding, it's just wasting some opcode space). Therefore what should be done: - The part concerning MFHI, MFLO, MTHI, MTLO should be dropped from your patch as it already works correctly. - The part of your patch concerning instructions calling gen_muldiv() is correct and should be kept to handle the non-DSP version of these instructions. - An entry with for the DSP version of these instructions should be created, calling gen_muldiv() passing the ac field from bits 14 and 15. This is basically the same code than now, just with extension 0x32 instead of 0x2c. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net