do firmware feature fixups after features are initialised
PATCH 1/1 do firmware feature fixups after features are initialised
On pSeries the firmware features are not setup until ppc_md.init_early, so we can't do the firmware feature sections fixups till after this.
Currently firmware feature sections is only used on iSeries which inits the firmware features much earlier. This is a bug in waiting on pSeries.
---
paulus: since we aren't hitting this currently, it can wait for 2.6.23.
arch/powerpc/kernel/setup_64.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
arch/powerpc/kernel/setup_64.c +8 -4
@@ -350,13 +350,11 @@ 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) +/* Apply CPUs-specific fixups to kernel text (nop out sections +* not relevant to this CPU) */ do_feature_fixups (cur_cpu_spec->cpu_features, &__start___ftr_fixup, &__stop___ftr_fixup); -do_feature_fixups (powerpc_firmware_features, - &__start___fw_ftr_fixup, &__stop___fw_ftr_fixup);/* * Unflatten the device-tree passed by prom_init or kexec
@@ -394,6 +392,12 @@ void __init setup_system(void)if (ppc_md.init_early) ppc_md.init_early (); +/* Apply firmware specific fixups to kernel text (nop out +* sections not relevant to this firmware) +*/ +do_feature_fixups (powerpc_firmware_features, + &__start___fw_ftr_fixup, &__stop___fw_ftr_fixup); +/* * We can discover serial ports now since the above did setup the * hash table management for us, thus ioremap works. We do that early
Quoted 10 lines · Michael Neuling
On pSeries the firmware features are not setup until ppc_md.init_early, so we can't do the firmware feature sections fixups till after this.
Currently firmware feature sections is only used on iSeries which inits the firmware features much earlier. This is a bug in waiting on pSeries.
Signed-off-by Michael Neuling <[email protected]>---
paulus: since we aren't hitting this currently, it can wait for 2.6.23.
This patch will cause the kernel to blow up at boot on various machines, I'm surprised we haven't hit that already.
The problem is that we can't service SLB miss on a !iseries machine if CONFIG_PPC_ISERIES is set, before the fixup occurs. (Some iseries code in there will not have been nop'ed out and SRR0 will be loaded with crap).
Thus we die when unflattening the device-tree on some machines.
There are two possibly solutions I see in the long run:
- We could set the FW features earlier on pseries, though that is a bit
annoying because that means doing it before the device-tree is unflattened.
- We could constraint lmb_alloc to the first segment until the FW fixup
occurs, either within lmb_alloc itself, or fixup the callers such as unflatten_device_tree, to pass an explicit limit.
What do you think ?
Ben.
Quoted 11 lines
There are two possibly solutions I see in the long run:
- We could set the FW features earlier on pseries, though that is
a bit
annoying because that means doing it before the device-tree is unflattened.- We could constraint
lmb_allocto the first segment until the FW
fixup
occurs, either withinlmb_allocitself, or fixup the callers such asunflatten_device_tree, to pass an explicit limit.
c) use a different SLB handler during early boot;
d) preload some SLB entries at very early boot to cover the first 4GB or so.
d) sounds nice and simple, but will it work?
Segher
Quoted 12 lines · Segher Boessenkool
Quoted 11 lines
There are two possibly solutions I see in the long run:
- We could set the FW features earlier on pseries, though that is
a bit
annoying because that means doing it before the device-tree is unflattened.- We could constraint
lmb_allocto the first segment until the FW
fixup
occurs, either withinlmb_allocitself, or fixup the callers such asunflatten_device_tree, to pass an explicit limit.
Quoted 5 lines
c) use a different SLB handler during early boot;
d) preload some SLB entries at very early boot to cover the first 4GB or so.d) sounds nice and simple, but will it work?
Nah, best to limit everything at boot to SLB 0. Thing is, 4G is not enough, you may have more RAM and have allocations from the top.
Ben.
Quoted 7 lines
Quoted 4 lines
d) preload some SLB entries at very early boot to cover the first 4GB or so.
d) sounds nice and simple, but will it work?
Nah, best to limit everything at boot to SLB 0. Thing is, 4G is not enough, you may have more RAM and have allocations from the top.
Yeah fair enough. You could of course preload the stuff at the top then, but that could quickly become a nasty hack, too.
Segher
Quoted 11 lines · Benjamin Herrenschmidt
There are two possibly solutions I see in the long run:
=20
=A0- We could set the FW features earlier on pseries, though that is a bit annoying because that means doing it before the device-tree is unflattened.
=20
=A0- We could constraintlmb_allocto the first segment until the FW fixup occurs, either withinlmb_allocitself, or fixup the callers such asunflatten_device_tree, to pass an explicit limit.
=20
What do you think ?
If I'm understanding this right, the first solution should be something along the lines of the patch below (not tested), which even removes more lines than it adds. It doesn't seem that annoying to me, and it makes sense to assume that the fw_features are set up after returning from the ppc_md probe.
Arnd <><
Index: linux-2.6/arch/powerpc/platforms/pseries/setup.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
=2D-- linux-2.6.orig/arch/powerpc/platforms/pseries/setup.c
+++ linux-2.6/arch/powerpc/platforms/pseries/setup.c
@@ -327,8 +327,6 @@ static void __init pSeries_init_early(vo
{
DBG(" -> pSeries_init_early()\n");
=20
=2D fw_feature_init();
=2D
if (firmware_has_feature(FW_FEATURE_LPAR))
find_udbg_vterm();
=20
@@ -350,14 +348,21 @@ static int __init pSeries_probe_hypertas
const char *uname, int depth, void *data)
{
+ const char *hypertas;
+ unsigned long len;
+
if (depth !=3D 1 ||
(strcmp(uname, "rtas") !=3D 0 && strcmp(uname, "rtas@0") !=3D 0))
=2D return 0;
+ return 0;
+
+ hypertas =3D of_get_flat_dt_prop(node, "ibm,hypertas-functions", &len);
+ if (!hypertas)
+ return 1;
=20
=2D if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL) !=3D NULL)
=2D powerpc_firmware_features |=3D FW_FEATURE_LPAR;
+ powerpc_firmware_features |=3D FW_FEATURE_LPAR;
+ fw_feature_init(hypertas, len);
=20
=2D return 1;
+ return 1;
}
=20
static int __init pSeries_probe(void)
Index: linux-2.6/arch/powerpc/platforms/pseries/pseries.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
=2D-- linux-2.6.orig/arch/powerpc/platforms/pseries/pseries.h
+++ linux-2.6/arch/powerpc/platforms/pseries/pseries.h
@@ -10,7 +10,7 @@
#ifndef _PSERIES_PSERIES_H
#define _PSERIES_PSERIES_H
=20
=2Dextern void __init fw_feature_init(void);
+extern void __init fw_feature_init(const char *, unsigned long); =20
struct pt_regs;
=20
arch/powerpc/platf= renamed from arch/powerpc/platforms/pseries/firmware.c +0 -0
orms/pseries/firmware.c
index 29bf83b..b0c8dbd 100644
=2D-- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -66,24 +66,14 @@ firmware_features_table[FIRMWARE_MAX_FEATURES] =3D {
* device-tree/ibm,hypertas-functions. Ultimately this functionality may
* be moved into prom.c prom_init().
*/
=2Dvoid __init fw_feature_init(void)
+void __init fw_feature_init(const char *hypertas, unsigned long len)
{
struct device_node *dn;
const char *hypertas, *s;
=2D int len, i;
+ int i;
=20
DBG(" -> fw_feature_init()\n");
=20
=2D dn =3D of_find_node_by_path("/rtas");
=2D if (dn =3D=3D NULL) {
=2D printk(KERN_ERR "WARNING! Cannot find RTAS in device-tree!\n");
=2D goto out;
=2D }
=2D
=2D hypertas =3D of_get_property(dn, "ibm,hypertas-functions", &len);
=2D if (hypertas =3D=3D NULL)
=2D goto out;
=2D
for (s =3D hypertas; s < hypertas + len; s +=3D strlen(s) + 1) {
for (i =3D 0; i < FIRMWARE_MAX_FEATURES; i++) {
/* check value against table of strings */
Quoted 14 lines · In message <[email protected]> you wrote:
Quoted 12 lines · Benjamin Herrenschmidt
There are two possibly solutions I see in the long run:
==A0- We could set the FW features earlier on pseries, though that is a bit annoying because that means doing it before the device-tree is unflattened.
==A0- We could constraint
lmb_allocto the first segment until the FW fixup occurs, either withinlmb_allocitself, or fixup the callers such asunflatten_device_tree, to pass an explicit limit.
=
Quoted 7 lines
Quoted 1 line
What do you think ?
If I'm understanding this right, the first solution should be something along the lines of the patch below (not tested), which even removes more lines than it adds. It doesn't seem that annoying to me, and it makes sense to assume that the
fw_featuresare set up after returning from theppc_mdprobe.
I'm not sure this patch is going to work as the do_feature_fixups isn't called any earlier?
Mikey
Quoted 28 lines
Arnd <><Index:
linux-2.6/arch/powerpc/platforms/pseries/setup.c=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3Darch/powerpc/platforms/pseries/setup.c +0
-2@@ -327,8 +327,6 @@ static void __init pSeries_init_early(vo {DBG (" -> pSeries_init_early()\n" ); = -fw_feature_init (); -if (firmware_has_feature (FW_FEATURE_LPAR ))find_udbg_vterm ();=
arch/powerpc/platforms/pseries/setup.c +3
-0@@ -350,14 +348,21 @@ static int __init pSeries_probe_hypertas const char *uname, int depth, void *data) { + const char *hypertas; +unsigned long len; + if (depth !=3D 1 ||
arch/powerpc/platforms/pseries/setup.c static int __init pSeries_probe_hypertas · line 358
Quoted 10 lines
arch/powerpc/platforms/pseries/setup.c +5
-2(strcmp(uname, "rtas") !=3D 0 && strcmp(uname, "rtas@0") !=3D 0)) -return 0 ; +return 0 ; + + hypertas =3D of_get_flat_dt_prop(node, "ibm,hypertas-functions", &len); +if (!hypertas) +return 1 ; = - if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL) !=3D NULL
)
Quoted 69 lines
arch/powerpc/platforms/pseries/setup.c +3
-2- powerpc_firmware_features |=3D FW_FEATURE_LPAR; + powerpc_firmware_features |=3D FW_FEATURE_LPAR; +fw_feature_init (hypertas, len); = -return 1 ; +return 1 ; }=
static int
__initpSeries_probe(void)
Index:linux-2.6/arch/powerpc/platforms/pseries/pseries.h=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3Darch/powerpc/platforms/pseries/pseries.h +1
-1@@ -10,7 +10,7 @@#ifndef _PSERIES_PSERIES_H#define _PSERIES_PSERIES_H = -extern void __init fw_feature_init (void ); +extern void __init fw_feature_init (const char *,unsigned long ); =struct
pt_regs;
=arch/powerpc/platf= renamed from arch/powerpc/platforms/pseries/firmware.c +0
-0
orms/pseries/firmware.c
index 29bf83b..b0c8dbd 100644arch/powerpc/platforms/pseries/firmware.c +2
-12@@ -66,24 +66,14 @@ firmware_features_table[FIRMWARE_MAX_FEATURES] =3D {* device-tree/ibm,hypertas-functions. Ultimately this functionality may * be moved into prom.c prom_init(). */ -void __init fw_feature_init (void ) +void __init fw_feature_init (const char *hypertas,unsigned long len) {struct device_node *dn;const char *hypertas, *s; -int len, i; +int i; =DBG (" -> fw_feature_init()\n" ); = - dn =3D of_find_node_by_path("/rtas"); - if (dn =3D=3D NULL) { -printk (KERN_ERR "WARNING! Cannot find RTAS in device-tree!\n" ); - goto out; - } - - hypertas =3D of_get_property(dn, "ibm,hypertas-functions", &len); - if (hypertas =3D=3D NULL) - goto out; - for (s =3D hypertas; s < hypertas + len; s +=3D strlen(s) + 1) {arch/powerpc/platforms/pseries/firmware.c +0
-0for (i =3D 0; i < FIRMWARE_MAX_FEATURES; i++) {/* check value against table of strings */ _______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Quoted 9 lines · Michael Neuling
Quoted 6 lines
If I'm understanding this right, the first solution should be something along the lines of the patch below (not tested), which even removes more lines than it adds. It doesn't seem that annoying to me, and it makes sense to assume that the
fw_featuresare set up after returning from theppc_mdprobe.I'm not sure this patch is going to work as the
do_feature_fixupsisn't called any earlier?
Right, my patch still assumes that yours gets removed.
Arnd <><
Quoted 10 lines
Quoted 8 lines
Quoted 5 lines
If I'm understanding this right, the first solution should be something along the lines of the patch below (not tested), which even removes more lines than it adds. It doesn't seem that annoying to me, and it makes sense to assume that the
fw_featuresare set up after returning from theppc_mdprobe.I'm not sure this patch is going to work as the
do_feature_fixupsisn't called any earlier?Right, my patch still assumes that yours gets removed.
Arrh, cool.. that would better :-)
Mikey
PATCH 1/1 fix future firmware feature fixups function failure
In reply to Arnd BergmannMove firmware feature initialisation from pSeries_init_early to the earlier pSeries_probe_hypertas so they are initialised before firmware feature fixups are applied.
Currently firmware feature sections are only used for iSeries which initialises the these features much earlier. This is a bug in waiting on pSeries.
Also adds some whitespace fixups.
---
Quoted 12 lines
Quoted 11 lines
There are two possibly solutions I see in the long run:
=20
=A0- We could set the FW features earlier on pseries, though that is a bit annoying because that means doing it before the device-tree is unflattened.
=20
=A0- We could constraintlmb_allocto the first segment until the FW fixup occurs, either withinlmb_allocitself, or fixup the callers such asunflatten_device_tree, to pass an explicit limit.
=20
What do you think ?
Quoted 5 lines
If I'm understanding this right, the first solution should be something along the lines of the patch below (not tested), which even removes more lines than it adds. It doesn't seem that annoying to me, and it makes sense to assume that the
fw_featuresare set up after returning from theppc_mdprobe.
I've cleaned this up and got it booting. Seems to be doing the right things as I can see fw feature sections being correctly NOPed out now after boot.
I've booted with pseries_defconfig and ppc64_defconfig.
Arnd, I've added a signed off by me, but it but this probably needs an explicit one from you also before it heads up.
Mikey
arch/powerpc/platforms/pseries/firmware.c | 19 +++---------------- arch/powerpc/platforms/pseries/pseries.h | 2 +- arch/powerpc/platforms/pseries/setup.c | 17 +++++++++++------ 3 files changed, 15 insertions(+), 23 deletions(-)
arch/powerpc/platforms/pseries/firmware.c +3 -16
@@ -66,24 +66,13 @@ firmware_features_table[FIRMWARE_MAX_FEA* device-tree/ibm,hypertas-functions. Ultimately this functionality may * be moved into prom.c prom_init(). */ -void __init fw_feature_init (void ) +void __init fw_feature_init (const char *hypertas,unsigned long len) { -struct device_node *dn; -const char *hypertas, *s; -int len, i; +const char *s; +int i;DBG (" -> fw_feature_init()\n" ); - dn =of_find_node_by_path ("/rtas" ); -if (dn ==NULL ) { -printk (KERN_ERR "WARNING! Cannot find RTAS in device-tree!\n" ); - goto out; - } - - hypertas =of_get_property (dn,"ibm,hypertas-functions" , &len); -if (hypertas ==NULL ) - goto out; -for (s = hypertas; s < hypertas + len; s +=strlen (s) +1 ) {for (i =0 ; i <FIRMWARE_MAX_FEATURES ; i++) {/* check value against table of strings */
@@ -98,7 +87,5 @@ void __init fw_feature_init(void) } } -out: -of_node_put (dn);DBG (" <- fw_feature_init()\n" ); }
arch/powerpc/platforms/pseries/pseries.h +1 -1
@@ -10,7 +10,7 @@#ifndef _PSERIES_PSERIES_H#define _PSERIES_PSERIES_H -extern void __init fw_feature_init (void ); +extern void __init fw_feature_init (const char *hypertas,unsigned long len);struct pt_regs ;
arch/powerpc/platforms/pseries/setup.c +11 -6
@@ -320,8 +320,6 @@ static void __init pSeries_init_early(vo {DBG (" -> pSeries_init_early()\n" ); -fw_feature_init (); -if (firmware_has_feature (FW_FEATURE_LPAR ))find_udbg_vterm ();
@@ -343,14 +341,21 @@ static int __init pSeries_probe_hypertas const char *uname, int depth, void *data) { + const char *hypertas; +unsigned long len; +if (depth !=1 || (strcmp (uname,"rtas" ) !=0 &&strcmp (uname,"rtas@0" ) !=0 )) - return 0; +return 0 ; + + hypertas =of_get_flat_dt_prop (node,"ibm,hypertas-functions" , &len); +if (!hypertas) +return 1 ; - if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL) != NULL) - powerpc_firmware_features |= FW_FEATURE_LPAR; + powerpc_firmware_features |=FW_FEATURE_LPAR ; +fw_feature_init (hypertas, len); -return 1 ; +return 1 ; }static int __init pSeries_probe (void )
Re: [PATCH] fix future firmware feature fixups function failure
Quoted 11 lines · Michael Neuling
Move firmware feature initialisation from
pSeries_init_earlyto the earlierpSeries_probe_hypertasso they are initialised before firmware feature fixups are applied.
=20
Currently firmware feature sections are only used for iSeries which initialises the these features much earlier. =A0This is a bug in waiting on pSeries.
=20
Also adds some whitespace fixups.
=20Signed-off-by Michael Neuling <[email protected]>
Haven't tested it myself, but it certainly looks good to me. It does require reverting your previous patch though, are you submitting the reversal patch as well?
Arnd <><
Re: [PATCH] fix future firmware feature fixups function failure
Quoted 13 lines · Michael Neuling
Quoted 11 lines
Move firmware feature initialisation from
pSeries_init_earlyto the earlierpSeries_probe_hypertasso they are initialised before firmware feature fixups are applied.
=20
Currently firmware feature sections are only used for iSeries which initialises the these features much earlier. =A0This is a bug in waiting on pSeries.
=20
Also adds some whitespace fixups.
=20Signed-off-by Michael Neuling <[email protected]>
Quoted 5 lines
Acked-by Arnd Bergmann <[email protected]>Haven't tested it myself, but it certainly looks good to me. It does require reverting your previous patch though, are you submitting the reversal patch as well?
Yep, this got reverted yesterday in826ea8f22cf612d534f33c492c98f7895043bfd1
Mikey