[PATCH] Ignore memory listed in PS3 device tree

[PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

The old 2.6.16 kernel handled memory statically and it was shown in the
device tree. The current kernel deals with the hypervisor and hotplugs
the memory, so we should just ignore anything that's reported in the
device-tree. This enables the current kernel to be booted from 2.6.16
without evil hacks in head_64.S to override the device-tree.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 08bd255..dd38408 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -52,6 +52,7 @@
 #include <asm/pSeries_reconfig.h>
 #include <asm/pci-bridge.h>
 #include <asm/kexec.h>
+#include <asm/firmware.h>
 
 #ifdef DEBUG
 #define DBG(fmt...) printk(KERN_ERR fmt)
@@ -862,6 +863,9 @@ static int __init early_init_dt_scan_memory(unsigned long node,
 	cell_t *reg, *endp;
 	unsigned long l;
 
+	if (firmware_has_feature(FW_FEATURE_PS3_LV1))
+		return 0;
+
 	/* Look for the ibm,dynamic-reconfiguration-memory node */
 	if (depth == 1 &&
 	    strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0)

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

On Thu, 2007-03-29 at 23:43 +0100, David Woodhouse wrote:
> The old 2.6.16 kernel handled memory statically and it was shown in the
> device tree. The current kernel deals with the hypervisor and hotplugs
> the memory, so we should just ignore anything that's reported in the
> device-tree. This enables the current kernel to be booted from 2.6.16
> without evil hacks in head_64.S to override the device-tree.
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>

How can this work ? Memory is parsed from early_init_devtree() which is
called before the platform probe() routines which is the one that sets
FW_FEATURE_PS3_LV1... 

It's really annoying that they didn't get their device-tree right in the
first place. They should put the RMO in there and nothing else I
suppose.

Ben.

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Fri, 2007-03-30 at 09:23 +1000, Benjamin Herrenschmidt wrote:
> How can this work ? Memory is parsed from early_init_devtree() which is
> called before the platform probe() routines which is the one that sets
> FW_FEATURE_PS3_LV1...  

Ah, good point. I was only testing on a PS3-only kernel -- I hadn't yet
graduated to a multiplatform kernel, because there were other problems
there (check_legacy_ioport).

At your suggestion, how about this approach instead...

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

--- linux-2.6.20.ppc64/include/asm-powerpc/lmb.h~	2007-02-04 18:44:54.000000000 +0000
+++ linux-2.6.20.ppc64/include/asm-powerpc/lmb.h	2007-03-30 01:13:45.000000000 +0100
@@ -51,6 +51,7 @@ extern unsigned long __init __lmb_alloc_
 extern unsigned long __init lmb_phys_mem_size(void);
 extern unsigned long __init lmb_end_of_DRAM(void);
 extern void __init lmb_enforce_memory_limit(unsigned long memory_limit);
+void __init lmb_reset_available(void);
 
 extern void lmb_dump_all(void);
 
--- linux-2.6.20.ppc64/arch/powerpc/mm/lmb.c~	2007-02-04 18:44:54.000000000 +0000
+++ linux-2.6.20.ppc64/arch/powerpc/mm/lmb.c	2007-03-30 01:12:17.000000000 +0100
@@ -338,3 +338,11 @@ void __init lmb_enforce_memory_limit(uns
 		}
 	}
 }
+
+void __init lmb_reset_available(void)
+{
+	lmb.memory.region[0].base = 0;
+	lmb.memory.region[0].size = 0;
+	lmb.memory.cnt = 1;
+	lmb.memory.size = 0;
+}
--- linux-2.6.20.ppc64/arch/powerpc/platforms/ps3/mm.c~	2007-03-29 17:48:27.000000000 +0100
+++ linux-2.6.20.ppc64/arch/powerpc/platforms/ps3/mm.c	2007-03-30 01:24:04.000000000 +0100
@@ -796,6 +796,9 @@ void __init ps3_mm_init(void)
 
 	DBG(" -> %s:%d\n", __func__, __LINE__);
 
+	/* If an old-style device-tree registered memory, forget it */
+	lmb_reset_available();
+
 	result = ps3_repository_read_mm_info(&map.rm.base, &map.rm.size,
 		&map.total);
 

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Segher Boessenkool]

> The old 2.6.16 kernel handled memory statically and it was shown in the
> device tree. The current kernel deals with the hypervisor and hotplugs
> the memory, so we should just ignore anything that's reported in the
> device-tree. This enables the current kernel to be booted from 2.6.16
> without evil hacks in head_64.S to override the device-tree.

