diff mbox

[4/5] sparc64: fix mmu context at trap levels above zero

Message ID 20100522105234.18257.53650.stgit@skyserv
State New
Headers show

Commit Message

Igor V. Kovalenko May 22, 2010, 10:52 a.m. UTC 
From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

- cpu_mmu_index return MMU_NUCLEUS_IDX if trap level is not zero
- cpu_get_tb_cpu_state: store trap level and primary context in flags
  this allows to restart code translation when address translation is changed
- stop translation block after writing to pstate and tl registers
- stop translation block after writing to alternate space
  this can be optimized to stop only if address translation can be changed
  by write operation (e.g. by comparing with MMU ASI values)

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/cpu.h       |   14 ++++++++++----
 target-sparc/helper.c    |   19 ++++++++++++++++++-
 target-sparc/translate.c |   10 +++++++---
 3 files changed, 35 insertions(+), 8 deletions(-)

Comments

Artyom Tarasenko April 4, 2011, 5:25 p.m. UTC | #1 
On Sat, May 22, 2010 at 12:52 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -572,6 +572,23 @@ static int get_physical_address(CPUState *env, target_phys_addr_t *physical,
>     /* ??? We treat everything as a small page, then explicitly flush
>        everything when an entry is evicted.  */
>     *page_size = TARGET_PAGE_SIZE;
> +
> +#if defined (DEBUG_MMU)
> +    /* safety net to catch wrong softmmu index use from dynamic code */

What does "wrong softmmu index" mean? Is it an error or an indication
that something is not implemented?
I'm hitting this net with the following message:

get_physical_address DATA tl=1 mmu_idx=2 primary context=0 secondary
context=0 address=fffb5f40

> +    if (env->tl > 0 && mmu_idx != MMU_NUCLEUS_IDX) {
> +        DPRINTF_MMU("get_physical_address %s tl=%d mmu_idx=%d"
> +                    " primary context=%" PRIx64
> +                    " secondary context=%" PRIx64
> +                " address=%" PRIx64
> +                "\n",
> +                (rw == 2 ? "CODE" : "DATA"),
> +                env->tl, mmu_idx,
> +                env->dmmu.mmu_primary_context,
> +                env->dmmu.mmu_secondary_context,
> +                address);
> +    }
> +#endif

Artyom
Blue Swirl April 4, 2011, 6:37 p.m. UTC | #2 
On Mon, Apr 4, 2011 at 8:25 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
> On Sat, May 22, 2010 at 12:52 PM, Igor V. Kovalenko
> <igor.v.kovalenko@gmail.com> wrote:
>> --- a/target-sparc/helper.c
>> +++ b/target-sparc/helper.c
>> @@ -572,6 +572,23 @@ static int get_physical_address(CPUState *env, target_phys_addr_t *physical,
>>     /* ??? We treat everything as a small page, then explicitly flush
>>        everything when an entry is evicted.  */
>>     *page_size = TARGET_PAGE_SIZE;
>> +
>> +#if defined (DEBUG_MMU)
>> +    /* safety net to catch wrong softmmu index use from dynamic code */
>
> What does "wrong softmmu index" mean? Is it an error or an indication
> that something is not implemented?
> I'm hitting this net with the following message:

The warning is not correct for CPUs without hypervisor mode. On T1/T2,
the default access mode when TL > 1 is hypervisor or nucleus mode.
Even then, the hypervisor could perform some accesses with kernel or
user ASIs.
Igor V. Kovalenko April 4, 2011, 7:12 p.m. UTC | #3 
On Mon, Apr 4, 2011 at 10:37 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Mon, Apr 4, 2011 at 8:25 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
>> On Sat, May 22, 2010 at 12:52 PM, Igor V. Kovalenko
>> <igor.v.kovalenko@gmail.com> wrote:
>>> --- a/target-sparc/helper.c
>>> +++ b/target-sparc/helper.c
>>> @@ -572,6 +572,23 @@ static int get_physical_address(CPUState *env, target_phys_addr_t *physical,
>>>     /* ??? We treat everything as a small page, then explicitly flush
>>>        everything when an entry is evicted.  */
>>>     *page_size = TARGET_PAGE_SIZE;
>>> +
>>> +#if defined (DEBUG_MMU)
>>> +    /* safety net to catch wrong softmmu index use from dynamic code */
>>
>> What does "wrong softmmu index" mean? Is it an error or an indication
>> that something is not implemented?
>> I'm hitting this net with the following message:
>
> The warning is not correct for CPUs without hypervisor mode. On T1/T2,
> the default access mode when TL > 1 is hypervisor or nucleus mode.
> Even then, the hypervisor could perform some accesses with kernel or
> user ASIs.

Right.

The warning is still good for CODE access. Check itself was intended
to catch reusing translated block of user or kernel mode after
entering trap so it must be corrected.
diff mbox

Patch

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 4fd58e9..8f0484b 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -556,7 +556,9 @@  static inline int cpu_mmu_index(CPUState *env1)
 #elif !defined(TARGET_SPARC64)
     return env1->psrs;
 #else
-    if (cpu_hypervisor_mode(env1)) {
+    if (env1->tl > 0) {
+        return MMU_NUCLEUS_IDX;
+    } else if (cpu_hypervisor_mode(env1)) {
         return MMU_HYPV_IDX;
     } else if (cpu_supervisor_mode(env1)) {
         return MMU_KERNEL_IDX;
@@ -636,9 +638,13 @@  static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
     *cs_base = env->npc;
 #ifdef TARGET_SPARC64
     // AM . Combined FPU enable bits . PRIV . DMMU enabled . IMMU enabled
-    *flags = ((env->pstate & PS_AM) << 2)
-        | (((env->pstate & PS_PEF) >> 1) | ((env->fprs & FPRS_FEF) << 2))
-        | (env->pstate & PS_PRIV) | ((env->lsu & (DMMU_E | IMMU_E)) >> 2);
+    *flags = ((env->pstate & PS_AM) << 2)          /* 5 */
+        | (((env->pstate & PS_PEF) >> 1)           /* 3 */
+        | ((env->fprs & FPRS_FEF) << 2))           /* 4 */
+        | (env->pstate & PS_PRIV)                  /* 2 */
+        | ((env->lsu & (DMMU_E | IMMU_E)) >> 2)    /* 1, 0 */
+        | ((env->tl & 0xff) << 8)
+        | (env->dmmu.mmu_primary_context << 16);   /* 16... */
 #else
     // FPU enable . Supervisor
     *flags = (env->psref << 4) | env->psrs;
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 1045c31..96a22f3 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -572,6 +572,23 @@  static int get_physical_address(CPUState *env, target_phys_addr_t *physical,
     /* ??? We treat everything as a small page, then explicitly flush
        everything when an entry is evicted.  */
     *page_size = TARGET_PAGE_SIZE;
+
+#if defined (DEBUG_MMU)
+    /* safety net to catch wrong softmmu index use from dynamic code */
+    if (env->tl > 0 && mmu_idx != MMU_NUCLEUS_IDX) {
+        DPRINTF_MMU("get_physical_address %s tl=%d mmu_idx=%d"
+                    " primary context=%" PRIx64
+                    " secondary context=%" PRIx64
+                " address=%" PRIx64
+                "\n",
+                (rw == 2 ? "CODE" : "DATA"),
+                env->tl, mmu_idx,
+                env->dmmu.mmu_primary_context,
+                env->dmmu.mmu_secondary_context,
+                address);
+    }
+#endif
+
     if (rw == 2)
         return get_physical_address_code(env, physical, prot, address,
                                          mmu_idx);
@@ -718,7 +735,7 @@  target_phys_addr_t cpu_get_phys_page_nofault(CPUState *env, target_ulong addr,
 
 target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr)
 {
-    return cpu_get_phys_page_nofault(env, addr, MMU_KERNEL_IDX);
+    return cpu_get_phys_page_nofault(env, addr, cpu_mmu_index(env));
 }
 #endif
 
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 86096d2..72ca0b4 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -3484,14 +3484,14 @@  static void disas_sparc_insn(DisasContext * dc)
                             case 6: // pstate
                                 save_state(dc, cpu_cond);
                                 gen_helper_wrpstate(cpu_tmp0);
-                                gen_op_next_insn();
-                                tcg_gen_exit_tb(0);
-                                dc->is_br = 1;
+                                dc->npc = DYNAMIC_PC;
                                 break;
                             case 7: // tl
+                                save_state(dc, cpu_cond);
                                 tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_tmp0);
                                 tcg_gen_st_i32(cpu_tmp32, cpu_env,
                                                offsetof(CPUSPARCState, tl));
+                                dc->npc = DYNAMIC_PC;
                                 break;
                             case 8: // pil
                                 gen_helper_wrpil(cpu_tmp0);
@@ -4550,6 +4550,7 @@  static void disas_sparc_insn(DisasContext * dc)
 #endif
                     save_state(dc, cpu_cond);
                     gen_st_asi(cpu_val, cpu_addr, insn, 4);
+                    dc->npc = DYNAMIC_PC;
                     break;
                 case 0x15: /* stba, store byte alternate */
 #ifndef TARGET_SPARC64
@@ -4560,6 +4561,7 @@  static void disas_sparc_insn(DisasContext * dc)
 #endif
                     save_state(dc, cpu_cond);
                     gen_st_asi(cpu_val, cpu_addr, insn, 1);
+                    dc->npc = DYNAMIC_PC;
                     break;
                 case 0x16: /* stha, store halfword alternate */
 #ifndef TARGET_SPARC64
@@ -4570,6 +4572,7 @@  static void disas_sparc_insn(DisasContext * dc)
 #endif
                     save_state(dc, cpu_cond);
                     gen_st_asi(cpu_val, cpu_addr, insn, 2);
+                    dc->npc = DYNAMIC_PC;
                     break;
                 case 0x17: /* stda, store double word alternate */
 #ifndef TARGET_SPARC64
@@ -4594,6 +4597,7 @@  static void disas_sparc_insn(DisasContext * dc)
                 case 0x1e: /* V9 stxa */
                     save_state(dc, cpu_cond);
                     gen_st_asi(cpu_val, cpu_addr, insn, 8);
+                    dc->npc = DYNAMIC_PC;
                     break;
 #endif
                 default: