[PATCH] powerpc/6xx: set High BAT Enable flag on G2 cores

Matthias Schiffer matthias.schiffer at ew.tq-group.com
Fri Dec 22 19:48:31 AEDT 2023


On Thu, 2023-12-21 at 13:57 +0000, Christophe Leroy wrote:
> 
> 
> Le 21/12/2023 à 13:45, Matthias Schiffer a écrit :
> > MMU_FTR_USE_HIGH_BATS is set for G2-based cores (G2_LE, e300cX), but the
> > high BATs need to be enabled in HID2 to work. Add register definitions
> > and introduce a G2 variant of __setup_cpu_603.
> 
> Well spotted.
> 
> I have a mpc8321, hence e300c2. I never had the problem you had.
> 
> But ... looks like U-boot configuration has CONFIG_HID2_HBE so that's 
> set by U-boot indeed, that's the reason why I never had that problem.

I'll extend the commit message to mention that U-Boot setting in v2.


> 
> > 
> > This fixes boot on CPUs like the MPC5200B with STRICT_KERNEL_RWX enabled.
> > 
> > Fixes: e4d6654ebe6e ("powerpc/mm/32s: rework mmu_mapin_ram()")
> > Signed-off-by: Matthias Schiffer <matthias.schiffer at ew.tq-group.com>
> > ---
> >   arch/powerpc/include/asm/cpu_setup.h      |  1 +
> >   arch/powerpc/include/asm/reg.h            |  2 ++
> >   arch/powerpc/kernel/cpu_setup_6xx.S       | 25 ++++++++++++++++++++++-
> >   arch/powerpc/kernel/cpu_specs_book3s_32.h | 10 ++++-----
> >   4 files changed, 32 insertions(+), 6 deletions(-)
> > 
> > 
> > I have only tested this on the MPC5200B (G2_LE), but according to the
> > e300 manual, e300cX cores should behave the same.
> > 
> > The Fixes tag is the best I could come up with - I believe that the
> > underlying issue of setting USE_HIGH_BATS without actually enabling them
> > is as old as Linux's PowerPC implementation, but the specific code
> > causing the boot failure was added in the mentioned commit.
> 
> Agreed, before that only BATs 0 to 3 were used anyway.
> There was also BAT 4 used by platforms/embedded6xx/wii.c  , but that's 
> probably not a G2 ?
> 
> > 
> > Another issue I found in the code is that
> > arch/powerpc/platforms/52xx/lite5200_sleep.S uses the SPRN_HID2 definition
> > which does not refer to HID2 on the 5200... but that will be for someone
> > else to fix, if there is still anyone left using that platform.
> 
> Maybe file an issue for it at https://github.com/linuxppc/issues/issues 
> if you don't plan to fix it ?
> 
> By the way, it looks like the SPRN_HID2 definition we have is very 
> specific to the IBM 750.
> 

IBM 750GX even - googling for IBM 750 came up with several other cores that either don't have HID2,
or have it at a different SPR.


> MPC 750 has SPRN_HID2 as 1011 == 0x3f3 like others.

> > 
> > 
> > diff --git a/arch/powerpc/include/asm/cpu_setup.h b/arch/powerpc/include/asm/cpu_setup.h
> > index 30e2fe3895024..68d804e74d221 100644
> > --- a/arch/powerpc/include/asm/cpu_setup.h
> > +++ b/arch/powerpc/include/asm/cpu_setup.h
> > @@ -35,6 +35,7 @@ void __setup_cpu_750fx(unsigned long offset, struct cpu_spec *spec);
> >   void __setup_cpu_7400(unsigned long offset, struct cpu_spec *spec);
> >   void __setup_cpu_7410(unsigned long offset, struct cpu_spec *spec);
> >   void __setup_cpu_745x(unsigned long offset, struct cpu_spec *spec);
> > +void __setup_cpu_g2(unsigned long offset, struct cpu_spec *spec);
> > 
> >   void __setup_cpu_ppc970(unsigned long offset, struct cpu_spec *spec);
> >   void __setup_cpu_ppc970MP(unsigned long offset, struct cpu_spec *spec);
> > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> > index 4ae4ab9090a2d..f5641fcd1da85 100644
> > --- a/arch/powerpc/include/asm/reg.h
> > +++ b/arch/powerpc/include/asm/reg.h
> > @@ -617,6 +617,8 @@
> >   #endif
> >   #define SPRN_HID2      0x3F8           /* Hardware Implementation Register 2 */
> 
> Should that HID2 be renamed to SPRN_HID2_750 to avoid confusion ?

Makes sense (should the suffix be "750GX"?). I can also add a FIXME comment to lite5200_sleep.S as
part of the rename.


>
> >   #define SPRN_HID2_GEKKO        0x398           /* Gekko HID2 Register */
> > +#define SPRN_HID2_G2   0x3F3           /* G2 HID2 Register */
> > +#define  HID2_HBE_G2   (1<<18)         /* High BAT Enable (G2) */
> >   #define SPRN_IABR      0x3F2   /* Instruction Address Breakpoint Register */
> >   #define SPRN_IABR2     0x3FA           /* 83xx */
> >   #define SPRN_IBCR      0x135           /* 83xx Insn Breakpoint Control Reg */
> > diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S b/arch/powerpc/kernel/cpu_setup_6xx.S
> > index f29ce3dd6140f..c67d32e04df9c 100644
> > --- a/arch/powerpc/kernel/cpu_setup_6xx.S
> > +++ b/arch/powerpc/kernel/cpu_setup_6xx.S
> > @@ -81,6 +81,20 @@ _GLOBAL(__setup_cpu_745x)
> >          bl      setup_745x_specifics
> >          mtlr    r5
> >          blr
> > +_GLOBAL(__setup_cpu_g2)
> > +       mflr    r5
> > +BEGIN_MMU_FTR_SECTION
> > +       li      r10,0
> > +       mtspr   SPRN_SPRG_603_LRU,r10           /* init SW LRU tracking */
> > +END_MMU_FTR_SECTION_IFSET(MMU_FTR_NEED_DTLB_SW_LRU)
> 
> MMU_FTR_NEED_DTLB_SW_LRU is dedicated to e300 core. You should also 
> remove it from __setup_cpu_603

