Re: [RFC PATCH 0/4] Add device-tree support to kexec-tools for ARM

From: Simon Horman <horms@verge.net.au>
To: Matthew Leach <matthew.leach@arm.com>
Cc: kexec@lists.infradead.org, Will Deacon <Will.Deacon@arm.com>
Subject: Re: [RFC PATCH 0/4]  Add device-tree support to kexec-tools for ARM
Date: Fri, 7 Sep 2012 07:00:34 +0900	[thread overview]
Message-ID: <20120906220031.GA10174@verge.net.au> (raw)
In-Reply-To: <000101cd8c1f$6f758520$4e608f60$@leach@arm.com>

On Thu, Sep 06, 2012 at 12:04:50PM +0100, Matthew Leach wrote:
> Hi Simon,
> 
> I have been having some issues using kexec with your dtb patches...
> 
> > Thanks. It was that part of the code that I spent the bulk of my time
> > on.
> > And although it is still has a few rough edges I would be happy for it
> > to be used.
> 
> I have looked at the output from your generic fs2dt and compared
> it to the original dts and all looks okay so I'm happy that this
> part of your code works fine.

:)

> > I would prefer to avoid requiring kernel changes unless necessary -
> > the kernels some of the boards I work with require DT since 3.5.
> > However, I am happy to discuss this further, there certainly is
> > merit to a clean implementation.
> 
> I believe that you are loading the dtb at an offset from the base
> of 0x1000, this is where the problem lies in that the dtb can be
> corrupted by the page tables of the decompressor.

Of course, sorry for missing that.

> Also, device trees can contain firmware and as such be on the
> order of megabytes in size. This could potentially corrupt the
> decompressor image depending upon the order that these two blobs
> are written to memory.

Yes, I see that now. The dtb I was using for testing was rather small,
less than 200b IIRC.

Actually, I accidently left debug code in the patchset that prints a hex
dump of the dtb to stdtout. That ought to be removed but that the dump
fited comfortably in few dozen lines illustrates how small a blob I had.

> I suggest that we put the DTB out of the way, perhaps just after
> the initrd segment, or at the initrd_offset in the case that
> there is no initrd. This would require a kernel change to set the
> correct parameter to the relocate_new_kerenel function, but the
> change is minimal.
> 
> If you are happy with this, I have a set of patches that does the
> job.

Thanks, for the explanation, and thanks to Will for his.
I'm happy with the approach that you propose.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec