From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NVs7a-0007qZ-JZ for qemu-devel@nongnu.org; Fri, 15 Jan 2010 14:46:14 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NVs7W-0007pc-0r for qemu-devel@nongnu.org; Fri, 15 Jan 2010 14:46:14 -0500 Received: from [199.232.76.173] (port=60434 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NVs7V-0007pZ-Sf for qemu-devel@nongnu.org; Fri, 15 Jan 2010 14:46:09 -0500 Received: from mail-pw0-f43.google.com ([209.85.160.43]:37978) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NVs7U-0004x9-Ij for qemu-devel@nongnu.org; Fri, 15 Jan 2010 14:46:09 -0500 Received: by pwj11 with SMTP id 11so540115pwj.2 for ; Fri, 15 Jan 2010 11:46:07 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1263581172-16129-1-git-send-email-atar4qemu@google.com> References: <1263581172-16129-1-git-send-email-atar4qemu@google.com> From: Blue Swirl Date: Fri, 15 Jan 2010 19:45:47 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: sparc32 do_unassigned_access overhaul List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Artyom Tarasenko Cc: qemu-devel@nongnu.org, Artyom Tarasenko On Fri, Jan 15, 2010 at 6:46 PM, Artyom Tarasenko wrote: > According to pages 9-31 - 9-34 of "SuperSPARC & MultiCache Controller > User's Manual": > > 1. "A lower priority fault may not overwrite the > =C2=A0 =C2=A0MFSR status of a higher priority fault." > 2. The MFAR is overwritten according to the policy defined for the MFSR > 3. The overwrite bit is asserted if the fault status register (MFSR) > =C2=A0 has been written more than once by faults of the same class > 4. SuperSPARC will never place instruction fault addresses in the MFAR. > > Implementation of points 1-3 allows booting Solaris 2.6 and 2.5.1. Nice work! This also passes my tests. However, there are some CODING_STYLE issues. > > Signed-off-by: Artyom Tarasenko > --- > diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c > index 381e6c4..3a56ce9 100644 > --- a/target-sparc/op_helper.c > +++ b/target-sparc/op_helper.c > @@ -3714,6 +3714,7 @@ void do_unassigned_access(target_phys_addr_t addr, = int is_write, int is_exec, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 int is_asi, int size) > =C2=A0{ > =C2=A0 =C2=A0 CPUState *saved_env; > + =C2=A0 =C2=A0int fault_type; > > =C2=A0 =C2=A0 /* XXX: hack to restore env in all cases, even if not calle= d from > =C2=A0 =C2=A0 =C2=A0 =C2=A0generated code */ > @@ -3731,18 +3732,27 @@ void do_unassigned_access(target_phys_addr_t addr= , int is_write, int is_exec, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0is_exec ? "exec" := is_write ? "write" : "read", size, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size =3D=3D 1 ? ""= : "s", addr, env->pc); > =C2=A0#endif > - =C2=A0 =C2=A0if (env->mmuregs[3]) /* Fault status register */ > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] =3D 1; /* overflow (not read= before another fault) */ > - =C2=A0 =C2=A0if (is_asi) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 16; > - =C2=A0 =C2=A0if (env->psrs) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 5; > - =C2=A0 =C2=A0if (is_exec) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 6; > - =C2=A0 =C2=A0if (is_write) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 7; > - =C2=A0 =C2=A0env->mmuregs[3] |=3D (5 << 2) | 2; > - =C2=A0 =C2=A0env->mmuregs[4] =3D addr; /* Fault address register */ > + =C2=A0 =C2=A0/* Don't overwrite translation and access faults */ > + =C2=A0 =C2=A0fault_type=3D(env->mmuregs[3]&0x1c)>>2; Must have spaces around '=3D', '&' and '>>'. > + =C2=A0 =C2=A0if ((fault_type > 4) || (fault_type=3D=3D0)) { Must have spaces around '=3D=3D'. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3]=3D0; /* Fault status registe= r */ and '=3D' > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (is_asi) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 16; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (env->psrs) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 5; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (is_exec) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 6; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (is_write) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1 << 7; Here you could add the {} which the original lacked, but as this is only code movement it's not needed. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D (5 << 2) | 2; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0/* SuperSPARC will never place instruction f= ault addresses in the FAR */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!is_exec) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[4] =3D addr; /* F= ault address register */ But this is new code so {} must be added. > + =C2=A0 =C2=A0} > + =C2=A0 =C2=A0/* overflow (same type fault was not read before another f= ault) */ > + =C2=A0 =C2=A0if (fault_type=3D=3D((env->mmuregs[3]&0x1c))>>2) Must have spaces around '=3D', '&' and '>>'. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mmuregs[3] |=3D 1; > + > =C2=A0 =C2=A0 if ((env->mmuregs[0] & MMU_E) && !(env->mmuregs[0] & MMU_NF= )) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (is_exec) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise_exception(TT_CODE_ACCESS)= ; > @@ -3750,6 +3760,10 @@ void do_unassigned_access(target_phys_addr_t addr,= int is_write, int is_exec, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise_exception(TT_DATA_ACCESS)= ; > =C2=A0 =C2=A0 } > =C2=A0 =C2=A0 env =3D saved_env; > + =C2=A0 =C2=A0/* flush neverland mappings created during no-fault mode, > + =C2=A0 =C2=A0 =C2=A0 so the sequential MMU faults report proper fault t= ypes */ > + =C2=A0 =C2=A0if (env->mmuregs[0] & MMU_NF) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0tlb_flush(env, 1); New code, {}. > =C2=A0} > =C2=A0#else > =C2=A0void do_unassigned_access(target_phys_addr_t addr, int is_write, in= t is_exec, >