[PATCH 6/10] ppc64: Create a fake flat device tree on iSeries

[David Gibson]

On Wed, Aug 10, 2005 at 02:16:15AM -0500, Milton Miller wrote:
> 1) These funtions would seem to be infrastucture, do they belong in
> iSeries_setup?   I note that they are not declared static.
> 
> 2) checking the blob afterwards is likely buggy because if we add
> more than 4 bytes, we will overwrite the next pointer and inc the
> data therein before we realize that we overwrote the  pointer.
> 	a) move pointer before blob data
> 	b) move check before set, and pass size to check
> 	c) see #6
> 
> 3) dt_prop is not checking if the property name already exists,
> wasting string space.  While this may not be an issue in small usage
> so far, there are obviously replacated strings (cpu propertys, but
> also things like "name").

We need to weight the space-wastage of duplicated strings against
either the extra complexity of a lookup table, or the time expenditure
of scanning the whole string table on every dt_prop().

> 4) While the blob size is arbitaray, a structure PAGE_SIZE + u64
> is a bit odd.  In fact if we swtich to only saving the string table
> it makes it take 2 pages instead of one.
> 
> 5) If you are going to statically allocate this for iSeries (as
> opossed to say a klimit bump), then the only thing in dynamic is
> setting the boot cpuid.  May as well use a static initializer.
> 
> Today this would add over 2 pages of bss for a "combined" kernel.

I think you'll find there are patches coming making more stuff
dynamic.  Virtual devices, in particular.

> 6) I suggest using propnum from fs2dt.c for #3 and open coding the
> checks for overflow against the device tree fields.  

Better yet, use the string insert function from libdt included in the
dtc sources (which probably amounts to almos the same thing).

> Putting the
> device data at klimit would allow the checks for overflow to be
> deferred until after its created (look for 2 nuls in string table
> -- the size is propnum(""), and placing data at after the reserve
> map and string table and either compare to total size or set
> totalsize according to what is used).

Hrm.. actually.. this may be too irritatingly inflexible, but since
the iseries device tree is pretty limited in what it contains, it
would potentially be possible to statically initialize a string table
containing every property name iSeries might ever use.  In fact, doing
it that way you could generate the skeleton of the device tree with
dtc (asm output mode), leaving some space in the structure section,
then add in the dynamic structure at runtime.  (I'd need to add
something to dtc to allow you to force a string into the string
section even if it's not used in the tree, of course, but that should
be easy)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson