[patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jimi Xenidis]

A devtree compiler (dtc) generated devtree blob is "relocatable" and  
so does not contain a reserved_map entry for the blob itself.
This means that if passed to Linux, Linux will not get lmb_reserve()  
the blob and it could be over.
The following patch will explicitly reserve the "blob" as it was  
given to us and stops prom_init.c from creating a reserved mapping  
for the blob.

NOTE: that the dtc must also not generate the blob reservation  
entry.  Should we try to detect this redundant entry? Should we bump  
the DT version number?


Signed-off-by:  <jimix@watson.ibm.com>
--
diff -r eb0990a251a9 arch/powerpc/kernel/prom.c
--- a/arch/powerpc/kernel/prom.c	Thu Mar 30 22:05:40 2006 -0500
+++ b/arch/powerpc/kernel/prom.c	Wed Apr 12 22:00:27 2006 -0400
@@ -1132,6 +1132,11 @@ static void __init early_reserve_mem(voi
	reserve_map = (u64 *)(((unsigned long)initial_boot_params) +
					initial_boot_params->off_mem_rsvmap);
+
+	/* before we do anything, lets reserve the dt blob */
+	lmb_reserve(__pa((unsigned long)initial_boot_params),
+		    initial_boot_params->totalsize);
+
#ifdef CONFIG_PPC32
	/*
	 * Handle the case where we might be booting from an old kexec
diff -r eb0990a251a9 arch/powerpc/kernel/prom_init.c
--- a/arch/powerpc/kernel/prom_init.c	Thu Mar 30 22:05:40 2006 -0500
+++ b/arch/powerpc/kernel/prom_init.c	Wed Apr 12 22:00:27 2006 -0400
@@ -1909,11 +1909,7 @@ static void __init flatten_device_tree(v
	/* Version 16 is not backward compatible */
	hdr->last_comp_version = 0x10;
-	/* Reserve the whole thing and copy the reserve map in, we
-	 * also bump mem_reserve_cnt to cause further reservations to
-	 * fail since it's too late.
-	 */
-	reserve_mem(RELOC(dt_header_start), hdr->totalsize);
+	/* Copy the reserve map in */
	memcpy(rsvmap, RELOC(mem_reserve_map), sizeof(mem_reserve_map));
#ifdef DEBUG_PROM
@@ -1926,6 +1922,9 @@ static void __init flatten_device_tree(v
				    RELOC(mem_reserve_map)[i].size);
	}
#endif
+	/* Bump mem_reserve_cnt to cause further reservations to fail
+	 * since it's too late.
+	 */
	RELOC(mem_reserve_cnt) = MEM_RESERVE_MAP_SIZE;
	prom_printf("Device tree strings 0x%x -> 0x%x\n",

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jimi Xenidis]

On Apr 12, 2006, at 10:05 PM, Jimi Xenidis wrote:
>
> NOTE: that the dtc must also not generate the blob reservation
> entry.

This reservation is only generated for "asm" dtc targets so it is  
only that case that need to be changed.

>   Should we try to detect this redundant entry? Should we bump
> the DT version number?

-JX

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Michael Neuling]

Hey Jimi,

> > NOTE: that the dtc must also not generate the blob reservation
> > entry.
> 
> This reservation is only generated for "asm" dtc targets so it is  
> only that case that need to be changed.

The user space kexec tools create a reserve map entry for the blob as
well.  

Mikey

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jimi Xenidis]

On Apr 13, 2006, at 1:36 AM, Michael Neuling wrote:

> Hey Jimi,
>
>>> NOTE: that the dtc must also not generate the blob reservation
>>> entry.
>>
>> This reservation is only generated for "asm" dtc targets so it is
>> only that case that need to be changed.
>
> The user space kexec tools create a reserve map entry for the blob as
> well.
good point, but I thought the kexec tools used dtc, no?
looking passed my own world I see:
   - bootx: creates a reservation entry
   - iSeries: not reserving the blob at all
-JX

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Michael Ellerman]

On Thu, 2006-04-13 at 06:37 -0400, Jimi Xenidis wrote:
> On Apr 13, 2006, at 1:36 AM, Michael Neuling wrote:
> 
> > Hey Jimi,
> >
> >>> NOTE: that the dtc must also not generate the blob reservation
> >>> entry.
> >>
> >> This reservation is only generated for "asm" dtc targets so it is
> >> only that case that need to be changed.
> >
> > The user space kexec tools create a reserve map entry for the blob as
> > well.
> good point, but I thought the kexec tools used dtc, no?