Will do, thanks.

> 
> > +
> > +BEGIN_FTR_SECTION
> > +       bl      __init_fpu_registers
> > +END_FTR_SECTION_IFCLR(CPU_FTR_FPU_UNAVAILABLE)
> > +       bl      setup_common_caches
> > +       bl      setup_g2_hid2
> > +       mtlr    r5
> > +       blr
> > 
> >   /* Enable caches for 603's, 604, 750 & 7400 */
> >   SYM_FUNC_START_LOCAL(setup_common_caches)
> > @@ -115,6 +129,16 @@ SYM_FUNC_START_LOCAL(setup_604_hid0)
> >          blr
> >   SYM_FUNC_END(setup_604_hid0)
> > 
> > +/* Enable high BATs for G2 (G2_LE, e300cX) */
> > +SYM_FUNC_START_LOCAL(setup_g2_hid2)
> > +       mfspr   r11,SPRN_HID2_G2
> > +       oris    r11,r11,HID2_HBE_G2 at h
> > +       mtspr   SPRN_HID2_G2,r11
> > +       sync
> > +       isync
> > +       blr
> > +SYM_FUNC_END(setup_g2_hid2)
> > +
> >   /* 7400 <= rev 2.7 and 7410 rev = 1.0 suffer from some
> >    * erratas we work around here.
> >    * Moto MPC710CE.pdf describes them, those are errata
> > @@ -495,4 +519,3 @@ _GLOBAL(__restore_cpu_setup)
> >          mtcr    r7
> >          blr
> >   _ASM_NOKPROBE_SYMBOL(__restore_cpu_setup)
> > -
> > diff --git a/arch/powerpc/kernel/cpu_specs_book3s_32.h b/arch/powerpc/kernel/cpu_specs_book3s_32.h
> > index 3714634d194a1..83f054fcf837c 100644
> > --- a/arch/powerpc/kernel/cpu_specs_book3s_32.h
> > +++ b/arch/powerpc/kernel/cpu_specs_book3s_32.h
> > @@ -69,7 +69,7 @@ static struct cpu_spec cpu_specs[] __initdata = {
> >                  .mmu_features           = MMU_FTR_USE_HIGH_BATS,
> >                  .icache_bsize           = 32,
> >                  .dcache_bsize           = 32,
> > -               .cpu_setup              = __setup_cpu_603,
> > +               .cpu_setup              = __setup_cpu_g2,
> >                  .machine_check          = machine_check_generic,
> >                  .platform               = "ppc603",
> >          },
> > @@ -83,7 +83,7 @@ static struct cpu_spec cpu_specs[] __initdata = {
> >                  .mmu_features           = MMU_FTR_USE_HIGH_BATS,
> >                  .icache_bsize           = 32,
> >                  .dcache_bsize           = 32,
> > -               .cpu_setup              = __setup_cpu_603,
> > +               .cpu_setup              = __setup_cpu_g2,
> >                  .machine_check          = machine_check_83xx,
> >                  .platform               = "ppc603",
> >          },
> > @@ -96,7 +96,7 @@ static struct cpu_spec cpu_specs[] __initdata = {
> >                  .mmu_features           = MMU_FTR_USE_HIGH_BATS | MMU_FTR_NEED_DTLB_SW_LRU,
> >                  .icache_bsize           = 32,
> >                  .dcache_bsize           = 32,
> > -               .cpu_setup              = __setup_cpu_603,
> > +               .cpu_setup              = __setup_cpu_g2,
> >                  .machine_check          = machine_check_83xx,
> >                  .platform               = "ppc603",
> >          },
> > @@ -109,7 +109,7 @@ static struct cpu_spec cpu_specs[] __initdata = {
> >                  .mmu_features           = MMU_FTR_USE_HIGH_BATS | MMU_FTR_NEED_DTLB_SW_LRU,
> >                  .icache_bsize           = 32,
> >                  .dcache_bsize           = 32,
> > -               .cpu_setup              = __setup_cpu_603,
> > +               .cpu_setup              = __setup_cpu_g2,
> >                  .machine_check          = machine_check_83xx,
> >                  .num_pmcs               = 4,
> >                  .platform               = "ppc603",
> > @@ -123,7 +123,7 @@ static struct cpu_spec cpu_specs[] __initdata = {
> >                  .mmu_features           = MMU_FTR_USE_HIGH_BATS | MMU_FTR_NEED_DTLB_SW_LRU,
> >                  .icache_bsize           = 32,
> >                  .dcache_bsize           = 32,
> > -               .cpu_setup              = __setup_cpu_603,
> > +               .cpu_setup              = __setup_cpu_g2,
> >                  .machine_check          = machine_check_83xx,
> >                  .num_pmcs               = 4,
> >                  .platform               = "ppc603",
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > https://www.tq-group.com/

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/


More information about the Linuxppc-dev mailing list