[PATCH] Fix pSeries identification in prom_init.c

[PATCH] Fix pSeries identification in prom_init.c

[Benjamin Herrenschmidt]

The OF trampoline code prom_init.c still needs to identify IBM pSeries
(PAPR) machines in order to run some platform specific code on them like
instanciating the TCE tables. The code doing that detection was changed
recently in 2.6.17 early stages but was done slightly incorrectly. It
should be testing for an exact match of "chrp" and it currently tests
for anything that begins with "chrp". That means it will incorrectly
match with platforms using Maple-like device-trees and have open
firmware. This fixes it by using strcmp instead of strncmp to match what
the actual platform detection code does.

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

Paul, if you are happy with that, please send upsteam ASAP.

Index: linux-work/arch/powerpc/kernel/prom_init.c
===================================================================
--- linux-work.orig/arch/powerpc/kernel/prom_init.c	2006-05-02 10:51:33.000000000 +1000
+++ linux-work/arch/powerpc/kernel/prom_init.c	2006-05-15 15:37:03.000000000 +1000
@@ -1636,7 +1636,7 @@
 			   compat, sizeof(compat)-1);
 	if (len <= 0)
 		return PLATFORM_GENERIC;
-	if (strncmp(compat, RELOC("chrp"), 4))
+	if (strcmp(compat, RELOC("chrp")))
 		return PLATFORM_GENERIC;
 
 	/* Default to pSeries. We need to know if we are running LPAR */

Re: [PATCH] Fix pSeries identification in prom_init.c

[Benjamin Herrenschmidt]

On Mon, 2006-05-15 at 15:46 +1000, Benjamin Herrenschmidt wrote:
> The OF trampoline code prom_init.c still needs to identify IBM pSeries
> (PAPR) machines in order to run some platform specific code on them like
> instanciating the TCE tables. The code doing that detection was changed
> recently in 2.6.17 early stages but was done slightly incorrectly. It
> should be testing for an exact match of "chrp" and it currently tests
> for anything that begins with "chrp". That means it will incorrectly
> match with platforms using Maple-like device-trees and have open
> firmware. This fixes it by using strcmp instead of strncmp to match what
> the actual platform detection code does.

Michael, I noticed you changed strcmp to strncmp, any reason why you did
that ?

> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> Paul, if you are happy with that, please send upsteam ASAP.
> 
> Index: linux-work/arch/powerpc/kernel/prom_init.c
> ===================================================================
> --- linux-work.orig/arch/powerpc/kernel/prom_init.c	2006-05-02 10:51:33.000000000 +1000
> +++ linux-work/arch/powerpc/kernel/prom_init.c	2006-05-15 15:37:03.000000000 +1000
> @@ -1636,7 +1636,7 @@
>  			   compat, sizeof(compat)-1);
>  	if (len <= 0)
>  		return PLATFORM_GENERIC;
> -	if (strncmp(compat, RELOC("chrp"), 4))
> +	if (strcmp(compat, RELOC("chrp")))
>  		return PLATFORM_GENERIC;
>  
>  	/* Default to pSeries. We need to know if we are running LPAR */
> 

Re: [PATCH] Fix pSeries identification in prom_init.c

[Michael Neuling]

> > The OF trampoline code prom_init.c still needs to identify IBM
> > pSeries (PAPR) machines in order to run some platform specific code
> > on them like instanciating the TCE tables. The code doing that
> > detection was changed recently in 2.6.17 early stages but was done
> > slightly incorrectly. It should be testing for an exact match of
> > "chrp" and it currently tests for anything that begins with
> > "chrp". That means it will incorrectly match with platforms using
> > Maple-like device-trees and have open firmware. This fixes it by
> > using strcmp instead of strncmp to match what the actual platform
> > detection code does.
> 
> Michael, I noticed you changed strcmp to strncmp, any reason why you
> did that ?

To be safe if we are returned a non terminated string.  I'd not realised
the case you've mentioned.

How much we should trust firmware?  With strcpy, should we explicitly
terminate the string first (I removed one of these originally)?  Patch
below, compiled not run.

-