No, but they should.

> looking passed my own world I see:
>    - iSeries: not reserving the blob at all

That sounds right. I think having the kernel do it is definitely the
right option.

cheers

-- 
Michael Ellerman
IBM OzLabs

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

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jimi Xenidis]

Actually, is this even an issue? can the LMB handle repeated  
reservations?
-JX
On Apr 13, 2006, at 7:07 AM, Michael Ellerman wrote:

> On Thu, 2006-04-13 at 06:37 -0400, Jimi Xenidis wrote:
>> On Apr 13, 2006, at 1:36 AM, Michael Neuling wrote:
>>
>>> Hey Jimi,
>>>
>>>>> NOTE: that the dtc must also not generate the blob reservation
>>>>> entry.
>>>>
>>>> This reservation is only generated for "asm" dtc targets so it is
>>>> only that case that need to be changed.
>>>
>>> The user space kexec tools create a reserve map entry for the  
>>> blob as
>>> well.
>> good point, but I thought the kexec tools used dtc, no?
>
> No, but they should.
>
>> looking passed my own world I see:
>>    - iSeries: not reserving the blob at all
>
> That sounds right. I think having the kernel do it is definitely the
> right option.
>
> cheers
>
> -- 
> Michael Ellerman
> IBM OzLabs
>
> 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
>

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Michael Ellerman]

On Thu, 2006-04-13 at 09:19 -0400, Jimi Xenidis wrote:
> Actually, is this even an issue? can the LMB handle repeated  
> reservations?

It can, but we were thinking about adding code to check and warn if
reservations overlap, because it usually indicates a bug. Although
that's probably ok in this case, as long as dtc gets fixed eventually.
Another option would be to not warn for identical reservations.

cheers

> On Apr 13, 2006, at 7:07 AM, Michael Ellerman wrote:
> 
> > On Thu, 2006-04-13 at 06:37 -0400, Jimi Xenidis wrote:
> >> On Apr 13, 2006, at 1:36 AM, Michael Neuling wrote:
> >>
> >>> Hey Jimi,
> >>>
> >>>>> NOTE: that the dtc must also not generate the blob reservation
> >>>>> entry.
> >>>>
> >>>> This reservation is only generated for "asm" dtc targets so it is
> >>>> only that case that need to be changed.
> >>>
> >>> The user space kexec tools create a reserve map entry for the  
> >>> blob as
> >>> well.
> >> good point, but I thought the kexec tools used dtc, no?
> >
> > No, but they should.
> >
> >> looking passed my own world I see:
> >>    - iSeries: not reserving the blob at all
> >
> > That sounds right. I think having the kernel do it is definitely the
> > right option.
> >
> > cheers
> >
> > -- 
> > Michael Ellerman
> > IBM OzLabs
> >
> > 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
> >
> 
-- 
Michael Ellerman
IBM OzLabs

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

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jon Loeliger]

On Thu, 2006-04-13 at 09:34, Michael Ellerman wrote:
> On Thu, 2006-04-13 at 09:19 -0400, Jimi Xenidis wrote:
> > Actually, is this even an issue? can the LMB handle repeated  
> > reservations?
> 
> It can, but we were thinking about adding code to check and warn if
> reservations overlap, because it usually indicates a bug. Although
> that's probably ok in this case, as long as dtc gets fixed eventually.
> Another option would be to not warn for identical reservations.

> > >>>>> NOTE: that the dtc must also not generate the blob reservation
> > >>>>> entry.

> > >> looking passed my own world I see:
> > >>    - iSeries: not reserving the blob at all
> > >
> > > That sounds right. I think having the kernel do it is definitely the
> > > right option.


OK, I'm back to reading the list and beginning to catch
up some here...

Let me see if I understand the consensus and direction:

    1) DTC should NOT reserve its own blob space in the
       memory map, as it does for generated ASM code now,

    2) Kernel should reserve the blob space early so as
       not to step on itself later,

    3a) Kernel LMB handling should be modified to warn
        for overlapping LMB reservations,

Except that Ben says:

    3b) We should make lmb_reserve() of redudant/overlapping
        entries become harmless I think. We need to be
        backward compatible with earlier blobs that do
        contain themselves in the reserve map.

I think we should interpret "harmless" to be "warn" and not
cause an error at this point in time.