Surely when you are booted you already have some
memory plugged for you -- shouldn't that memory be
in the device tree?


Segher

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Geoff Levand]

David Woodhouse wrote:
> The old 2.6.16 kernel handled memory statically and it was shown in the
> device tree. The current kernel deals with the hypervisor and hotplugs
> the memory, so we should just ignore anything that's reported in the
> device-tree. This enables the current kernel to be booted from 2.6.16
> without evil hacks in head_64.S to override the device-tree.

The proper way is to parse out just the command line from the incompatible
DT, and use that with the new DT supplied by the bootwrapper.  I planed
to do that in my bootwrapper, but haven't coded it yet.

-Geoff

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Fri, 2007-03-30 at 10:41 -0700, Geoff Levand wrote:
> The proper way is to parse out just the command line from the incompatible
> DT, and use that with the new DT supplied by the bootwrapper.  I planed
> to do that in my bootwrapper, but haven't coded it yet. 

There is no bootwrapper here. We boot a plain vmlinux, which needs to
cope. If it needs fixing up beforehand, it needs to be done in kexec.

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Segher Boessenkool]

>> The proper way is to parse out just the command line from the 
>> incompatible
>> DT, and use that with the new DT supplied by the bootwrapper.  I 
>> planed
>> to do that in my bootwrapper, but haven't coded it yet.
>
> There is no bootwrapper here. We boot a plain vmlinux, which needs to
> cope.

Maybe you should use a bootwrapper, instead.

It doesn't make sense to duplicate lots of firmware
and bootloader/bootwrapper functionality in the kernel
just for kexec.  (Or to do that at all, really).


Segher

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Fri, 2007-03-30 at 20:11 +0200, Segher Boessenkool wrote:
> Maybe you should use a bootwrapper, instead.
> 
> It doesn't make sense to duplicate lots of firmware
> and bootloader/bootwrapper functionality in the kernel
> just for kexec.  (Or to do that at all, really).

kexec has a --devicetreeblob option, if we need to resort to that.
And we're not talking about "lots of functionality". It's just a case of
ignoring the memory which is mentioned in the device-tree.

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Segher Boessenkool]

>> Maybe you should use a bootwrapper, instead.
>>
>> It doesn't make sense to duplicate lots of firmware
>> and bootloader/bootwrapper functionality in the kernel
>> just for kexec.  (Or to do that at all, really).
>
> kexec has a --devicetreeblob option, if we need to resort to that.

That would seem a good option, yes.

> And we're not talking about "lots of functionality". It's just a case 
> of
> ignoring the memory which is mentioned in the device-tree.

Yes, every single little addition isn't very big, sure.
It all accumulates though and at some point it will be
too much.  My tolerances are lower than yours I guess ;-)


Segher

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

On Sat, 2007-03-31 at 17:27 +0200, Segher Boessenkool wrote:

> Yes, every single little addition isn't very big, sure.
> It all accumulates though and at some point it will be
> too much.  My tolerances are lower than yours I guess ;-)

In that specific case, I beleive it's fair enough to do the fixup in the
kernel for a couple of reasons. One is that can be kept fairly self
contained in the ps3 code (by resetting the LMB available ranges) and
the other is that not doing so would make the existing bootloader unable
to boot new kernels, which is annoying.

Ben.

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Sun, 2007-04-01 at 08:34 +1000, Benjamin Herrenschmidt wrote:
> In that specific case, I beleive it's fair enough to do the fixup in
> the kernel for a couple of reasons. One is that can be kept fairly
> self contained in the ps3 code (by resetting the LMB available ranges)
> and the other is that not doing so would make the existing bootloader
> unable to boot new kernels, which is annoying.

Almost as annoying as the fact that a _new_ bootloader built with the
current kernel isn't actually able to boot anything at all :)

SLOF, anyone? :)

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On Fri, 2007-03-30 at 15:25 +0200, Segher Boessenkool wrote:
> > The old 2.6.16 kernel handled memory statically and it was shown in the
> > device tree. The current kernel deals with the hypervisor and hotplugs
> > the memory, so we should just ignore anything that's reported in the
> > device-tree. This enables the current kernel to be booted from 2.6.16
> > without evil hacks in head_64.S to override the device-tree.
> 
> Surely when you are booted you already have some
> memory plugged for you -- shouldn't that memory be
> in the device tree?

