Patchwork [2/3] sparc64: implement global translation table entries

login
register
mail settings
Submitter Igor V. Kovalenko
Date May 3, 2010, 7:29 a.m.
Message ID <20100503072939.367.3130.stgit@skyserv>
Download mbox | patch
Permalink /patch/51482/
State New
Headers show

Comments

Igor V. Kovalenko - May 3, 2010, 7:29 a.m.
From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

- match global tte against any context
- show global tte in MMU dump

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/cpu.h       |   18 ++++++++++++++++
 target-sparc/helper.c    |   33 ++++++++++++-----------------
 target-sparc/op_helper.c |   52 ++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 75 insertions(+), 28 deletions(-)
Blue Swirl - May 3, 2010, 8:06 p.m.
On 5/3/10, Igor V. Kovalenko <igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
>  - match global tte against any context
>  - show global tte in MMU dump
>
>  Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

I get this error:
  CC    sparc64-softmmu/op_helper.o
cc1: warnings being treated as errors
/src/qemu/target-sparc/op_helper.c: In function 'demap_tlb':
/src/qemu/target-sparc/op_helper.c:129: error: 'context' may be used
uninitialized in this function
Igor V. Kovalenko - May 3, 2010, 8:21 p.m.
On Tue, May 4, 2010 at 12:06 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On 5/3/10, Igor V. Kovalenko <igor.v.kovalenko@gmail.com> wrote:
>> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>
>>  - match global tte against any context
>>  - show global tte in MMU dump
>>
>>  Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> I get this error:
>  CC    sparc64-softmmu/op_helper.o
> cc1: warnings being treated as errors
> /src/qemu/target-sparc/op_helper.c: In function 'demap_tlb':
> /src/qemu/target-sparc/op_helper.c:129: error: 'context' may be used
> uninitialized in this function
>

My gcc (Gentoo 4.4.3-r2 p1.2) is silent, and looking at the change all
4 possible cases are handled in switch statement.
It should initializes context in 3 usable cases and returns from the
4th which is reserved.
How do we fix this issue?
Blue Swirl - May 3, 2010, 8:29 p.m.
On 5/3/10, Igor Kovalenko <igor.v.kovalenko@gmail.com> wrote:
> On Tue, May 4, 2010 at 12:06 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
>  > On 5/3/10, Igor V. Kovalenko <igor.v.kovalenko@gmail.com> wrote:
>  >> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>  >>
>  >>  - match global tte against any context
>  >>  - show global tte in MMU dump
>  >>
>  >>  Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>  >
>  > I get this error:
>  >  CC    sparc64-softmmu/op_helper.o
>  > cc1: warnings being treated as errors
>  > /src/qemu/target-sparc/op_helper.c: In function 'demap_tlb':
>  > /src/qemu/target-sparc/op_helper.c:129: error: 'context' may be used
>  > uninitialized in this function
>  >
>
>
> My gcc (Gentoo 4.4.3-r2 p1.2) is silent, and looking at the change all
>  4 possible cases are handled in switch statement.

I think gcc is not intelligent enough to know that x & 3 has only 4
possible cases. :-)

>  It should initializes context in 3 usable cases and returns from the
>  4th which is reserved.
>  How do we fix this issue?