I do not think we should have the blob generate its own
reservation because it is possible that some post-processing
(like U-Boot) can modify and extend it.  Only after that can
the blob's true size be determined.  (Sure, it could update
on the fly too... but double blah).

In all of this, I'm on deck for step 1) above.

jdl

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Michael Ellerman]

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

On Tue, 2006-04-18 at 11:48 -0500, Jon Loeliger wrote:
> On Thu, 2006-04-13 at 09:34, Michael Ellerman wrote:
> > On Thu, 2006-04-13 at 09:19 -0400, Jimi Xenidis wrote:
> > > Actually, is this even an issue? can the LMB handle repeated  
> > > reservations?
> > 
> > It can, but we were thinking about adding code to check and warn if
> > reservations overlap, because it usually indicates a bug. Although
> > that's probably ok in this case, as long as dtc gets fixed eventually.
> > Another option would be to not warn for identical reservations.
> 
> > > >>>>> NOTE: that the dtc must also not generate the blob reservation
> > > >>>>> entry.
> 
> > > >> looking passed my own world I see:
> > > >>    - iSeries: not reserving the blob at all
> > > >
> > > > That sounds right. I think having the kernel do it is definitely the
> > > > right option.
> 
> 
> OK, I'm back to reading the list and beginning to catch
> up some here...
> 
> Let me see if I understand the consensus and direction:
> 
>     1) DTC should NOT reserve its own blob space in the
>        memory map, as it does for generated ASM code now,
> 
>     2) Kernel should reserve the blob space early so as
>        not to step on itself later,
> 
>     3a) Kernel LMB handling should be modified to warn
>         for overlapping LMB reservations,
> 
> Except that Ben says:
> 
>     3b) We should make lmb_reserve() of redudant/overlapping
>         entries become harmless I think. We need to be
>         backward compatible with earlier blobs that do
>         contain themselves in the reserve map.
> 
> I think we should interpret "harmless" to be "warn" and not
> cause an error at this point in time.
> 
> I do not think we should have the blob generate its own
> reservation because it is possible that some post-processing
> (like U-Boot) can modify and extend it.  Only after that can
> the blob's true size be determined.  (Sure, it could update
> on the fly too... but double blah).
> 
> In all of this, I'm on deck for step 1) above.

Nice summary :)
I'm up for 3a, we should make redundant/overlapping reserves "harmless",
by which I mean "not an error", but there should definitely be a warning
in the dmesg - as it will _usually_ indicate a bug.

cheers

-- 
Michael Ellerman
IBM OzLabs

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: 191 bytes --]

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jimi Xenidis]

ok, to accept (1) means to accept (2).

AFAICT LMB is already handling overlaps.
So the first rough patch I sent should be sufficient.

On Apr 18, 2006, at 2:04 PM, Michael Ellerman wrote:

> On Tue, 2006-04-18 at 11:48 -0500, Jon Loeliger wrote:
>> On Thu, 2006-04-13 at 09:34, Michael Ellerman wrote:
>>> On Thu, 2006-04-13 at 09:19 -0400, Jimi Xenidis wrote:
>>>> Actually, is this even an issue? can the LMB handle repeated
>>>> reservations?
>>>
>>> It can, but we were thinking about adding code to check and warn if
>>> reservations overlap, because it usually indicates a bug. Although
>>> that's probably ok in this case, as long as dtc gets fixed  
>>> eventually.
>>> Another option would be to not warn for identical reservations.
>>
>>>>>>>>> NOTE: that the dtc must also not generate the blob reservation
>>>>>>>>> entry.
>>
>>>>>> looking passed my own world I see:
>>>>>>    - iSeries: not reserving the blob at all
>>>>>
>>>>> That sounds right. I think having the kernel do it is  
>>>>> definitely the
>>>>> right option.
>>
>>
>> OK, I'm back to reading the list and beginning to catch
>> up some here...
>>
>> Let me see if I understand the consensus and direction:
>>
>>     1) DTC should NOT reserve its own blob space in the
>>        memory map, as it does for generated ASM code now,
>>
>>     2) Kernel should reserve the blob space early so as
>>        not to step on itself later,
>>
>>     3a) Kernel LMB handling should be modified to warn
>>         for overlapping LMB reservations,
>>
>> Except that Ben says:
>>
>>     3b) We should make lmb_reserve() of redudant/overlapping
>>         entries become harmless I think. We need to be
>>         backward compatible with earlier blobs that do
>>         contain themselves in the reserve map.
>>
>> I think we should interpret "harmless" to be "warn" and not
>> cause an error at this point in time.
>>
>> I do not think we should have the blob generate its own
>> reservation because it is possible that some post-processing
>> (like U-Boot) can modify and extend it.  Only after that can
>> the blob's true size be determined.  (Sure, it could update
>> on the fly too... but double blah).
>>
>> In all of this, I'm on deck for step 1) above.
>
> Nice summary :)
> I'm up for 3a, we should make redundant/overlapping reserves  
> "harmless",
> by which I mean "not an error", but there should definitely be a  
> warning
> in the dmesg - as it will _usually_ indicate a bug.
>
> cheers
>
> -- 
> Michael Ellerman
> IBM OzLabs
>
> 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
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jon Loeliger]