See ps3_mm_init(), called from _probe_.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Segher Boessenkool]

>>> The old 2.6.16 kernel handled memory statically and it was shown in 
>>> the
>>> device tree. The current kernel deals with the hypervisor and 
>>> hotplugs
>>> the memory, so we should just ignore anything that's reported in the
>>> device-tree. This enables the current kernel to be booted from 2.6.16
>>> without evil hacks in head_64.S to override the device-tree.
>>
>> Surely when you are booted you already have some
>> memory plugged for you -- shouldn't that memory be
>> in the device tree?
>
> See ps3_mm_init(), called from _probe_.

Yes sure, the hypervisor has given you some memory
already, and it tells you about it.

My question remains: shouldn't Linux get that
information from the device tree, instead?  The
bootwrapper or bootloader can put it there.


Segher

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

On Mon, 2007-04-02 at 19:36 +0200, Segher Boessenkool wrote:
> >>> The old 2.6.16 kernel handled memory statically and it was shown in 
> >>> the
> >>> device tree. The current kernel deals with the hypervisor and 
> >>> hotplugs
> >>> the memory, so we should just ignore anything that's reported in the
> >>> device-tree. This enables the current kernel to be booted from 2.6.16
> >>> without evil hacks in head_64.S to override the device-tree.
> >>
> >> Surely when you are booted you already have some
> >> memory plugged for you -- shouldn't that memory be
> >> in the device tree?
> >
> > See ps3_mm_init(), called from _probe_.
> 
> Yes sure, the hypervisor has given you some memory
> already, and it tells you about it.

My point was it's not coming out of the device tree, it's just manually
lmb_add'ed.

> My question remains: shouldn't Linux get that
> information from the device tree, instead?  The
> bootwrapper or bootloader can put it there.

Yes it _should_, but it doesn't.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Tue, 2007-04-03 at 12:42 +1000, Michael Ellerman wrote:
> My point was it's not coming out of the device tree, it's just
> manually lmb_add'ed.
> 
> > My question remains: shouldn't Linux get that
> > information from the device tree, instead?  The
> > bootwrapper or bootloader can put it there.
> 
> Yes it _should_, but it doesn't.

It _used_ to. The 2.6.16 kernel just used the memory listed in the
device tree. That's _why_ we need this hack -- because when we boot from
2.6.16 with the memory listed in the device tree, it ends up getting
registered twice by the new kernel.

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Mon, 2007-04-02 at 23:16 -0400, David Woodhouse wrote:
> > > My question remains: shouldn't Linux get that
> > > information from the device tree, instead?  The
> > > bootwrapper or bootloader can put it there.
> > 
> > Yes it _should_, but it doesn't.
> 
> It _used_ to. The 2.6.16 kernel just used the memory listed in the
> device tree. That's _why_ we need this hack -- because when we boot
> from
> 2.6.16 with the memory listed in the device tree, it ends up getting
> registered twice by the new kernel. 

Why _did_ we make this change? It makes /sbin/kexec unhappy too, since
it doesn't like the apparent lack of _any_ memory.

It's not as if anyone's going to be expanding the amount of memory in
their PS3, so it really needs to be dynamically detected.

If we were to put the memory back in the device-tree, and start with a
single region up to 0x07c00000 followed by another region with the rest
of it, then we wouldn't need the rmo_top hack to /sbin/kexec either.

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Geoff Levand]

David Woodhouse wrote:
> On Mon, 2007-04-02 at 23:16 -0400, David Woodhouse wrote:
>> > > My question remains: shouldn't Linux get that
>> > > information from the device tree, instead?  The
>> > > bootwrapper or bootloader can put it there.
>> > 
>> > Yes it _should_, but it doesn't.
>> 
>> It _used_ to. The 2.6.16 kernel just used the memory listed in the
>> device tree. That's _why_ we need this hack -- because when we boot
>> from
>> 2.6.16 with the memory listed in the device tree, it ends up getting
>> registered twice by the new kernel. 
> 
> Why _did_ we make this change? It makes /sbin/kexec unhappy too, since
> it doesn't like the apparent lack of _any_ memory.
> 
> It's not as if anyone's going to be expanding the amount of memory in
> their PS3, so it really needs to be dynamically detected.
> 
> If we were to put the memory back in the device-tree, and start with a
> single region up to 0x07c00000 followed by another region with the rest
> of it, then we wouldn't need the rmo_top hack to /sbin/kexec either.

