| Message ID | 20110707234405.GC6748@schlenkerla.am.freescale.net |
|---|---|
| State | New |
| Headers | show |
Hi Scott, I'm a little bit late, but this patch is not compatible with e500. In fact all the modification breaks e500v2 MMU support. What kind PPC core are you working on? Regards, On 07/08/2011 01:44 AM, Scott Wood wrote: > This definition is backward compatible with MAV=1.0 as long as > the guest does not set reserved bits in MAS1/MAS4. > > Also, fix the shift in booke206_tlb_to_page_size -- it's the base > that should be able to hold a 4G page size, not the shift count. > > Signed-off-by: Scott Wood <scottwood@freescale.com> > --- > Unchanged in patchset v2 > > hw/ppce500_mpc8544ds.c | 2 +- > target-ppc/cpu.h | 4 ++-- > target-ppc/helper.c | 5 +++-- > 3 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c > index 3626e26..1aed612 100644 > --- a/hw/ppce500_mpc8544ds.c > +++ b/hw/ppce500_mpc8544ds.c > @@ -187,7 +187,7 @@ out: > /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ > static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) > { > - return (ffs(size >> 10) - 1) >> 1; > + return ffs(size >> 10) - 1; > } > > static void mmubooke_create_initial_mapping(CPUState *env, > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > index f8bf2b1..9cf8327 100644 > --- a/target-ppc/cpu.h > +++ b/target-ppc/cpu.h > @@ -654,8 +654,8 @@ enum { > #define MAS0_ATSEL_TLB 0 > #define MAS0_ATSEL_LRAT MAS0_ATSEL > > -#define MAS1_TSIZE_SHIFT 8 > -#define MAS1_TSIZE_MASK (0xf << MAS1_TSIZE_SHIFT) > +#define MAS1_TSIZE_SHIFT 7 > +#define MAS1_TSIZE_MASK (0x1f << MAS1_TSIZE_SHIFT) > > #define MAS1_TS_SHIFT 12 > #define MAS1_TS (1 << MAS1_TS_SHIFT) > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > index 176128a..892c6e3 100644 > --- a/target-ppc/helper.c > +++ b/target-ppc/helper.c > @@ -1293,7 +1293,7 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) > { > uint32_t tlbncfg; > int tlbn = booke206_tlbm_to_tlbn(env, tlb); > - target_phys_addr_t tlbm_size; > + int tlbm_size; > > tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn]; > > @@ -1301,9 +1301,10 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) > tlbm_size = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; > } else { > tlbm_size = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT; > + tlbm_size <<= 1; > } > > - return (1 << (tlbm_size << 1)) << 10; > + return 1024ULL << tlbm_size; > } > > /* TLB check function for MAS based SoftTLBs */
Hi Fabien, Could you please elaborate a bit on the case that broke for you with these? The patches shouldn't change any guest facing behavior :o. Alex On 07.05.2012, at 17:47, Fabien Chouteau wrote: > Hi Scott, > > I'm a little bit late, but this patch is not compatible with e500. > > In fact all the modification breaks e500v2 MMU support. What kind PPC > core are you working on? > > Regards, > > On 07/08/2011 01:44 AM, Scott Wood wrote: >> This definition is backward compatible with MAV=1.0 as long as >> the guest does not set reserved bits in MAS1/MAS4. >> >> Also, fix the shift in booke206_tlb_to_page_size -- it's the base >> that should be able to hold a 4G page size, not the shift count. >> >> Signed-off-by: Scott Wood <scottwood@freescale.com> >> --- >> Unchanged in patchset v2 >> >> hw/ppce500_mpc8544ds.c | 2 +- >> target-ppc/cpu.h | 4 ++-- >> target-ppc/helper.c | 5 +++-- >> 3 files changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c >> index 3626e26..1aed612 100644 >> --- a/hw/ppce500_mpc8544ds.c >> +++ b/hw/ppce500_mpc8544ds.c >> @@ -187,7 +187,7 @@ out: >> /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ >> static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) >> { >> - return (ffs(size >> 10) - 1) >> 1; >> + return ffs(size >> 10) - 1; >> } >> >> static void mmubooke_create_initial_mapping(CPUState *env, >> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h >> index f8bf2b1..9cf8327 100644 >> --- a/target-ppc/cpu.h >> +++ b/target-ppc/cpu.h >> @@ -654,8 +654,8 @@ enum { >> #define MAS0_ATSEL_TLB 0 >> #define MAS0_ATSEL_LRAT MAS0_ATSEL >> >> -#define MAS1_TSIZE_SHIFT 8 >> -#define MAS1_TSIZE_MASK (0xf << MAS1_TSIZE_SHIFT) >> +#define MAS1_TSIZE_SHIFT 7 >> +#define MAS1_TSIZE_MASK (0x1f << MAS1_TSIZE_SHIFT) >> >> #define MAS1_TS_SHIFT 12 >> #define MAS1_TS (1 << MAS1_TS_SHIFT) >> diff --git a/target-ppc/helper.c b/target-ppc/helper.c >> index 176128a..892c6e3 100644 >> --- a/target-ppc/helper.c >> +++ b/target-ppc/helper.c >> @@ -1293,7 +1293,7 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) >> { >> uint32_t tlbncfg; >> int tlbn = booke206_tlbm_to_tlbn(env, tlb); >> - target_phys_addr_t tlbm_size; >> + int tlbm_size; >> >> tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn]; >> >> @@ -1301,9 +1301,10 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) >> tlbm_size = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; >> } else { >> tlbm_size = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT; >> + tlbm_size <<= 1; >> } >> >> - return (1 << (tlbm_size << 1)) << 10; >> + return 1024ULL << tlbm_size; >> } >> >> /* TLB check function for MAS based SoftTLBs */ > > > -- > Fabien Chouteau
Am 07.05.2012 18:28, schrieb Alexander Graf: > Hi Fabien, > > Could you please elaborate a bit on the case that broke for you with these? The patches shouldn't change any guest facing behavior :o. > > > Alex > > On 07.05.2012, at 17:47, Fabien Chouteau wrote: > >> Hi Scott, >> >> I'm a little bit late, but this patch is not compatible with e500. >> >> In fact all the modification breaks e500v2 MMU support. What kind PPC >> core are you working on? >> >> Regards, >> >> On 07/08/2011 01:44 AM, Scott Wood wrote: >>> This definition is backward compatible with MAV=1.0 as long as >>> the guest does not set reserved bits in MAS1/MAS4. >>> >>> Also, fix the shift in booke206_tlb_to_page_size -- it's the base >>> that should be able to hold a 4G page size, not the shift count. >>> >>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>> --- >>> Unchanged in patchset v2 >>> >>> hw/ppce500_mpc8544ds.c | 2 +- >>> target-ppc/cpu.h | 4 ++-- >>> target-ppc/helper.c | 5 +++-- >>> 3 files changed, 6 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c >>> index 3626e26..1aed612 100644 >>> --- a/hw/ppce500_mpc8544ds.c >>> +++ b/hw/ppce500_mpc8544ds.c >>> @@ -187,7 +187,7 @@ out: >>> /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ >>> static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) >>> { >>> - return (ffs(size >> 10) - 1) >> 1; >>> + return ffs(size >> 10) - 1; >>> } >>> >>> static void mmubooke_create_initial_mapping(CPUState *env, >>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h >>> index f8bf2b1..9cf8327 100644 >>> --- a/target-ppc/cpu.h >>> +++ b/target-ppc/cpu.h >>> @@ -654,8 +654,8 @@ enum { >>> #define MAS0_ATSEL_TLB 0 >>> #define MAS0_ATSEL_LRAT MAS0_ATSEL >>> >>> -#define MAS1_TSIZE_SHIFT 8 >>> -#define MAS1_TSIZE_MASK (0xf << MAS1_TSIZE_SHIFT) >>> +#define MAS1_TSIZE_SHIFT 7 >>> +#define MAS1_TSIZE_MASK (0x1f << MAS1_TSIZE_SHIFT) >>> >>> #define MAS1_TS_SHIFT 12 >>> #define MAS1_TS (1 << MAS1_TS_SHIFT) >>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c >>> index 176128a..892c6e3 100644 >>> --- a/target-ppc/helper.c >>> +++ b/target-ppc/helper.c >>> @@ -1293,7 +1293,7 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) >>> { >>> uint32_t tlbncfg; >>> int tlbn = booke206_tlbm_to_tlbn(env, tlb); >>> - target_phys_addr_t tlbm_size; >>> + int tlbm_size; >>> >>> tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn]; >>> >>> @@ -1301,9 +1301,10 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) >>> tlbm_size = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; >>> } else { >>> tlbm_size = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT; >>> + tlbm_size <<= 1; >>> } >>> >>> - return (1 << (tlbm_size << 1)) << 10; >>> + return 1024ULL << tlbm_size; Here the page size changes, doesn't it? The << 1 shift is only happening in the else branch whereas it was always done before. Andreas >>> } >>> >>> /* TLB check function for MAS based SoftTLBs */
On 05/07/2012 06:28 PM, Alexander Graf wrote: > Hi Fabien, > > Could you please elaborate a bit on the case that broke for you with these? The patches shouldn't change any guest facing behavior :o. > > My bad, The problem comes from my initialization of tlb entries at board reset. I use MAS1_TSIZE_SHIFT: size = 0x1 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ but since the definition as changed, the value is incorrect. It should be: size = 0x10 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ Sorry for the noise... > Alex > > On 07.05.2012, at 17:47, Fabien Chouteau wrote: > >> Hi Scott, >> >> I'm a little bit late, but this patch is not compatible with e500. >> >> In fact all the modification breaks e500v2 MMU support. What kind PPC >> core are you working on? >> >> Regards, >> >> On 07/08/2011 01:44 AM, Scott Wood wrote: >>> This definition is backward compatible with MAV=1.0 as long as >>> the guest does not set reserved bits in MAS1/MAS4. >>> >>> Also, fix the shift in booke206_tlb_to_page_size -- it's the base >>> that should be able to hold a 4G page size, not the shift count. >>> >>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>> --- >>> Unchanged in patchset v2 >>> >>> hw/ppce500_mpc8544ds.c | 2 +- >>> target-ppc/cpu.h | 4 ++-- >>> target-ppc/helper.c | 5 +++-- >>> 3 files changed, 6 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c >>> index 3626e26..1aed612 100644 >>> --- a/hw/ppce500_mpc8544ds.c >>> +++ b/hw/ppce500_mpc8544ds.c >>> @@ -187,7 +187,7 @@ out: >>> /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ >>> static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) >>> { >>> - return (ffs(size >> 10) - 1) >> 1; >>> + return ffs(size >> 10) - 1; >>> } >>> >>> static void mmubooke_create_initial_mapping(CPUState *env, >>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h >>> index f8bf2b1..9cf8327 100644 >>> --- a/target-ppc/cpu.h >>> +++ b/target-ppc/cpu.h >>> @@ -654,8 +654,8 @@ enum { >>> #define MAS0_ATSEL_TLB 0 >>> #define MAS0_ATSEL_LRAT MAS0_ATSEL >>> >>> -#define MAS1_TSIZE_SHIFT 8 >>> -#define MAS1_TSIZE_MASK (0xf << MAS1_TSIZE_SHIFT) >>> +#define MAS1_TSIZE_SHIFT 7 >>> +#define MAS1_TSIZE_MASK (0x1f << MAS1_TSIZE_SHIFT) >>> >>> #define MAS1_TS_SHIFT 12 >>> #define MAS1_TS (1 << MAS1_TS_SHIFT) >>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c >>> index 176128a..892c6e3 100644 >>> --- a/target-ppc/helper.c >>> +++ b/target-ppc/helper.c >>> @@ -1293,7 +1293,7 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) >>> { >>> uint32_t tlbncfg; >>> int tlbn = booke206_tlbm_to_tlbn(env, tlb); >>> - target_phys_addr_t tlbm_size; >>> + int tlbm_size; >>> >>> tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn]; >>> >>> @@ -1301,9 +1301,10 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) >>> tlbm_size = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; >>> } else { >>> tlbm_size = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT; >>> + tlbm_size <<= 1; >>> } >>> >>> - return (1 << (tlbm_size << 1)) << 10; >>> + return 1024ULL << tlbm_size; >>> } >>> >>> /* TLB check function for MAS based SoftTLBs */ >> >> >> -- >> Fabien Chouteau >
On 05/09/2012 05:54 AM, Fabien Chouteau wrote: > On 05/07/2012 06:28 PM, Alexander Graf wrote: >> Hi Fabien, >> >> Could you please elaborate a bit on the case that broke for you with these? The patches shouldn't change any guest facing behavior :o. >> >> > > My bad, > > The problem comes from my initialization of tlb entries at board reset. > I use MAS1_TSIZE_SHIFT: > > size = 0x1 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ > > but since the definition as changed, the value is incorrect. It should > be: > > size = 0x10 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ You should be using booke206_bytes_to_tsize(), or perhaps create some #defines for the various tsizes. -Scott
On 05/15/2012 05:28 PM, Scott Wood wrote: > On 05/09/2012 05:54 AM, Fabien Chouteau wrote: >> On 05/07/2012 06:28 PM, Alexander Graf wrote: >>> Hi Fabien, >>> >>> Could you please elaborate a bit on the case that broke for you with these? The patches shouldn't change any guest facing behavior :o. >>> >>> >> >> My bad, >> >> The problem comes from my initialization of tlb entries at board reset. >> I use MAS1_TSIZE_SHIFT: >> >> size = 0x1 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ >> >> but since the definition as changed, the value is incorrect. It should >> be: >> >> size = 0x10 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ > > You should be using booke206_bytes_to_tsize(), or perhaps create some > #defines for the various tsizes. > Do you mean booke206_page_size_to_tlb()? BTW, this function is defined locally twice and with different implementations. hw/ppce500_mpc8544ds.c:176:static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) hw/ppce500_mpc8544ds.c-177-{ hw/ppce500_mpc8544ds.c-178- return ffs(size >> 10) - 1; hw/ppce500_mpc8544ds.c-179-} -- hw/ppce500_spin.c:71:static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) hw/ppce500_spin.c-72-{ hw/ppce500_spin.c-73- return (ffs(size >> 10) - 1) >> 1; hw/ppce500_spin.c-74-}
On 05/15/2012 11:50 AM, Fabien Chouteau wrote: > On 05/15/2012 05:28 PM, Scott Wood wrote: >> On 05/09/2012 05:54 AM, Fabien Chouteau wrote: >>> On 05/07/2012 06:28 PM, Alexander Graf wrote: >>>> Hi Fabien, >>>> >>>> Could you please elaborate a bit on the case that broke for you with these? The patches shouldn't change any guest facing behavior :o. >>>> >>>> >>> >>> My bad, >>> >>> The problem comes from my initialization of tlb entries at board reset. >>> I use MAS1_TSIZE_SHIFT: >>> >>> size = 0x1 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ >>> >>> but since the definition as changed, the value is incorrect. It should >>> be: >>> >>> size = 0x10 << MAS1_TSIZE_SHIFT; /* 4 KBytes */ >> >> You should be using booke206_bytes_to_tsize(), or perhaps create some >> #defines for the various tsizes. >> > > Do you mean booke206_page_size_to_tlb()? No, I was referring to something on an internal branch, sorry. -Scott
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c index 3626e26..1aed612 100644 --- a/hw/ppce500_mpc8544ds.c +++ b/hw/ppce500_mpc8544ds.c @@ -187,7 +187,7 @@ out: /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) { - return (ffs(size >> 10) - 1) >> 1; + return ffs(size >> 10) - 1; } static void mmubooke_create_initial_mapping(CPUState *env, diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index f8bf2b1..9cf8327 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -654,8 +654,8 @@ enum { #define MAS0_ATSEL_TLB 0 #define MAS0_ATSEL_LRAT MAS0_ATSEL -#define MAS1_TSIZE_SHIFT 8 -#define MAS1_TSIZE_MASK (0xf << MAS1_TSIZE_SHIFT) +#define MAS1_TSIZE_SHIFT 7 +#define MAS1_TSIZE_MASK (0x1f << MAS1_TSIZE_SHIFT) #define MAS1_TS_SHIFT 12 #define MAS1_TS (1 << MAS1_TS_SHIFT) diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 176128a..892c6e3 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -1293,7 +1293,7 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) { uint32_t tlbncfg; int tlbn = booke206_tlbm_to_tlbn(env, tlb); - target_phys_addr_t tlbm_size; + int tlbm_size; tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn]; @@ -1301,9 +1301,10 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) tlbm_size = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; } else { tlbm_size = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT; + tlbm_size <<= 1; } - return (1 << (tlbm_size << 1)) << 10; + return 1024ULL << tlbm_size; } /* TLB check function for MAS based SoftTLBs */
This definition is backward compatible with MAV=1.0 as long as the guest does not set reserved bits in MAS1/MAS4. Also, fix the shift in booke206_tlb_to_page_size -- it's the base that should be able to hold a 4G page size, not the shift count. Signed-off-by: Scott Wood <scottwood@freescale.com> --- Unchanged in patchset v2 hw/ppce500_mpc8544ds.c | 2 +- target-ppc/cpu.h | 4 ++-- target-ppc/helper.c | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-)