On Tue, 2006-04-18 at 13:04, Michael Ellerman wrote:

> > OK, I'm back to reading the list and beginning to catch
> > up some here...
> > 
> > Let me see if I understand the consensus and direction:
> > 
> >     1) DTC should NOT reserve its own blob space in the
> >        memory map, as it does for generated ASM code now,

> > In all of this, I'm on deck for step 1) above.

I have updated the DTC to have several bug fixes:

    - asm_emit_cell() little endian bug fix
    - use two .long and not .quad directives
    - Eliminate emitting memreserve for blob in ASM output

These updates are available here:

    www.jdl.com/git_repos
and
    git://ozlabs.org/srv/projects/dtc/dtc.git

Please verify and let me know of any problems!

> Nice summary :)

Thanks.

> I'm up for 3a, we should make redundant/overlapping reserves "harmless",
> by which I mean "not an error", but there should definitely be a warning
> in the dmesg - as it will _usually_ indicate a bug.

Please feel free to patch these into existence as needed! :-)

Thanks,
jdl

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Mark A. Greer]

On Thu, Apr 20, 2006 at 10:42:28AM -0500, Jon Loeliger wrote:
> On Tue, 2006-04-18 at 13:04, Michael Ellerman wrote:
> 
> > > OK, I'm back to reading the list and beginning to catch
> > > up some here...
> > > 
> > > Let me see if I understand the consensus and direction:
> > > 
> > >     1) DTC should NOT reserve its own blob space in the
> > >        memory map, as it does for generated ASM code now,
> 
> > > In all of this, I'm on deck for step 1) above.
> 
> I have updated the DTC to have several bug fixes:
> 
>     - asm_emit_cell() little endian bug fix
>     - use two .long and not .quad directives
>     - Eliminate emitting memreserve for blob in ASM output
> 
> These updates are available here:
> 
>     www.jdl.com/git_repos
> and
>     git://ozlabs.org/srv/projects/dtc/dtc.git
> 
> Please verify and let me know of any problems!

Hey John,

I did a quick test and the new dtc seems to work fine for my dts file.

Mark

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Benjamin Herrenschmidt]

On Wed, 2006-04-12 at 22:05 -0400, Jimi Xenidis wrote:
> A devtree compiler (dtc) generated devtree blob is "relocatable" and  
> so does not contain a reserved_map entry for the blob itself.
> This means that if passed to Linux, Linux will not get lmb_reserve()  
> the blob and it could be over.
> The following patch will explicitly reserve the "blob" as it was  
> given to us and stops prom_init.c from creating a reserved mapping  
> for the blob.
> 
> NOTE: that the dtc must also not generate the blob reservation  
> entry.  Should we try to detect this redundant entry? Should we bump  
> the DT version number?

We should make lmb_reserve() of redudant/overlapping entries become
harmless I think. We need to be backward compatible with earlier blobs
that do contain themselves in the reserve map

Ben.

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Jimi Xenidis]

On Apr 13, 2006, at 7:12 PM, Benjamin Herrenschmidt wrote:
>
> We should make lmb_reserve() of redudant/overlapping entries become
> harmless I think.

Hmm.. I think it would be worthy of a warning, no?

> We need to be backward compatible with earlier blobs
> that do contain themselves in the reserve map

