On Thu, 2023-12-21 at 13:57 +0000, Christophe Leroy wrote:

Le 21/12/2023 à 13:45, Matthias Schiffer a écrit :

quoted

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.

quoted

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 <redacted>
---
  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 ?

quoted

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.

quoted

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.

quoted

  #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.

quoted

+
+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@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/