| Message ID | 1297522467-5975-13-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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.
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) */ {
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(-)