From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IfPCp-0005PL-CK for qemu-devel@nongnu.org; Tue, 09 Oct 2007 20:13:43 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IfPCm-0005P9-NU for qemu-devel@nongnu.org; Tue, 09 Oct 2007 20:13:42 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IfPCm-0005P6-If for qemu-devel@nongnu.org; Tue, 09 Oct 2007 20:13:40 -0400 Received: from relay01.mx.bawue.net ([193.7.176.67]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1IfPCl-0004kh-PZ for qemu-devel@nongnu.org; Tue, 09 Oct 2007 20:13:40 -0400 Date: Wed, 10 Oct 2007 01:12:38 +0100 From: Thiemo Seufer Subject: Re: [Qemu-devel] RFC: cleanups: CPU_MEM_INDEX Message-ID: <20071010001238.GA3379@networkno.de> References: <1191958401.9976.87.camel@rapid> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1191958401.9976.87.camel@rapid> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "J. Mayer" Cc: qemu-devel@nongnu.org J. Mayer wrote: > Here's a proposal to add a int cpu_mem_index (CPUState *env) function in > targets cpu.h header. > The idea of this patch is: > - avoid many #ifdef TARGET_xxx in exec-all.h and softmmu_header.h then > make the code more readable > - avoid multiple implementation of the same code (3, in that particular > case) this to avoid potential conflicts if the definition has to be > updated for any reason (ie support for new memory access modes, > emulation optimisation...) > > Please comment. > > -- > J. Mayer > Never organized > Index: cpu-exec.c > =================================================================== > RCS file: /sources/qemu/qemu/cpu-exec.c,v > retrieving revision 1.119 > diff -u -d -d -p -r1.119 cpu-exec.c > --- cpu-exec.c 8 Oct 2007 13:16:13 -0000 1.119 > +++ cpu-exec.c 9 Oct 2007 10:36:07 -0000 > @@ -885,7 +885,7 @@ static inline int handle_cpu_signal(unsi > > /* see if it is an MMU fault */ > ret = cpu_x86_handle_mmu_fault(env, address, is_write, > - ((env->hflags & HF_CPL_MASK) == 3), 0); > + cpu_mem_index(env), 0); > if (ret < 0) > return 0; /* not an MMU fault */ > if (ret == 0) > @@ -1007,7 +1009,8 @@ static inline int handle_cpu_signal(unsi > } > > /* see if it is an MMU fault */ > - ret = cpu_ppc_handle_mmu_fault(env, address, is_write, msr_pr, 0); > + ret = cpu_ppc_handle_mmu_fault(env, address, is_write, > + cpu_mem_index(env), 0); > if (ret < 0) > return 0; /* not an MMU fault */ > if (ret == 0) > @@ -1191,7 +1197,8 @@ static inline int handle_cpu_signal(unsi > } > > /* see if it is an MMU fault */ > - ret = cpu_alpha_handle_mmu_fault(env, address, is_write, 1, 0); > + ret = cpu_alpha_handle_mmu_fault(env, address, is_write, > + cpu_mem_index(env), 0); I have the feeling some architectures are missing here. :-) > if (ret < 0) > return 0; /* not an MMU fault */ > if (ret == 0) > Index: exec-all.h > =================================================================== > RCS file: /sources/qemu/qemu/exec-all.h,v > retrieving revision 1.67 > diff -u -d -d -p -r1.67 exec-all.h > --- exec-all.h 8 Oct 2007 13:16:14 -0000 1.67 > +++ exec-all.h 9 Oct 2007 10:36:07 -0000 > @@ -601,27 +606,7 @@ static inline target_ulong get_phys_addr > int is_user, index, pd; > > index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > -#if defined(TARGET_I386) > - is_user = ((env->hflags & HF_CPL_MASK) == 3); > -#elif defined (TARGET_PPC) > - is_user = msr_pr; > -#elif defined (TARGET_MIPS) > - is_user = ((env->hflags & MIPS_HFLAG_MODE) == MIPS_HFLAG_UM); > -#elif defined (TARGET_SPARC) > - is_user = (env->psrs == 0); > -#elif defined (TARGET_ARM) > - is_user = ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_USR); > -#elif defined (TARGET_SH4) > - is_user = ((env->sr & SR_MD) == 0); > -#elif defined (TARGET_ALPHA) > - is_user = ((env->ps >> 3) & 3); > -#elif defined (TARGET_M68K) > - is_user = ((env->sr & SR_S) == 0); > -#elif defined (TARGET_CRIS) > - is_user = (0); > -#else > -#error unimplemented CPU > -#endif > + is_user = cpu_mem_index(env); > if (__builtin_expect(env->tlb_table[is_user][index].addr_code != > (addr & TARGET_PAGE_MASK), 0)) { I presume cpu_mem_index is supposed to do more than checking for usermode. In that case, is_user should get renamed, and the cpu_mem_index implementation of some (most?) CPUs should have a FIXME comment as reminder to implement the missing MMU modes. Other than that it looks good to me (and reminds me to check what the supervisor mode on MIPS actually does now :-). Thiemo