From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48231) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqM4v-0001eW-Sa for qemu-devel@nongnu.org; Mon, 17 Nov 2014 08:10:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XqM4r-00057b-6x for qemu-devel@nongnu.org; Mon, 17 Nov 2014 08:10:49 -0500 Received: from mailapp01.imgtec.com ([195.59.15.196]:43270) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqM4q-00057V-Uv for qemu-devel@nongnu.org; Mon, 17 Nov 2014 08:10:45 -0500 Message-ID: <5469F3C9.9090801@imgtec.com> Date: Mon, 17 Nov 2014 13:10:33 +0000 From: Leon Alrae MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] mips: Correct the handling of writes to CP0.Status for MIPSr6 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Maciej W. Rozycki" Cc: qemu-devel@nongnu.org, Aurelien Jarno On 10/11/2014 13:45, Maciej W. Rozycki wrote: > Correct these issues with the handling of CP0.Status for MIPSr6: > > * only ignore the bit pattern of 0b11 on writes to CP0.Status.KSU, that > is for processors that do implement Supervisor Mode, let the bit > pattern be written to CP0.Status.UM:R0 freely (of course the value > written to read-only CP0.Status.R0 will be discarded anyway); this is > in accordance to the relevant architecture specification[1], > > * check the newly written pattern rather than the current contents of > CP0.Status for the KSU bits being 0b11, > > * use meaningful macro names to refer to CP0.Status bits rather than > magic numbers. > > References: > > [1] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64 > Privileged Resource Architecture", MIPS Technologies, Inc., Document > Number: MD00091, Revision 6.00, March 31, 2014, Table 9.45 "Status > Register Field Descriptions", pp. 210-211. > > Signed-off-by: Maciej W. Rozycki > --- > Leon, > > Noticed in porting the next change I'm going to post. NB I have no > reasonable way to do run-time checks of an r6 configuration, so please > double check this works for you even though I believe it is obviously > correct; I did check CP0.Status.KSU rejects the pattern of 0b11 via GDB > on MIPS64R6-generic with the aid of the next change. > > I suggest making it a policy not to accept new code using magic > numbers. Then the existing places can be gradually identified and > corrected. > > Please apply. > > Thanks, > > Maciej > > qemu-mips-r6-status-r0.diff > Index: qemu-git-trunk/target-mips/op_helper.c > =================================================================== > --- qemu-git-trunk.orig/target-mips/op_helper.c 2014-11-09 23:44:45.467759913 +0000 > +++ qemu-git-trunk/target-mips/op_helper.c 2014-11-09 23:45:27.977531070 +0000 > @@ -1423,10 +1423,12 @@ void helper_mtc0_status(CPUMIPSState *en > uint32_t mask = env->CP0_Status_rw_bitmask; > > if (env->insn_flags & ISA_MIPS32R6) { > - if (extract32(env->CP0_Status, CP0St_KSU, 2) == 0x3) { > + bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3; > + > + if (has_supervisor && extract32(arg1, CP0St_KSU, 2) == 0x3) { > mask &= ~(3 << CP0St_KSU); > } > - mask &= ~(0x00180000 & arg1); > + mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & arg1); > } > > val = arg1 & mask; > Thanks for fixing and cleaning this up. Reviewed-by: Leon Alrae