I'm in the middle of implementing the zImage and kexec support for the
2.6.21 kernel, but haven't pushed all of it out to ps3-linux.git yet.
Once that is done, there is little need for a 2.6.16 based bootloader,
since it would be just as well to use a recent kernel.

In the interim, for compatibility with the legacy 2.6.16 based bootloader,
I have the hack ps3-no-bootwrapper-hack.diff.  Ben Collins did his
hack for Ubuntu (follows) which tries to use the DT passed from
the bootloader.  I planed to add it, or similar hack to ps3-linux.git
sometime soon.  As I mentioned in another mail, the proper way is to
just grab the command line passed from the bootloader and use it with
the built-in DT.  Ben's patch does effectively the same thing.

-Geoff


From: Ben Collins <ben.collins@ubuntu.com>
[PATCH]: Use the dt passed from kboot

It's untested, but patched against ps3-linux-dev git.

diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -1642,7 +1642,7 @@ _STATIC(__start_initialization_iSeries)
 
 #if defined(CONFIG_PPC_PS3)
 _STATIC(__start_ps3_kexec)
-#if 1 /* override DT passed from bootloader */
+#if 0 /* override DT passed from bootloader */
 	LOAD_REG_IMMEDIATE(r3, dt_blob_start)
 	clrldi	r3, r3, 2 /* convert to physical */
 #endif
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -969,6 +969,8 @@ static void __init early_reserve_mem(void)
 
 void __init early_init_devtree(void *params)
 {
+	unsigned long dt_root;
+
 	DBG(" -> early_init_devtree(%p)\n", params);
 
 	/* Setup flat device-tree pointer */
@@ -988,7 +990,15 @@ void __init early_init_devtree(void *params)
 	/* Scan memory nodes and rebuild LMBs */
 	lmb_init();
 	of_scan_flat_dt(early_init_dt_scan_root, NULL);
-	of_scan_flat_dt(early_init_dt_scan_memory, NULL);
+
+	/* 
+	 * For PS3, the values in the dt were set by the boot loader
+	 * (for kexec), and we don't want to use them. Instead, use the
+	 * lv1 callbacks (done later in ps3_setup).
+	 */
+	dt_root = of_get_flat_dt_root();
+	if (!of_flat_dt_is_compatible(dt_root, "PS3"))
+		of_scan_flat_dt(early_init_dt_scan_memory, NULL);
 
 	/* Save command line for /proc/cmdline and then parse parameters */
 	strlcpy(boot_command_line, cmd_line, COMMAND_LINE_SIZE);

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Tue, 2007-04-03 at 14:18 -0700, Geoff Levand wrote:
> I'm in the middle of implementing the zImage and kexec support for the
> 2.6.21 kernel, but haven't pushed all of it out to ps3-linux.git yet.
> Once that is done, there is little need for a 2.6.16 based bootloader,
> since it would be just as well to use a recent kernel.

True, but there are people out there who _do_ have a 2.6.16-based
bootloader, and we don't really want to force them to update the
bootloader before they can update the kernel.

> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -1642,7 +1642,7 @@ _STATIC(__start_initialization_iSeries)
>  
>  #if defined(CONFIG_PPC_PS3)
>  _STATIC(__start_ps3_kexec)
> -#if 1 /* override DT passed from bootloader */
> +#if 0 /* override DT passed from bootloader */
>  	LOAD_REG_IMMEDIATE(r3, dt_blob_start)
>  	clrldi	r3, r3, 2 /* convert to physical */
>  #endif

I've been selective about which patches from ps3-linux-patches I've
included in the Fedora kernel -- I was after a _minimal_ set which
allows the machine to be functional. I haven't included the hacks in
head_64.S -- I'll probably need something like that when I come to build
the otheros.bld bootloader, but in fact that can probably live in the
zImage wrapper rather than the kernel itself. The otheros.bld is the
only time we'll be using a zImage wrapper.

> @@ -988,7 +990,15 @@ void __init early_init_devtree(void *params)
> -	of_scan_flat_dt(early_init_dt_scan_memory, NULL);
> +	if (!of_flat_dt_is_compatible(dt_root, "PS3"))
> +		of_scan_flat_dt(early_init_dt_scan_memory, NULL);

Yeah, that'll have much the same effect as my original patch, although
Ben suggested that I do it differently; hence the second version with
lmb_reset_available().

But to go back to the unanswered question.... why _did_ we change from
having the memory in the device-tree, to dynamically querying for it?

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Geoff Levand]