I'd add a default case to one of the cases. Another possibility is to
initialize the context with 0 and then make one of the cases empty.
Igor V. Kovalenko - May 4, 2010, 7:12 p.m.
On Tue, May 4, 2010 at 12:29 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On 5/3/10, Igor Kovalenko <igor.v.kovalenko@gmail.com> wrote:
>> On Tue, May 4, 2010 at 12:06 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
>>  > On 5/3/10, Igor V. Kovalenko <igor.v.kovalenko@gmail.com> wrote:
>>  >> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>  >>
>>  >>  - match global tte against any context
>>  >>  - show global tte in MMU dump
>>  >>
>>  >>  Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>  >
>>  > I get this error:
>>  >  CC    sparc64-softmmu/op_helper.o
>>  > cc1: warnings being treated as errors
>>  > /src/qemu/target-sparc/op_helper.c: In function 'demap_tlb':
>>  > /src/qemu/target-sparc/op_helper.c:129: error: 'context' may be used
>>  > uninitialized in this function
>>  >
>>
>>
>> My gcc (Gentoo 4.4.3-r2 p1.2) is silent, and looking at the change all
>>  4 possible cases are handled in switch statement.
>
> I think gcc is not intelligent enough to know that x & 3 has only 4
> possible cases. :-)
>
>>  It should initializes context in 3 usable cases and returns from the
>>  4th which is reserved.
>>  How do we fix this issue?
>
> I'd add a default case to one of the cases. Another possibility is to
> initialize the context with 0 and then make one of the cases empty.
>

Added default case, resent this patch only.

Patch

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 0e7f390..b705728 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -513,6 +513,24 @@  static inline void cpu_set_cwp(CPUSPARCState *env1, int new_cwp)
 /* sun4m.c, sun4u.c */
 void cpu_check_irqs(CPUSPARCState *env);
 
