qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 05/13] target-openrisc: Remove TLB flush on exce


From: Sebastian Macke
Subject: Re: [Qemu-devel] [PATCH 05/13] target-openrisc: Remove TLB flush on exception
Date: Tue, 29 Oct 2013 15:41:30 -0700
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.0.1

On 29/10/2013 12:47 PM, Peter Maydell wrote:
On 29 October 2013 19:04, Sebastian Macke <address@hidden> 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 <address@hidden>
---
  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?









reply via email to

[Prev in Thread] Current Thread [Next in Thread]