[PATCH V2] powerpc/mm: Don't do debug print before we do feature fixup

[PATCH V2] powerpc/mm: Don't do debug print before we do feature fixup

[Aneesh Kumar K.V]

We use feature fixup in segment and page fault path and hence we should
not call any function that can cause either of these before we finish
feature fixup.

Calling into early debug routine can result in segment fault as updated
in https://lkml.kernel.org/r/CAOJe8K2L8D1M_yZPmyiZ11JvJ+HAS0aFHOnsqaY=xLOnQTA3AA@mail.gmail.com

Avoid the segment fault by removing the debug print. Also remove the
matching closing debug print at the end of the function.

Even though the problem existed for a long time, this became more apparent
after commit caca285e5ab4 ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code"). Before that commit we used to use feature
fixup to finalize code path related to 1T and 256MB segment.

Cc: Denis Kirjanov <kda@linux-powerpc.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from V1:
* update commit message
* update Cc: list with correct email address

 arch/powerpc/kernel/setup_64.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 8222950f820f..da98f532d160 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -466,7 +466,6 @@ static void __init initialize_cache_info(void)
  */
 void __init setup_system(void)
 {
-	DBG(" -> setup_system()\n");
 
 	/* Apply the CPUs-specific and firmware specific fixups to kernel
 	 * text (nop out sections not relevant to this CPU or this firmware)
@@ -584,7 +583,6 @@ void __init setup_system(void)
 		       (unsigned long long)PHYSICAL_START);
 	pr_info("-----------------------------------------------------\n");
 
-	DBG(" <- setup_system()\n");
 }
 
 /* This returns the limit below which memory accesses to the linear
-- 
2.7.4

Re: [PATCH V2] powerpc/mm: Don't do debug print before we do feature fixup

[Benjamin Herrenschmidt]

On Sat, 2016-06-18 at 23:57 +0530, Aneesh Kumar K.V wrote:
> We use feature fixup in segment and page fault path and hence we should
> not call any function that can cause either of these before we finish
> feature fixup.
> 
> Calling into early debug routine can result in segment fault as updated
> in https://lkml.kernel.org/r/CAOJe8K2L8D1M_yZPmyiZ11JvJ+HAS0aFHOnsqaY=xLOnQTA3AA@mail.gmail.com
> 
> Avoid the segment fault by removing the debug print. Also remove the
> matching closing debug print at the end of the function.

Please add a comment at the beginning of setup_system() explaining
the situation. IE, that until the fixups are done, not even debug
printfs can be used.

It used to work though, the fact that we used 256M segment in that
case wasn't an issue, in part because btext only really existed on
machines that didn't have 1T segments. It was only ever going to be
a segment fault, the rest was bolted (early ioremap).

Ben.

Re: [PATCH V2] powerpc/mm: Don't do debug print before we do feature fixup

[Denis Kirjanov]

On 6/19/16, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Sat, 2016-06-18 at 23:57 +0530, Aneesh Kumar K.V wrote:
>> We use feature fixup in segment and page fault path and hence we should
>> not call any function that can cause either of these before we finish
>> feature fixup.
>>
>> Calling into early debug routine can result in segment fault as updated
>> in
>> https://lkml.kernel.org/r/CAOJe8K2L8D1M_yZPmyiZ11JvJ+HAS0aFHOnsqaY=xLOnQTA3AA@mail.gmail.com
>>
>> Avoid the segment fault by removing the debug print. Also remove the
>> matching closing debug print at the end of the function.
>
> Please add a comment at the beginning of setup_system() explaining
> the situation. IE, that until the fixups are done, not even debug
> printfs can be used.
>
Right, otherwise we'll hit the same issue in the future.

> It used to work though, the fact that we used 256M segment in that
> case wasn't an issue, in part because btext only really existed on
> machines that didn't have 1T segments. It was only ever going to be
> a segment fault, the rest was bolted (early ioremap).
>
> Ben.
>
>