+#if defined (TARGET_SPARC64)
+
+static inline int compare_masked(uint64_t x, uint64_t y, uint64_t mask)
+{
+    return (x & mask) == (y & mask);
+}
+
+#define MMU_CONTEXT_BITS 13
+#define MMU_CONTEXT_MASK ((1 << MMU_CONTEXT_BITS) - 1)
+
+static inline int tlb_compare_context(const SparcTLBEntry *tlb,
+                                      uint64_t context)
+{
+    return compare_masked(context, tlb->tag, MMU_CONTEXT_MASK);
+}
+
+#endif
+
 static inline void PUT_PSR(CPUSPARCState *env1, target_ulong val)
 {
     env1->psr = val & PSR_ICC;
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 1f0f7d4..4ece01b 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -381,17 +381,11 @@  static inline target_phys_addr_t ultrasparc_truncate_physical(uint64_t x)
  * UltraSparc IIi I/DMMUs
  */
 
-static inline int compare_masked(uint64_t x, uint64_t y, uint64_t mask)
-{
-    return (x & mask) == (y & mask);
-}
-
 // Returns true if TTE tag is valid and matches virtual address value in context
 // requires virtual address mask value calculated from TTE entry size
 static inline int ultrasparc_tag_match(SparcTLBEntry *tlb,
                                        uint64_t address, uint64_t context,
-                                       target_phys_addr_t *physical,
-                                       int is_nucleus)
+                                       target_phys_addr_t *physical)
 {
     uint64_t mask;
 
@@ -413,8 +407,7 @@  static inline int ultrasparc_tag_match(SparcTLBEntry *tlb,
 
     // valid, context match, virtual address match?
     if (TTE_IS_VALID(tlb->tte) &&
-        ((is_nucleus && compare_masked(0, tlb->tag, 0x1fff))
-         || TTE_IS_GLOBAL(tlb->tte) || compare_masked(context, tlb->tag, 0x1fff))
+        (TTE_IS_GLOBAL(tlb->tte) || tlb_compare_context(tlb, context))
         && compare_masked(address, tlb->tag, mask))
     {
         // decode physical address
@@ -431,7 +424,6 @@  static int get_physical_address_data(CPUState *env,
 {
     unsigned int i;
     uint64_t context;
-    int is_nucleus;
 
     if ((env->lsu & DMMU_E) == 0) { /* DMMU disabled */
         *physical = ultrasparc_truncate_physical(address);
@@ -439,14 +431,16 @@  static int get_physical_address_data(CPUState *env,
         return 0;
     }
 
-    context = env->dmmu.mmu_primary_context & 0x1fff;
-    is_nucleus = env->tl > 0;
+    if (env->tl == 0) {
+        context = env->dmmu.mmu_primary_context & 0x1fff;
+    } else {
+        context = 0;
+    }
 
     for (i = 0; i < 64; i++) {
         // ctx match, vaddr match, valid?
         if (ultrasparc_tag_match(&env->dtlb[i],
-                                 address, context, physical,
-                                 is_nucleus)) {
+                                 address, context, physical)) {
             // access ok?
             if (((env->dtlb[i].tte & 0x4) && is_user) ||
                 (!(env->dtlb[i].tte & 0x2) && (rw == 1))) {
@@ -492,7 +486,6 @@  static int get_physical_address_code(CPUState *env,
 {
     unsigned int i;
     uint64_t context;
-    int is_nucleus;
 
     if ((env->lsu & IMMU_E) == 0 || (env->pstate & PS_RED) != 0) {
         /* IMMU disabled */
@@ -501,14 +494,16 @@  static int get_physical_address_code(CPUState *env,
         return 0;
     }
 
-    context = env->dmmu.mmu_primary_context & 0x1fff;
-    is_nucleus = env->tl > 0;
+    if (env->tl == 0) {
+        context = env->dmmu.mmu_primary_context & 0x1fff;
+    } else {
+        context = 0;
+    }
 
     for (i = 0; i < 64; i++) {
         // ctx match, vaddr match, valid?
         if (ultrasparc_tag_match(&env->itlb[i],
-                                 address, context, physical,
-                                 is_nucleus)) {
+                                 address, context, physical)) {
             // access ok?
             if ((env->itlb[i].tte & 0x4) && is_user) {
                 if (env->immu.sfsr) /* Fault status register */
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index b27778b..e048845 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -129,24 +129,58 @@  static void demap_tlb(SparcTLBEntry *tlb, target_ulong demap_addr,
 {
     unsigned int i;
     target_ulong mask;
+    uint64_t context;
+
+    int is_demap_context = (demap_addr >> 6) & 1;
+
+    // demap context
+    switch ((demap_addr >> 4) & 3) {
+    case 0: // primary
+        context = env1->dmmu.mmu_primary_context;
+        break;
+    case 1: // secondary
+        context = env1->dmmu.mmu_secondary_context;
+        break;
+    case 2: // nucleus
+        context = 0;
+        break;
+    case 3: // reserved
+        return;
+    }
 
     for (i = 0; i < 64; i++) {
         if (TTE_IS_VALID(tlb[i].tte)) {
 
-            mask = 0xffffffffffffe000ULL;
-            mask <<= 3 * ((tlb[i].tte >> 61) & 3);
+            if (is_demap_context) {
+                // will remove non-global entries matching context value
+                if (TTE_IS_GLOBAL(tlb[i].tte) ||
+                    !tlb_compare_context(&tlb[i], context)) {
+                    continue;
+                }
+            } else {
+                // demap page
+                // will remove any entry matching VA
+                mask = 0xffffffffffffe000ULL;
+                mask <<= 3 * ((tlb[i].tte >> 61) & 3);
+
+                if (!compare_masked(demap_addr, tlb[i].tag, mask)) {
+                    continue;
+                }
+
+                // entry should be global or matching context value
+                if (!TTE_IS_GLOBAL(tlb[i].tte) &&
+                    !tlb_compare_context(&tlb[i], context)) {
+                    continue;
+                }
+            }
 
-            if ((demap_addr & mask) == (tlb[i].tag & mask)) {
-                replace_tlb_entry(&tlb[i], 0, 0, env1);
+            replace_tlb_entry(&tlb[i], 0, 0, env1);
 #ifdef DEBUG_MMU
-                DPRINTF_MMU("%s demap invalidated entry [%02u]\n", strmmu, i);
-                dump_mmu(env1);
+            DPRINTF_MMU("%s demap invalidated entry [%02u]\n", strmmu, i);
+            dump_mmu(env1);
 #endif
-            }
-            //return;
         }
     }
-
 }
 
 static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,