David Woodhouse wrote:
> On Tue, 2007-04-03 at 14:18 -0700, Geoff Levand wrote:
>> I'm in the middle of implementing the zImage and kexec support for the
>> 2.6.21 kernel, but haven't pushed all of it out to ps3-linux.git yet.
>> Once that is done, there is little need for a 2.6.16 based bootloader,
>> since it would be just as well to use a recent kernel.

> But to go back to the unanswered question.... why _did_ we change from
> having the memory in the device-tree, to dynamically querying for it?

There are several points here I feel need explanation.  One is that memory
is a dynamic resource allocated by the HV, so it needs to be managed as a
dynamic resource by the guest OS.  Another point is that the HV has two
types of memory; boot memory, and virtual memory.  Boot memory is what will
be in is in the DT when I finish the zImage implementation.  Virtual memory
is what I currently have as hotplug memory.

So there are really two answers, one is that having the boot memory in the
DT hasn't changed, I just haven't finished that part yet.  The other is that
we get the amount of memory at runtime because it is a dynamic resource, and
cannot be known at DT compile time.  It is just by some luck that the 2.6.16
kernel continues to work correctly on the PS3, but it is not guaranteed to,
and has a good chance of failing on non-ps3 systems that use the lv1 hypervisor.

-Geoff

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Arnd Bergmann]

On Wednesday 04 April 2007, Geoff Levand wrote:
> So there are really two answers, one is that having the boot memory in the
> DT hasn't changed, I just haven't finished that part yet. =A0The other is=
 that
> we get the amount of memory at runtime because it is a dynamic resource, =
and
> cannot be known at DT compile time.

But the device tree can easily be modified before entering the new kernel,
when the kboot kernel allocates all the memory it can get, it just needs
to list it in the device tree in some preallocated device node.

	Arnd <><

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Geoff Levand]

Arnd Bergmann wrote:
> On Wednesday 04 April 2007, Geoff Levand wrote:
>> So there are really two answers, one is that having the boot memory in=
 the
>> DT hasn't changed, I just haven't finished that part yet. =EF=BF=BDThe=
 other is that
>> we get the amount of memory at runtime because it is a dynamic resourc=
e, and
>> cannot be known at DT compile time.
>=20
> But the device tree can easily be modified before entering the new kern=
el,
> when the kboot kernel allocates all the memory it can get, it just need=
s
> to list it in the device tree in some preallocated device node.

Yes, but boot mem is not allocated by the guest, it is allocated by the p=
olicy
module.  The guest can only query how much boot mem has been allocated fo=
r
it.  This can be done by the bootloader, or the kernel itself, if done ea=
rly
enough.  That info needs to be added to the DT before the kernel sets up =
its
boot mem structures.  My current plan is to do it in the bootloader, sinc=
e
there is not a platform hook in the current powerpc arch code that is ear=
ly
enough.

But you seem to say we can do something in the first stage kernel and pas=
s
some info to the second stage kernel?  The trouble with that is that when
the kernel is used as a first stage, where does the info come from?  I wa=
nt
to have a single kernel that can be either programmed into flash or loade=
d
by kexec.  For that to work the DT setup must be done in the wrapper, or
the kernel itself.

-Geoff

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Tue, 2007-04-03 at 16:58 -0700, Geoff Levand wrote:
> But you seem to say we can do something in the first stage kernel and pass
> some info to the second stage kernel?  The trouble with that is that when
> the kernel is used as a first stage, where does the info come from? 

>From the bootwrapper, surely? If the bootwrapper sets up the initial
device-tree and passes it to the first stage kernel, then all that
information will be passed through when you kexec the second stage
vmlinux.

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

