| Message ID | 1334088263-10530-1-git-send-email-meadori@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Am 10.04.2012 22:04, schrieb Meador Inge: > commit f7aa558396dd0f6b7a2b22c05cb503c655854102 pulled the dcache and icache > line size initialization inside of a '#if !defined(CONFIG_USER_ONLY)' block. > This is not correct because instructions like 'dcbz' need the dcache size > initialized even for user mode. > > Signed-off-by: Meador Inge <meadori@codesourcery.com> Looks okay and compiles, Reviewed-by: Andreas Färber <afaerber@suse.de> Scott, are you planning to review this e500 patch? Or should I go ahead and apply? Andreas > --- > target-ppc/translate_init.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index b1f8785..ef8735a 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -4466,25 +4466,32 @@ static void init_proc_e500 (CPUPPCState *env, int version) > env->nb_pids = 3; > env->nb_ways = 2; > env->id_tlbs = 0; > +#endif > switch (version) { > case fsl_e500v1: > /* e500v1 */ > +#if !defined(CONFIG_USER_ONLY) > tlbncfg[0] = gen_tlbncfg(2, 1, 1, 0, 256); > tlbncfg[1] = gen_tlbncfg(16, 1, 9, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16); > +#endif > env->dcache_line_size = 32; > env->icache_line_size = 32; > break; > case fsl_e500v2: > /* e500v2 */ > +#if !defined(CONFIG_USER_ONLY) > tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512); > tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16); > +#endif > env->dcache_line_size = 32; > env->icache_line_size = 32; > break; > case fsl_e500mc: > /* e500mc */ > +#if !defined(CONFIG_USER_ONLY) > tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512); > tlbncfg[1] = gen_tlbncfg(64, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 64); > +#endif > env->dcache_line_size = 64; > env->icache_line_size = 64; > l1cfg0 |= 0x1000000; /* 64 byte cache block size */ > @@ -4492,7 +4499,6 @@ static void init_proc_e500 (CPUPPCState *env, int version) > default: > cpu_abort(env, "Unknown CPU: " TARGET_FMT_lx "\n", env->spr[SPR_PVR]); > } > -#endif > gen_spr_BookE206(env, 0x000000DF, tlbncfg); > /* XXX : not implemented */ > spr_register(env, SPR_HID0, "HID0",
On 04/12/2012 11:59 AM, Andreas Färber wrote: > Am 10.04.2012 22:04, schrieb Meador Inge: >> commit f7aa558396dd0f6b7a2b22c05cb503c655854102 pulled the dcache and icache >> line size initialization inside of a '#if !defined(CONFIG_USER_ONLY)' block. >> This is not correct because instructions like 'dcbz' need the dcache size >> initialized even for user mode. >> >> Signed-off-by: Meador Inge <meadori@codesourcery.com> > > Looks okay and compiles, > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > Scott, are you planning to review this e500 patch? Or should I go ahead > and apply? I'm OK with it, though it may make more sense for USER_ONLY to just pick an arbitrary cache line size (probably 32) than to try to imitate a specific core. -Scott
Am 12.04.2012 19:24, schrieb Scott Wood: > On 04/12/2012 11:59 AM, Andreas Färber wrote: >> Am 10.04.2012 22:04, schrieb Meador Inge: >>> commit f7aa558396dd0f6b7a2b22c05cb503c655854102 pulled the dcache and icache >>> line size initialization inside of a '#if !defined(CONFIG_USER_ONLY)' block. >>> This is not correct because instructions like 'dcbz' need the dcache size >>> initialized even for user mode. >>> >>> Signed-off-by: Meador Inge <meadori@codesourcery.com> >> >> Looks okay and compiles, >> >> Reviewed-by: Andreas Färber <afaerber@suse.de> >> >> Scott, are you planning to review this e500 patch? Or should I go ahead >> and apply? > > I'm OK with it, though it may make more sense for USER_ONLY to just pick > an arbitrary cache line size (probably 32) than to try to imitate a > specific core. Meador, what was your specific problem: The value being zero or not matching real hardware? Scott's suggestion would avoid some #ifdef'ery so I'd prefer that if possible. I'm planning for a PULL later today, so let me know. Andreas
On 04/13/2012 06:40 AM, Andreas Färber wrote: > Am 12.04.2012 19:24, schrieb Scott Wood: >> On 04/12/2012 11:59 AM, Andreas Färber wrote: >>> Am 10.04.2012 22:04, schrieb Meador Inge: >>>> commit f7aa558396dd0f6b7a2b22c05cb503c655854102 pulled the dcache and icache >>>> line size initialization inside of a '#if !defined(CONFIG_USER_ONLY)' block. >>>> This is not correct because instructions like 'dcbz' need the dcache size >>>> initialized even for user mode. >>>> >>>> Signed-off-by: Meador Inge <meadori@codesourcery.com> >>> >>> Looks okay and compiles, >>> >>> Reviewed-by: Andreas Färber <afaerber@suse.de> >>> >>> Scott, are you planning to review this e500 patch? Or should I go ahead >>> and apply? >> >> I'm OK with it, though it may make more sense for USER_ONLY to just pick >> an arbitrary cache line size (probably 32) than to try to imitate a >> specific core. > > Meador, what was your specific problem: The value being zero or not > matching real hardware? My specific problem was the value being zero. Therefore nothing actually got cleared because the 'dcbz' implementation uses 'dcache_line_size' as an upper bound on a loop. On a side note, the way I actually ran into this was by running a application linked against an optimized version of glibc for e500 that uses 'dcbz' in 'memset'. ld.so wasn't correctly clearing .bss as a result. That was fun to debug :-) > Scott's suggestion would avoid some #ifdef'ery so I'd prefer that if > possible. I'm planning for a PULL later today, so let me know. Sounds good to me. I think the #ifdef stuff is gross, but I wasn't sure of way around it.
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index b1f8785..ef8735a 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -4466,25 +4466,32 @@ static void init_proc_e500 (CPUPPCState *env, int version) env->nb_pids = 3; env->nb_ways = 2; env->id_tlbs = 0; +#endif switch (version) { case fsl_e500v1: /* e500v1 */ +#if !defined(CONFIG_USER_ONLY) tlbncfg[0] = gen_tlbncfg(2, 1, 1, 0, 256); tlbncfg[1] = gen_tlbncfg(16, 1, 9, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16); +#endif env->dcache_line_size = 32; env->icache_line_size = 32; break; case fsl_e500v2: /* e500v2 */ +#if !defined(CONFIG_USER_ONLY) tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512); tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16); +#endif env->dcache_line_size = 32; env->icache_line_size = 32; break; case fsl_e500mc: /* e500mc */ +#if !defined(CONFIG_USER_ONLY) tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512); tlbncfg[1] = gen_tlbncfg(64, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 64); +#endif env->dcache_line_size = 64; env->icache_line_size = 64; l1cfg0 |= 0x1000000; /* 64 byte cache block size */ @@ -4492,7 +4499,6 @@ static void init_proc_e500 (CPUPPCState *env, int version) default: cpu_abort(env, "Unknown CPU: " TARGET_FMT_lx "\n", env->spr[SPR_PVR]); } -#endif gen_spr_BookE206(env, 0x000000DF, tlbncfg); /* XXX : not implemented */ spr_register(env, SPR_HID0, "HID0",
commit f7aa558396dd0f6b7a2b22c05cb503c655854102 pulled the dcache and icache line size initialization inside of a '#if !defined(CONFIG_USER_ONLY)' block. This is not correct because instructions like 'dcbz' need the dcache size initialized even for user mode. Signed-off-by: Meador Inge <meadori@codesourcery.com> --- target-ppc/translate_init.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)