The OF trampoline code prom_init.c still needs to identify IBM pSeries
(PAPR) machines in order to run some platform specific code on them like
instantiating the TCE tables. The code doing that detection was changed
recently in 2.6.17 early stages but was done slightly incorrectly. It
should be testing for an exact match of "chrp" and it currently tests
for anything that begins with "chrp". That means it will incorrectly
match with platforms using Maple-like device-trees and have open
firmware. This fixes it by using strcmp instead of strncmp to match what
the actual platform detection code does.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Michael Neuling <mikey@neuling.org>
---

 arch/powerpc/kernel/prom_init.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6-powerpc/arch/powerpc/kernel/prom_init.c
===================================================================
--- linux-2.6-powerpc.orig/arch/powerpc/kernel/prom_init.c
+++ linux-2.6-powerpc/arch/powerpc/kernel/prom_init.c
@@ -1636,7 +1636,8 @@ static int __init prom_find_machine_type
 			   compat, sizeof(compat)-1);
 	if (len <= 0)
 		return PLATFORM_GENERIC;
-	if (strncmp(compat, RELOC("chrp"), 4))
+	compat[len] = 0;
+	if (strcmp(compat, RELOC("chrp")))
 		return PLATFORM_GENERIC;
 
 	/* Default to pSeries. We need to know if we are running LPAR */

Re: [PATCH] Fix pSeries identification in prom_init.c

[Benjamin Herrenschmidt]

> To be safe if we are returned a non terminated string.  I'd not realised
> the case you've mentioned.
> 
> How much we should trust firmware?  With strcpy, should we explicitly
> terminate the string first (I removed one of these originally)?  Patch
> below, compiled not run.

I wouldn't bother. If it returns a non-terminated string there, a lot of
stuff will break anyway including the kernel probe code

Ben.

> The OF trampoline code prom_init.c still needs to identify IBM pSeries
> (PAPR) machines in order to run some platform specific code on them like
> instantiating the TCE tables. The code doing that detection was changed
> recently in 2.6.17 early stages but was done slightly incorrectly. It
> should be testing for an exact match of "chrp" and it currently tests
> for anything that begins with "chrp". That means it will incorrectly
> match with platforms using Maple-like device-trees and have open
> firmware. This fixes it by using strcmp instead of strncmp to match what
> the actual platform detection code does.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> 
>  arch/powerpc/kernel/prom_init.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6-powerpc/arch/powerpc/kernel/prom_init.c
> ===================================================================
> --- linux-2.6-powerpc.orig/arch/powerpc/kernel/prom_init.c
> +++ linux-2.6-powerpc/arch/powerpc/kernel/prom_init.c
> @@ -1636,7 +1636,8 @@ static int __init prom_find_machine_type
>  			   compat, sizeof(compat)-1);
>  	if (len <= 0)
>  		return PLATFORM_GENERIC;
> -	if (strncmp(compat, RELOC("chrp"), 4))
> +	compat[len] = 0;
> +	if (strcmp(compat, RELOC("chrp")))
>  		return PLATFORM_GENERIC;
>  
>  	/* Default to pSeries. We need to know if we are running LPAR */
> 

Re: [PATCH] Fix pSeries identification in prom_init.c

[segher]

> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> Paul, if you are happy with that, please send upsteam ASAP.
>
> Index: linux-work/arch/powerpc/kernel/prom_init.c
> =================================================================== ---
> linux-work.orig/arch/powerpc/kernel/prom_init.c	2006-05-02
> 10:51:33.000000000 +1000 +++
> linux-work/arch/powerpc/kernel/prom_init.c	2006-05-15 15:37:03.000000000
> +1000 @@ -1636,7 +1636,7 @@
>  			   compat, sizeof(compat)-1);
>  	if (len <= 0)
>  		return PLATFORM_GENERIC;
> -	if (strncmp(compat, RELOC("chrp"), 4))
> +	if (strcmp(compat, RELOC("chrp")))
>  		return PLATFORM_GENERIC;
>
>  	/* Default to pSeries. We need to know if we are running LPAR */

Confirmed it works correctly on affected systems.  Thanks Ben.

Signed-off-by: Segher Boessenkool <segher@kernel.crashing.org>