On Tue, 2007-04-03 at 10:45 -0400, David Woodhouse wrote:
> On Mon, 2007-04-02 at 23:16 -0400, David Woodhouse wrote:
> > > > My question remains: shouldn't Linux get that
> > > > information from the device tree, instead?  The
> > > > bootwrapper or bootloader can put it there.
> > > 
> > > Yes it _should_, but it doesn't.
> > 
> > It _used_ to. The 2.6.16 kernel just used the memory listed in the
> > device tree. That's _why_ we need this hack -- because when we boot
> > from
> > 2.6.16 with the memory listed in the device tree, it ends up getting
> > registered twice by the new kernel. 
> 
> Why _did_ we make this change? It makes /sbin/kexec unhappy too, since
> it doesn't like the apparent lack of _any_ memory.
> 
> It's not as if anyone's going to be expanding the amount of memory in
> their PS3, so it really needs to be dynamically detected.
> 
> If we were to put the memory back in the device-tree, and start with a
> single region up to 0x07c00000 followed by another region with the rest
> of it, then we wouldn't need the rmo_top hack to /sbin/kexec either.

I think the right approach on the PS3 would be to have the RMO in
the /memory node... and _maybe_ the hotplugged memory in a separate
range in there, though that is less important.

Would it be possible to modify the 2.6.16 kernel to split the /memory
range in two (or is it already doing that ?). I have no problem relying
on an updated 2.6.16 when booting off an old distro.

As far as 2.6.2x is concerned, I think we should put the RMO at least
in /memory and that can easily be done by having the ps3 platform code
overwrite the /memory node content. The other approach would be to fix
it up from the zImage wrapper, but that would require booting off zImage
unless you are coming from a sane kernel.

Ben.

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

On Tue, 2007-04-03 at 14:18 -0700, Geoff Levand wrote:

> I'm in the middle of implementing the zImage and kexec support for the
> 2.6.21 kernel, but haven't pushed all of it out to ps3-linux.git yet.
> Once that is done, there is little need for a 2.6.16 based bootloader,
> since it would be just as well to use a recent kernel.
> 
> In the interim, for compatibility with the legacy 2.6.16 based bootloader,
> I have the hack ps3-no-bootwrapper-hack.diff.  Ben Collins did his
> hack for Ubuntu (follows) which tries to use the DT passed from
> the bootloader.  I planed to add it, or similar hack to ps3-linux.git
> sometime soon.  As I mentioned in another mail, the proper way is to
> just grab the command line passed from the bootloader and use it with
> the built-in DT.  Ben's patch does effectively the same thing.

I think that "hack" should be done differently and merged because we
should really support booting off 2.6.16.

The way to do so is to reset the LMB's from ps3_mm_init() before
starting adding to them... or more specifically, reset the LMB's
available region (NOT the reserved ranages !)

We also want to "Fixup" the device tree memory node to make sure that
what is exposed to userspace contains only the RMO as the first region
in there (exposing the rest of memory in a second region or not is
unimportant).

Ben.

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

> There are several points here I feel need explanation.  One is that memory
> is a dynamic resource allocated by the HV, so it needs to be managed as a
> dynamic resource by the guest OS.

That happens on pSeries too and we still keep the device-tree in check,
but that's related to the way all hotplug operations work on p... on
these, the HV provides us with device-tree updates.

I still think that the RMO is _not_ a dynamic resource, and as such
should be present in the memory node as this is expected of quite a few
things around including kexec tools.

>   Another point is that the HV has two
> types of memory; boot memory, and virtual memory.  Boot memory is what will
> be in is in the DT when I finish the zImage implementation.  Virtual memory
> is what I currently have as hotplug memory.
> 
> So there are really two answers, one is that having the boot memory in the
> DT hasn't changed, I just haven't finished that part yet.  The other is that
> we get the amount of memory at runtime because it is a dynamic resource, and
> cannot be known at DT compile time.  It is just by some luck that the 2.6.16
> kernel continues to work correctly on the PS3, but it is not guaranteed to,
> and has a good chance of failing on non-ps3 systems that use the lv1 hypervisor.

Adding/removing memory outside of the RMO without updating the DT is
fine with me. The RMO itself should be in the DT though.

Ben.

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Segher Boessenkool]

> I want
> to have a single kernel that can be either programmed into flash or 
> loaded
> by kexec.  For that to work the DT setup must be done in the wrapper, 
> or
> the kernel itself.

Do it in the bootwrapper please.  That way, the kernel
doesn't have to special-case this for PS3; also, it more
logically belongs there than in the kernel.


Segher

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Geoff Levand]