Do you think it is possible that the blob may have a single  
reservation that includes the blob but is larger? if not then we  
could simply do..
--
diff -r eb0990a251a9 arch/powerpc/kernel/prom.c
--- a/arch/powerpc/kernel/prom.c	Thu Mar 30 22:05:40 2006 -0500
+++ b/arch/powerpc/kernel/prom.c	Fri Apr 14 08:44:10 2006 -0400
@@ -1129,9 +1129,17 @@ static void __init early_reserve_mem(voi
{
	u64 base, size;
	u64 *reserve_map;
+	unsigned long self_base;
+	unsigned long self_size;
	reserve_map = (u64 *)(((unsigned long)initial_boot_params) +
					initial_boot_params->off_mem_rsvmap);
+
+	/* before we do anything, lets reserve the dt blob */
+	self_base = __pa((unsigned long)initial_boot_params);
+	self_size = initial_boot_params->totalsize;
+	lmb_reserve(self_base, self_size);
+
#ifdef CONFIG_PPC32
	/*
	 * Handle the case where we might be booting from an old kexec
@@ -1146,6 +1154,9 @@ static void __init early_reserve_mem(voi
			size_32 = *(reserve_map_32++);
			if (size_32 == 0)
				break;
+			/* skip if the reservation is for the blob */
+			if (base_32 == self_base && size_32 == self_size)
+				continue;
			DBG("reserving: %x -> %x\n", base_32, size_32);
			lmb_reserve(base_32, size_32);
		}
@@ -1157,6 +1168,9 @@ static void __init early_reserve_mem(voi
		size = *(reserve_map++);
		if (size == 0)
			break;
+		/* skip if the reservation is for the blob */
+		if (base == self_base && size == self_size)
+			continue;
		DBG("reserving: %llx -> %llx\n", base, size);
		lmb_reserve(base, size);
	}

Re: [patch][rfc]flattened device tree: Passing a dtb (blob) to Linux.

[Michael Ellerman]

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

On Fri, 2006-04-14 at 08:45 -0400, Jimi Xenidis wrote:
> On Apr 13, 2006, at 7:12 PM, Benjamin Herrenschmidt wrote:
> >
> > We should make lmb_reserve() of redudant/overlapping entries become
> > harmless I think.
> 
> Hmm.. I think it would be worthy of a warning, no?

Definitely. We had weird bugs in kexec because regions were overlapping.
I just haven't got 'round to writing the patch .. maybe I should :)

> > We need to be backward compatible with earlier blobs
> > that do contain themselves in the reserve map
> 
> Do you think it is possible that the blob may have a single  
> reservation that includes the blob but is larger? if not then we  
> could simply do..

Well it's definitely possible, but I don't know if anyone actually does
it. But I guess we should try to be nice and not warn for old blobs.

This patch looks like a decent approach, although I'd want to add a
check so that if the blob is reserved _three_ times we do warn, it's
getting a bit silly isn't it :)

cheers

> --
> diff -r eb0990a251a9 arch/powerpc/kernel/prom.c
> --- a/arch/powerpc/kernel/prom.c	Thu Mar 30 22:05:40 2006 -0500
> +++ b/arch/powerpc/kernel/prom.c	Fri Apr 14 08:44:10 2006 -0400
> @@ -1129,9 +1129,17 @@ static void __init early_reserve_mem(voi
> {
> 	u64 base, size;
> 	u64 *reserve_map;
> +	unsigned long self_base;
> +	unsigned long self_size;
> 	reserve_map = (u64 *)(((unsigned long)initial_boot_params) +
> 					initial_boot_params->off_mem_rsvmap);
> +
> +	/* before we do anything, lets reserve the dt blob */
> +	self_base = __pa((unsigned long)initial_boot_params);
> +	self_size = initial_boot_params->totalsize;
> +	lmb_reserve(self_base, self_size);
> +
> #ifdef CONFIG_PPC32
> 	/*
> 	 * Handle the case where we might be booting from an old kexec
> @@ -1146,6 +1154,9 @@ static void __init early_reserve_mem(voi
> 			size_32 = *(reserve_map_32++);
> 			if (size_32 == 0)
> 				break;
> +			/* skip if the reservation is for the blob */
> +			if (base_32 == self_base && size_32 == self_size)
> +				continue;
> 			DBG("reserving: %x -> %x\n", base_32, size_32);
> 			lmb_reserve(base_32, size_32);
> 		}
> @@ -1157,6 +1168,9 @@ static void __init early_reserve_mem(voi
> 		size = *(reserve_map++);
> 		if (size == 0)
> 			break;
> +		/* skip if the reservation is for the blob */
> +		if (base == self_base && size == self_size)
> +			continue;
> 		DBG("reserving: %llx -> %llx\n", base, size);
> 		lmb_reserve(base, size);
> 	}
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
-- 
Michael Ellerman
IBM OzLabs

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: 191 bytes --]

