diff mbox

powerpc: Fix crash during static key init on ppc32

Message ID 1470814054.3015.98.camel@kernel.crashing.org (mailing list archive)
State Accepted
Headers show

Commit Message

Benjamin Herrenschmidt Aug. 10, 2016, 7:27 a.m. UTC
We cannot do those initializations from apply_feature_fixups() as
this function runs in a very restricted environment in 32-bit where
the kernel isn't running at its linked address and the PTRRELOC()
macro must be used for any global accesss.

Instead, split them into a separtate steup_feature_keys() function
which is called in a more suitable spot on ppc32.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Comments

Christian Kujau Aug. 10, 2016, 8:17 a.m. UTC | #1
On Wed, 10 Aug 2016, Benjamin Herrenschmidt wrote:
> We cannot do those initializations from apply_feature_fixups() as
> this function runs in a very restricted environment in 32-bit where
> the kernel isn't running at its linked address and the PTRRELOC()
> macro must be used for any global accesss.
> 
> Instead, split them into a separtate steup_feature_keys() function
> which is called in a more suitable spot on ppc32.

Wow, cool. With that applied (on top of mainline from some minutes ago), 
this PowerPC G4 boots again. Thanks!

  Tested-by: Christian Kujau <lists@nerdbynature.de>

@Michael: thanks for the git-bisect tutorial, although I'm glad that I was 
able to skip it this time.

Christian.

> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
> index 57fec8a..ddf54f5 100644
> --- a/arch/powerpc/include/asm/feature-fixups.h
> +++ b/arch/powerpc/include/asm/feature-fixups.h
> @@ -186,6 +186,7 @@ label##3:					       	\
>  
>  #ifndef __ASSEMBLY__
>  void apply_feature_fixups(void);
> +void setup_feature_keys(void);
>  #endif
>  
>  #endif /* __ASM_POWERPC_FEATURE_FIXUPS_H */
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index c3e861d..9a2ae96 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -102,6 +102,9 @@ extern unsigned int memset_nocache_branch; /* Insn to be replaced by NOP */
>  
>  notrace void __init machine_init(u64 dt_ptr)
>  {
> +	/* Configure static keys first */
> +	setup_feature_keys();
> +
>  	/* Enable early debugging if any specified (see udbg.h) */
>  	udbg_early_init();
>  
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index eafb9a7..7ac8e6e 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -300,6 +300,7 @@ void __init early_setup(unsigned long dt_ptr)
>  
>  	/* Apply all the dynamic patching */
>  	apply_feature_fixups();
> +	setup_feature_keys();
>  
>  	/* Initialize the hash table or TLB handling */
>  	early_init_mmu();
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
> index 74145f0..043415f 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -188,7 +188,10 @@ void __init apply_feature_fixups(void)
>  			  &__start___fw_ftr_fixup, &__stop___fw_ftr_fixup);
>  #endif
>  	do_final_fixups();
> +}
>  
> +void __init setup_feature_keys(void)
> +{
>  	/*
>  	 * Initialise jump label. This causes all the cpu/mmu_has_feature()
>  	 * checks to take on their correct polarity based on the current set of
>
Gabriel Paubert Aug. 10, 2016, 8:52 a.m. UTC | #2
On Wed, Aug 10, 2016 at 01:17:55AM -0700, Christian Kujau wrote:
> On Wed, 10 Aug 2016, Benjamin Herrenschmidt wrote:
> > We cannot do those initializations from apply_feature_fixups() as
> > this function runs in a very restricted environment in 32-bit where
> > the kernel isn't running at its linked address and the PTRRELOC()
> > macro must be used for any global accesss.
> > 
> > Instead, split them into a separtate steup_feature_keys() function
> > which is called in a more suitable spot on ppc32.
> 
> Wow, cool. With that applied (on top of mainline from some minutes ago), 
> this PowerPC G4 boots again. Thanks!

Just a question, does sleep work on your PowerBook? 

It has been broken for several releases of the kernel on mine, but I've not
had time to investigate it and it might also be a distro/systemd issue.

    Regards,
    Gabriel
Christian Kujau Aug. 11, 2016, 12:25 a.m. UTC | #3
On Wed, 10 Aug 2016, Gabriel Paubert wrote:
> Just a question, does sleep work on your PowerBook? 

Oh, this PowerBook is on 24/7, it's a small home server and I don't think 
I ever used any sleep or hibernation modes on this machine. Sorry :\

C.
Michael Ellerman Aug. 11, 2016, 11:16 a.m. UTC | #4
On Wed, 2016-10-08 at 07:27:34 UTC, Benjamin Herrenschmidt wrote:
> We cannot do those initializations from apply_feature_fixups() as
> this function runs in a very restricted environment in 32-bit where
> the kernel isn't running at its linked address and the PTRRELOC()
> macro must be used for any global accesss.
> 
> Instead, split them into a separtate steup_feature_keys() function
> which is called in a more suitable spot on ppc32.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/97f6e0cc35026a2a09147a6da6

cheers
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index 57fec8a..ddf54f5 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -186,6 +186,7 @@  label##3:					       	\
 
 #ifndef __ASSEMBLY__
 void apply_feature_fixups(void);
+void setup_feature_keys(void);
 #endif
 
 #endif /* __ASM_POWERPC_FEATURE_FIXUPS_H */
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index c3e861d..9a2ae96 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -102,6 +102,9 @@  extern unsigned int memset_nocache_branch; /* Insn to be replaced by NOP */
 
 notrace void __init machine_init(u64 dt_ptr)
 {
+	/* Configure static keys first */
+	setup_feature_keys();
+
 	/* Enable early debugging if any specified (see udbg.h) */
 	udbg_early_init();
 
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index eafb9a7..7ac8e6e 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -300,6 +300,7 @@  void __init early_setup(unsigned long dt_ptr)
 
 	/* Apply all the dynamic patching */
 	apply_feature_fixups();
+	setup_feature_keys();
 
 	/* Initialize the hash table or TLB handling */
 	early_init_mmu();
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 74145f0..043415f 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -188,7 +188,10 @@  void __init apply_feature_fixups(void)
 			  &__start___fw_ftr_fixup, &__stop___fw_ftr_fixup);
 #endif
 	do_final_fixups();
+}
 
+void __init setup_feature_keys(void)
+{
 	/*
 	 * Initialise jump label. This causes all the cpu/mmu_has_feature()
 	 * checks to take on their correct polarity based on the current set of