David Woodhouse wrote:
> The old 2.6.16 kernel handled memory statically and it was shown in the
> device tree. The current kernel deals with the hypervisor and hotplugs
> the memory, so we should just ignore anything that's reported in the
> device-tree. This enables the current kernel to be booted from 2.6.16
> without evil hacks in head_64.S to override the device-tree.
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 08bd255..dd38408 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -52,6 +52,7 @@
>  #include <asm/pSeries_reconfig.h>
>  #include <asm/pci-bridge.h>
>  #include <asm/kexec.h>
> +#include <asm/firmware.h>
>  
>  #ifdef DEBUG
>  #define DBG(fmt...) printk(KERN_ERR fmt)
> @@ -862,6 +863,9 @@ static int __init early_init_dt_scan_memory(unsigned long node,
>  	cell_t *reg, *endp;
>  	unsigned long l;
>  
> +	if (firmware_has_feature(FW_FEATURE_PS3_LV1))
> +		return 0;
> +
>  	/* Look for the ibm,dynamic-reconfiguration-memory node */
>  	if (depth == 1 &&
>  	    strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0)
> 

I committed a change that pulls the command line out of the incompatible DT and
merges it with the built-in DT.

  http://git.kernel.org/?p=linux/kernel/git/geoff/ps3-linux-patches.git;a=blob;f=ps3-hacks/ps3-no-bootwrapper-hack.diff

-Geoff

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

On Fri, 2007-04-06 at 14:22 -0700, Geoff Levand wrote:

> I committed a change that pulls the command line out of the incompatible DT and
> merges it with the built-in DT.
> 
>   http://git.kernel.org/?p=linux/kernel/git/geoff/ps3-linux-patches.git;a=blob;f=ps3-hacks/ps3-no-bootwrapper-hack.diff

A bit yukcy but I suppose it'll do for now ... though this shouldn't be
kept as a long term approach unless there's some versionning or such.
You also need to pull the initrd btw.

Ben.

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Sat, 2007-04-07 at 08:34 +1000, Benjamin Herrenschmidt wrote:
> A bit yukcy but I suppose it'll do for now ... though this shouldn't
> be kept as a long term approach unless there's some versionning or
> such. You also need to pull the initrd btw. 

I really don't think we should be doing it like that in the kernel
proper. Provide a DT blob in the bootwrapper by all means, but there's
no real need to override the device-tree like that when all we really
need to do is refrain from crashing when there's memory listed in the
device-tree.

If we're going to avoid hacking kexec userspace tools, we _want_ the RMO
memory listed in the device-tree anyway. But we can fix that up for
ourselves rather than overriding the whole device-tree, surely?

-- 
dwmw2

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Geoff Levand]

David Woodhouse wrote:
> On Sat, 2007-04-07 at 08:34 +1000, Benjamin Herrenschmidt wrote:
>> A bit yukcy but I suppose it'll do for now ... though this shouldn't
>> be kept as a long term approach unless there's some versionning or
>> such. You also need to pull the initrd btw. 
> 
> I really don't think we should be doing it like that in the kernel
> proper. Provide a DT blob in the bootwrapper by all means, but there's
> no real need to override the device-tree like that when all we really
> need to do is refrain from crashing when there's memory listed in the
> device-tree.
> 
> If we're going to avoid hacking kexec userspace tools, we _want_ the RMO
> memory listed in the device-tree anyway. But we can fix that up for
> ourselves rather than overriding the whole device-tree, surely?
> 

Its just a short term hack.

-Geoff

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[Benjamin Herrenschmidt]

On Fri, 2007-04-06 at 18:43 -0400, David Woodhouse wrote:
> On Sat, 2007-04-07 at 08:34 +1000, Benjamin Herrenschmidt wrote:
> > A bit yukcy but I suppose it'll do for now ... though this shouldn't
> > be kept as a long term approach unless there's some versionning or
> > such. You also need to pull the initrd btw. 
> 
> I really don't think we should be doing it like that in the kernel
> proper. Provide a DT blob in the bootwrapper by all means, but there's
> no real need to override the device-tree like that when all we really
> need to do is refrain from crashing when there's memory listed in the
> device-tree.

That's not the only bug of the old device-tree though.

> If we're going to avoid hacking kexec userspace tools, we _want_ the RMO
> memory listed in the device-tree anyway. But we can fix that up for
> ourselves rather than overriding the whole device-tree, surely?
> 

Re: [Cbe-oss-dev] [PATCH] Ignore memory listed in PS3 device tree

[David Woodhouse]

On Sat, 2007-04-07 at 09:35 +1000, Benjamin Herrenschmidt wrote:
> That's not the only bug of the old device-tree though. 

No, but others can be fixed up later if we need to.

-- 
dwmw2