[PATCH] powerpc: Auto reserve of device tree blob

[Michael Neuling]

From: Jimi Xenidis <jimix@watson.ibm.com>

A devtree compiler (dtc) generated devtree blob is "relocatable" and so
does not contain a reserved_map entry for the blob itself.  This means
that if passed to Linux, Linux will not get lmb_reserve() the blob and
it could be over.  The following patch will explicitly reserve the
"blob" as it was given to us and stops prom_init.c from creating a
reserved mapping for the blob.

NOTE: that the dtc/kexec should not generate the blob reservation entry.
Although if they do, LMB reserver handles overlaps.

Signed-off-by:  <jimix@watson.ibm.com>
Acked-by: Michael Neuling <mikey@neuling.org>
---
After some discussion last time, it seemed this was the right was to do
this.  I'll post the kexec cleanups if/when this gets upstream.

Mikey

 arch/powerpc/kernel/prom.c      |    5 +++++
 arch/powerpc/kernel/prom_init.c |    9 ++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

Index: linux-2.6-powerpc/arch/powerpc/kernel/prom.c
===================================================================
--- linux-2.6-powerpc.orig/arch/powerpc/kernel/prom.c
+++ linux-2.6-powerpc/arch/powerpc/kernel/prom.c
@@ -1240,6 +1240,11 @@ static void __init early_reserve_mem(voi
 
 	reserve_map = (u64 *)(((unsigned long)initial_boot_params) +
 					initial_boot_params->off_mem_rsvmap);
+
+	/* before we do anything, lets reserve the dt blob */
+	lmb_reserve(__pa((unsigned long)initial_boot_params),
+		    initial_boot_params->totalsize);
+
 #ifdef CONFIG_PPC32
 	/* 
 	 * Handle the case where we might be booting from an old kexec
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
@@ -2031,11 +2031,7 @@ static void __init flatten_device_tree(v
 	/* Version 16 is not backward compatible */
 	hdr->last_comp_version = 0x10;
 
-	/* Reserve the whole thing and copy the reserve map in, we
-	 * also bump mem_reserve_cnt to cause further reservations to
-	 * fail since it's too late.
-	 */
-	reserve_mem(RELOC(dt_header_start), hdr->totalsize);
+	/* Copy the reserve map in */
 	memcpy(rsvmap, RELOC(mem_reserve_map), sizeof(mem_reserve_map));
 
 #ifdef DEBUG_PROM
@@ -2048,6 +2044,9 @@ static void __init flatten_device_tree(v
 				    RELOC(mem_reserve_map)[i].size);
 	}
 #endif
+	/* Bump mem_reserve_cnt to cause further reservations to fail
+	 * since it's too late.
+	 */
 	RELOC(mem_reserve_cnt) = MEM_RESERVE_MAP_SIZE;
 
 	prom_printf("Device tree strings 0x%x -> 0x%x\n",

Re: [PATCH] powerpc: Auto reserve of device tree blob

[Jon Loeliger]

On Thu, 2006-05-18 at 17:03, Michael Neuling wrote:
> From: Jimi Xenidis <jimix@watson.ibm.com>
> 
> A devtree compiler (dtc) generated devtree blob is "relocatable" and so
> does not contain a reserved_map entry for the blob itself.  This means
> that if passed to Linux, Linux will not get lmb_reserve() the blob and
> it could be over.  The following patch will explicitly reserve the
> "blob" as it was given to us and stops prom_init.c from creating a
> reserved mapping for the blob.
> 
> NOTE: that the dtc/kexec should not generate the blob reservation entry.
> Although if they do, LMB reserver handles overlaps.

The current DTC does not issue the memory reservation request.
As discussed earlier, it is now relying on the kernel
to do a reservation for the itself.  While I've not seen
it applied yet, I am assuming that Jimi's patch from 14 April
is what is needed here and will be applied.  (Maybe it
just needs a "Signed-off" to make it happen.  Dunno.)

Thanks,
jdl

Re: [PATCH] powerpc: Auto reserve of device tree blob

[Jon Loeliger]

On Mon, 2006-05-22 at 11:25, Jon Loeliger wrote:
> While I've not seen
> it applied yet, I am assuming that Jimi's patch from 14 April
> is what is needed here and will be applied.

And that patch appears to be in Paul's tree now!

jdl