From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50337) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VbHyo-0008SU-3H for qemu-devel@nongnu.org; Tue, 29 Oct 2013 18:41:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VbHyh-0006sG-O2 for qemu-devel@nongnu.org; Tue, 29 Oct 2013 18:41:42 -0400 Received: from www11.your-server.de ([213.133.104.11]:49662) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VbHyh-0006sC-F2 for qemu-devel@nongnu.org; Tue, 29 Oct 2013 18:41:35 -0400 Message-ID: <5270399A.1080907@macke.de> Date: Tue, 29 Oct 2013 15:41:30 -0700 From: Sebastian Macke MIME-Version: 1.0 References: <1383073495-5332-1-git-send-email-sebastian@macke.de> <1383073495-5332-6-git-send-email-sebastian@macke.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 05/13] target-openrisc: Remove TLB flush on exception List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: openrisc@lists.openrisc.net, openrisc@lists.opencores.org, QEMU Developers , Ethan Hunt On 29/10/2013 12:47 PM, Peter Maydell wrote: > On 29 October 2013 19:04, Sebastian Macke wrote: >> The TLB flush is not necessary as the mmu_index field >> already takes care of correct memory locations. >> Instead the tb flag field must be expanded that >> the exception takes the correct translation block. >> >> Signed-off-by: Sebastian Macke >> --- >> target-openrisc/cpu.h | 4 ++-- >> target-openrisc/interrupt.c | 4 ---- >> 2 files changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h >> index 24afe6f..057821d 100644 >> --- a/target-openrisc/cpu.h >> +++ b/target-openrisc/cpu.h >> @@ -85,7 +85,7 @@ enum { >> #define SPR_VR 0xFFFF003F >> >> /* Internal flags, delay slot flag */ >> -#define D_FLAG 1 >> +#define D_FLAG 2 > Since this set of #defines effectively is the documentation for > what the tb_flags usage is, can you update it to include the > new flag you've added, please? I will. I think I have done it in one of the later patches. But the D_FLAG was there before. What I did was just changing it to 2 because 1 is used by the new SR_SM (supervisor mode) Flag. >> /* Interrupt */ >> #define NR_IRQS 32 >> @@ -412,7 +412,7 @@ static inline void cpu_get_tb_cpu_state(CPUOpenRISCState *env, >> *pc = env->pc; >> *cs_base = 0; >> /* D_FLAG -- branch instruction exception */ >> - *flags = (env->flags & D_FLAG); >> + *flags = (env->flags & D_FLAG) | (env->sr & SR_SM); >> } >> >> static inline int cpu_mmu_index(CPUOpenRISCState *env) >> diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c >> index d1d6ae2..ee98ed3 100644 >> --- a/target-openrisc/interrupt.c >> +++ b/target-openrisc/interrupt.c >> @@ -41,10 +41,6 @@ void openrisc_cpu_do_interrupt(CPUState *cs) >> env->epcr += 4; >> } >> >> - /* For machine-state changed between user-mode and supervisor mode, >> - we need flush TLB when we enter&exit EXCP. */ >> - tlb_flush(env, 1); >> - >> env->esr = ENV_GET_SR(env); >> env->sr &= ~SR_DME; >> env->sr &= ~SR_IME; > It looks suspicious that this patch doesn't include any change to > translate.c which reads the tb flag you've just added. Either: > (a) the translated code doesn't actually build in any dependencies > on the SR_SM flag, in which case it doesn't need to be a tb_flag at all > (b) the translated code is still referring directly to env->sr somewhere, > in which case it needs to be changed to use the tb_flags version instead > > Also, are you sure that tlb_flush() is needed purely for the change to the > SR_SM flags and not for any of the other CPU state changes that > openrisc_cpu_do_interrupt() is making when it does the user->supervisor > state change? > > thanks > -- PMM The exception is going into supervisor mode and disables the mmu. The mmu_index is changed and it should work. But then the emulated Linux crashes. This does not happen when I add the supervisor mode flag to the tb_flags. I was also a little bit confused when I implemented it. But I don't know the internals of QEMU as good as you. And some other targets doing it the same way I think. What is included in the tb hash? The virtual pc + physical page + the tb_flags? Not the mmu_index?