Patchwork [12/15] Support 1T segments on ppc

login
register
mail settings
Submitter David Gibson
Date Feb. 12, 2011, 2:54 p.m.
Message ID <1297522467-5975-13-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/82927/
State New
Headers show

Comments

David Gibson - Feb. 12, 2011, 2:54 p.m.
Traditionally, the "segments" used for the two-stage translation used on
powerpc MMUs were 256MB in size.  This was the only option on all hash
page table based 32-bit powerpc cpus, and on the earlier 64-bit hash page
table based cpus.  However, newer 64-bit cpus also permit 1TB segments

This patch adds support for 1TB segment translation to the qemu code.

Signed-off-by: David Gibson <dwg@au1.ibm.com>
---
 target-ppc/cpu.h    |    7 ++++++
 target-ppc/helper.c |   58 ++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 48 insertions(+), 17 deletions(-)
Alexander Graf - Feb. 12, 2011, 3:57 p.m.
On 12.02.2011, at 15:54, David Gibson wrote:

> Traditionally, the "segments" used for the two-stage translation used on
> powerpc MMUs were 256MB in size.  This was the only option on all hash
> page table based 32-bit powerpc cpus, and on the earlier 64-bit hash page
> table based cpus.  However, newer 64-bit cpus also permit 1TB segments
> 
> This patch adds support for 1TB segment translation to the qemu code.
> 
> Signed-off-by: David Gibson <dwg@au1.ibm.com>
> ---
> target-ppc/cpu.h    |    7 ++++++
> target-ppc/helper.c |   58 ++++++++++++++++++++++++++++++++++++---------------
> 2 files changed, 48 insertions(+), 17 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 3df6758..53b788f 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -114,6 +114,7 @@ enum powerpc_mmu_t {
>     POWERPC_MMU_601        = 0x0000000A,
> #if defined(TARGET_PPC64)
> #define POWERPC_MMU_64       0x00010000
> +#define POWERPC_MMU_1TSEG    0x00020000
>     /* 64 bits PowerPC MMU                                     */
>     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
>     /* 620 variant (no segment exceptions)                     */
> @@ -382,9 +383,11 @@ struct ppc_slb_t {
> 
> /* Bits in the SLB VSID word */
> #define SLB_VSID_SHIFT          12
> +#define SLB_VSID_SHIFT_1T       24
> #define SLB_VSID_SSIZE_SHIFT    62
> #define SLB_VSID_B              0xc000000000000000ULL
> #define SLB_VSID_B_256M         0x0000000000000000ULL
> +#define SLB_VSID_B_1T           0x4000000000000000ULL
> #define SLB_VSID_VSID           0x3FFFFFFFFFFFF000ULL
> #define SLB_VSID_PTEM           (SLB_VSID_B | SLB_VSID_VSID)
> #define SLB_VSID_KS             0x0000000000000800ULL
> @@ -398,6 +401,10 @@ struct ppc_slb_t {
> #define SEGMENT_SHIFT_256M      28
> #define SEGMENT_MASK_256M       ~((1ULL << SEGMENT_SHIFT_256M) - 1)
> 
> +#define SEGMENT_SHIFT_1T        40
> +#define SEGMENT_MASK_1T         ~((1ULL << SEGMENT_SHIFT_1T) - 1)
> +
> +
> /*****************************************************************************/
> /* Machine state register bits definition                                    */
> #define MSR_SF   63 /* Sixty-four-bit mode                            hflags */
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index 6a1127f..158da09 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -669,19 +669,25 @@ static inline int find_pte(CPUState *env, mmu_ctx_t *ctx, int h, int rw,
> #if defined(TARGET_PPC64)
> static inline ppc_slb_t *slb_lookup(CPUPPCState *env, target_ulong eaddr)
> {
> -    uint64_t esid;
> +    uint64_t match_256M, match_1T;
>     int n;
> 
>     LOG_SLB("%s: eaddr " TARGET_FMT_lx "\n", __func__, eaddr);
> 
> -    esid = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
> +    match_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V |
> +        (SLB_VSID_B_256M >> SLB_VSID_SSIZE_SHIFT);
> +    match_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V |
> +        (SLB_VSID_B_1T >> SLB_VSID_SSIZE_SHIFT);
> 
>     for (n = 0; n < env->slb_nr; n++) {
>         ppc_slb_t *slb = &env->slb[n];
> 
>         LOG_SLB("%s: slot %d %016" PRIx64 " %016"
>                     PRIx64 "\n", __func__, n, slb->esid, slb->vsid);
> -        if (slb->esid == esid) {
> +        /* We check for 1T matches on all MMUs here - if the MMU
> +         * doesn't have 1T segment support, we will have prevented 1T
> +         * entries from being inserted in the slbmte code. */
> +        if ((slb->esid == match_256M) || (slb->esid == match_1T)) {
>             return slb;
>         }
>     }
> @@ -734,16 +740,21 @@ void ppc_slb_invalidate_one (CPUPPCState *env, uint64_t T0)
> int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
> {
>     int slot = rb & 0xfff;
> -    uint64_t esid = rb & ~0xfff;
>     ppc_slb_t *slb = &env->slb[slot];
> -
> -    if (slot >= env->slb_nr) {
> -        return -1;
> -    }
> -
> -    slb->esid = esid;
> + 
> +    if (rb & (0x1000 - env->slb_nr))

Braces...

> +	return -1; /* Reserved bits set or slot too high */
> +    if (rs & (SLB_VSID_B & ~SLB_VSID_B_1T))

here too

> +	return -1; /* Bad segment size */
> +    if ((rs & SLB_VSID_B) && !(env->mmu_model & POWERPC_MMU_1TSEG))

and here

> + 	return -1; /* 1T segment on MMU that doesn't support it */
> + 
> +    /* We stuff a copy of the B field into slb->esid to simplify
> +     * lookup later */
> +    slb->esid = (rb & (SLB_ESID_ESID | SLB_ESID_V)) |
> +        (rs >> SLB_VSID_SSIZE_SHIFT);

Wouldn't it be easier to add another field?

>     slb->vsid = rs;
> -
> + 
>     LOG_SLB("%s: %d " TARGET_FMT_lx " - " TARGET_FMT_lx " => %016" PRIx64
>             " %016" PRIx64 "\n", __func__, slot, rb, rs,
>             slb->esid, slb->vsid);
> @@ -760,7 +771,8 @@ int ppc_load_slb_esid (CPUPPCState *env, target_ulong rb, target_ulong *rt)
>         return -1;
>     }
> 
> -    *rt = slb->esid;
> +    /* Mask out the extra copy of the B field inserted in store_slb */
> +    *rt = slb->esid & ~0x3;
>     return 0;
> }
> 
> @@ -793,6 +805,7 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
>     if (env->mmu_model & POWERPC_MMU_64) {
>         ppc_slb_t *slb;
>         target_ulong pageaddr;
> +        int segment_bits;
> 
>         LOG_MMU("Check SLBs\n");
>         slb = slb_lookup(env, eaddr);
> @@ -800,7 +813,14 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
>             return -5;
>         }
> 
> -        vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
> +	if (slb->vsid & SLB_VSID_B) {
> +	    vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT_1T;
> +	    segment_bits = 40;
> +	} else {
> +	    vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
> +	    segment_bits = 28;
> +	}
> +
>         target_page_bits = (slb->vsid & SLB_VSID_L)
>             ? TARGET_PAGE_BITS_16M : TARGET_PAGE_BITS;
>         ctx->key = !!(pr ? (slb->vsid & SLB_VSID_KP)
> @@ -808,11 +828,15 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
>         ds = 0;
>         ctx->nx = !!(slb->vsid & SLB_VSID_N);
> 
> -        pageaddr = eaddr & ((1ULL << 28) - (1ULL << target_page_bits));
> -        /* XXX: this is false for 1 TB segments */
> -        hash = vsid ^ (pageaddr >> target_page_bits);
> +        pageaddr = eaddr & ((1ULL << segment_bits) 
> +                            - (1ULL << target_page_bits));
> +	if (slb->vsid & SLB_VSID_B)

Braces

> +	    hash = vsid ^ (vsid << 25) ^ (pageaddr >> target_page_bits);
> +	else

here too


Alex
David Gibson - Feb. 13, 2011, 9:34 a.m.
On Sat, Feb 12, 2011 at 04:57:39PM +0100, Alexander Graf wrote:
> On 12.02.2011, at 15:54, David Gibson wrote:
[snip]
> > +    if (rb & (0x1000 - env->slb_nr))
> 
> Braces...

Oops, yeah.  These later patches in the series I haven't really
audited for coding style adequately yet.  I'll fix these before the
next version.

[snip]
> > + 	return -1; /* 1T segment on MMU that doesn't support it */
> > + 
> > +    /* We stuff a copy of the B field into slb->esid to simplify
> > +     * lookup later */
> > +    slb->esid = (rb & (SLB_ESID_ESID | SLB_ESID_V)) |
> > +        (rs >> SLB_VSID_SSIZE_SHIFT);
> 
> Wouldn't it be easier to add another field?

Easier for what?  The reason I put these bits in here is that the rest
of the things slb_lookup() needs to scan for are all in the esid
field, so putting B in there means slb_lookup() needs only one
comparison per-slot, per segment size.
Alexander Graf - Feb. 13, 2011, 12:37 p.m.
On 13.02.2011, at 10:34, David Gibson wrote:

> On Sat, Feb 12, 2011 at 04:57:39PM +0100, Alexander Graf wrote:
>> On 12.02.2011, at 15:54, David Gibson wrote:
> [snip]
>>> +    if (rb & (0x1000 - env->slb_nr))
>> 
>> Braces...
> 
> Oops, yeah.  These later patches in the series I haven't really
> audited for coding style adequately yet.  I'll fix these before the
> next version.
> 
> [snip]
>>> + 	return -1; /* 1T segment on MMU that doesn't support it */
>>> + 
>>> +    /* We stuff a copy of the B field into slb->esid to simplify
>>> +     * lookup later */
>>> +    slb->esid = (rb & (SLB_ESID_ESID | SLB_ESID_V)) |
>>> +        (rs >> SLB_VSID_SSIZE_SHIFT);
>> 
>> Wouldn't it be easier to add another field?
> 
> Easier for what?  The reason I put these bits in here is that the rest
> of the things slb_lookup() needs to scan for are all in the esid
> field, so putting B in there means slb_lookup() needs only one
> comparison per-slot, per segment size.

Hrm - but it also needs random & ~3 masking in other code which is very unpretty. Comparing two numbers really shouldn't hurt performance too much, but makes the code better maintainable.

struct slb_entry {
    uint64_t esid;
    uint64_t vsid;
    int b;
}

or so :).

Alex
David Gibson - Feb. 13, 2011, 1:38 p.m.
On Sun, Feb 13, 2011 at 01:37:12PM +0100, Alexander Graf wrote:
> On 13.02.2011, at 10:34, David Gibson wrote:
> > On Sat, Feb 12, 2011 at 04:57:39PM +0100, Alexander Graf wrote:
> >> On 12.02.2011, at 15:54, David Gibson wrote:
> > [snip]
> >>> +    if (rb & (0x1000 - env->slb_nr))
> >> 
> >> Braces...
> > 
> > Oops, yeah.  These later patches in the series I haven't really
> > audited for coding style adequately yet.  I'll fix these before the
> > next version.
> > 
> > [snip]
> >>> + 	return -1; /* 1T segment on MMU that doesn't support it */
> >>> + 
> >>> +    /* We stuff a copy of the B field into slb->esid to simplify
> >>> +     * lookup later */
> >>> +    slb->esid = (rb & (SLB_ESID_ESID | SLB_ESID_V)) |
> >>> +        (rs >> SLB_VSID_SSIZE_SHIFT);
> >> 
> >> Wouldn't it be easier to add another field?
> > 
> > Easier for what?  The reason I put these bits in here is that the rest
> > of the things slb_lookup() needs to scan for are all in the esid
> > field, so putting B in there means slb_lookup() needs only one
> > comparison per-slot, per segment size.
> 
> Hrm - but it also needs random & ~3 masking in other code which is
> very unpretty. Comparing two numbers really shouldn't hurt
> performance too much, but makes the code better maintainable.

Well, it's only one place.  But fair enough, I'll avoid this hack in
the next version.

> struct slb_entry {
>     uint64_t esid;
>     uint64_t vsid;
>     int b;
> }
> 
> or so :).

Actually, we don't even need that.  The B field is already in
slb->vsid.

Patch

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 3df6758..53b788f 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -114,6 +114,7 @@  enum powerpc_mmu_t {
     POWERPC_MMU_601        = 0x0000000A,
 #if defined(TARGET_PPC64)
 #define POWERPC_MMU_64       0x00010000
+#define POWERPC_MMU_1TSEG    0x00020000
     /* 64 bits PowerPC MMU                                     */
     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
     /* 620 variant (no segment exceptions)                     */
@@ -382,9 +383,11 @@  struct ppc_slb_t {
 
 /* Bits in the SLB VSID word */
 #define SLB_VSID_SHIFT          12
+#define SLB_VSID_SHIFT_1T       24
 #define SLB_VSID_SSIZE_SHIFT    62
 #define SLB_VSID_B              0xc000000000000000ULL
 #define SLB_VSID_B_256M         0x0000000000000000ULL
+#define SLB_VSID_B_1T           0x4000000000000000ULL
 #define SLB_VSID_VSID           0x3FFFFFFFFFFFF000ULL
 #define SLB_VSID_PTEM           (SLB_VSID_B | SLB_VSID_VSID)
 #define SLB_VSID_KS             0x0000000000000800ULL
@@ -398,6 +401,10 @@  struct ppc_slb_t {
 #define SEGMENT_SHIFT_256M      28
 #define SEGMENT_MASK_256M       ~((1ULL << SEGMENT_SHIFT_256M) - 1)
 
+#define SEGMENT_SHIFT_1T        40
+#define SEGMENT_MASK_1T         ~((1ULL << SEGMENT_SHIFT_1T) - 1)
+
+
 /*****************************************************************************/
 /* Machine state register bits definition                                    */
 #define MSR_SF   63 /* Sixty-four-bit mode                            hflags */
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 6a1127f..158da09 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -669,19 +669,25 @@  static inline int find_pte(CPUState *env, mmu_ctx_t *ctx, int h, int rw,
 #if defined(TARGET_PPC64)
 static inline ppc_slb_t *slb_lookup(CPUPPCState *env, target_ulong eaddr)
 {
-    uint64_t esid;
+    uint64_t match_256M, match_1T;
     int n;
 
     LOG_SLB("%s: eaddr " TARGET_FMT_lx "\n", __func__, eaddr);
 
-    esid = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
+    match_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V |
+        (SLB_VSID_B_256M >> SLB_VSID_SSIZE_SHIFT);
+    match_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V |
+        (SLB_VSID_B_1T >> SLB_VSID_SSIZE_SHIFT);
 
     for (n = 0; n < env->slb_nr; n++) {
         ppc_slb_t *slb = &env->slb[n];
 
         LOG_SLB("%s: slot %d %016" PRIx64 " %016"
                     PRIx64 "\n", __func__, n, slb->esid, slb->vsid);
-        if (slb->esid == esid) {
+        /* We check for 1T matches on all MMUs here - if the MMU
+         * doesn't have 1T segment support, we will have prevented 1T
+         * entries from being inserted in the slbmte code. */
+        if ((slb->esid == match_256M) || (slb->esid == match_1T)) {
             return slb;
         }
     }
@@ -734,16 +740,21 @@  void ppc_slb_invalidate_one (CPUPPCState *env, uint64_t T0)
 int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
 {
     int slot = rb & 0xfff;
-    uint64_t esid = rb & ~0xfff;
     ppc_slb_t *slb = &env->slb[slot];
-
-    if (slot >= env->slb_nr) {
-        return -1;
-    }
-
-    slb->esid = esid;
+ 
+    if (rb & (0x1000 - env->slb_nr))
+	return -1; /* Reserved bits set or slot too high */
+    if (rs & (SLB_VSID_B & ~SLB_VSID_B_1T))
+	return -1; /* Bad segment size */
+    if ((rs & SLB_VSID_B) && !(env->mmu_model & POWERPC_MMU_1TSEG))
+ 	return -1; /* 1T segment on MMU that doesn't support it */
+ 
+    /* We stuff a copy of the B field into slb->esid to simplify
+     * lookup later */
+    slb->esid = (rb & (SLB_ESID_ESID | SLB_ESID_V)) |
+        (rs >> SLB_VSID_SSIZE_SHIFT);
     slb->vsid = rs;
-
+ 
     LOG_SLB("%s: %d " TARGET_FMT_lx " - " TARGET_FMT_lx " => %016" PRIx64
             " %016" PRIx64 "\n", __func__, slot, rb, rs,
             slb->esid, slb->vsid);
@@ -760,7 +771,8 @@  int ppc_load_slb_esid (CPUPPCState *env, target_ulong rb, target_ulong *rt)
         return -1;
     }
 
-    *rt = slb->esid;
+    /* Mask out the extra copy of the B field inserted in store_slb */
+    *rt = slb->esid & ~0x3;
     return 0;
 }
 
@@ -793,6 +805,7 @@  static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
     if (env->mmu_model & POWERPC_MMU_64) {
         ppc_slb_t *slb;
         target_ulong pageaddr;
+        int segment_bits;
 
         LOG_MMU("Check SLBs\n");
         slb = slb_lookup(env, eaddr);
@@ -800,7 +813,14 @@  static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
             return -5;
         }
 
-        vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
+	if (slb->vsid & SLB_VSID_B) {
+	    vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT_1T;
+	    segment_bits = 40;
+	} else {
+	    vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
+	    segment_bits = 28;
+	}
+
         target_page_bits = (slb->vsid & SLB_VSID_L)
             ? TARGET_PAGE_BITS_16M : TARGET_PAGE_BITS;
         ctx->key = !!(pr ? (slb->vsid & SLB_VSID_KP)
@@ -808,11 +828,15 @@  static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
         ds = 0;
         ctx->nx = !!(slb->vsid & SLB_VSID_N);
 
-        pageaddr = eaddr & ((1ULL << 28) - (1ULL << target_page_bits));
-        /* XXX: this is false for 1 TB segments */
-        hash = vsid ^ (pageaddr >> target_page_bits);
+        pageaddr = eaddr & ((1ULL << segment_bits) 
+                            - (1ULL << target_page_bits));
+	if (slb->vsid & SLB_VSID_B)
+	    hash = vsid ^ (vsid << 25) ^ (pageaddr >> target_page_bits);
+	else
+	    hash = vsid ^ (pageaddr >> target_page_bits);
         /* Only 5 bits of the page index are used in the AVPN */
-        ctx->ptem = (slb->vsid & SLB_VSID_PTEM) | ((pageaddr >> 16) & 0x0F80);
+        ctx->ptem = (slb->vsid & SLB_VSID_PTEM) | 
+            ((pageaddr >> 16) & ((1ULL << segment_bits) - 0x80));
     } else
 #endif /* defined(TARGET_PPC64) */
     {