[SLOF] [PATCH slof] fdt: Pass the resulting device tree to QEMU
aik at ozlabs.ru
Mon Oct 2 22:09:52 AEDT 2017
On 02/10/17 21:16, Segher Boessenkool wrote:
> On Mon, Oct 02, 2017 at 12:08:32PM +1100, Alexey Kardashevskiy wrote:
>> On 02/10/17 06:34, Segher Boessenkool wrote:
>>> On Fri, Sep 29, 2017 at 07:07:29PM +1000, Alexey Kardashevskiy wrote:
>>>> +: fdt-struct-add ( bytes len nullterminate -- )
>>>> + >r
>>>> + dup >r ( bytes len r: nullterminate len )
>>>> + fdt-struct-cur @ swap ( bytes cur len r: nullterminate len )
>>>> + move
>>>> + fdt-struct-cur @ ( cur r: nullterminate len )
>>>> + r> + ( cur r: nullterminate )
>>>> + r> IF
>>>> + 0 over c!
>>>> + 1+
>>>> + THEN
>>>> + 3 + -4 and
>>>> + fdt-struct-cur !
>>> You want a comment on this function,
>> You mean text comments, not just stack comments? Ok.
> Yeah, a comment on what the word does, since the name doesn't make it
> obvious. Better is to have a better name, but I can't think of any
> either. Maybe this needs to factored a bit better?
>>> and you probably want a convenience
>>> word to do the zero-termination (e.g. fdt-struct-add-zero-terminated).
>> Seeing too many fdt-struct-cur hurts my eyes so I'd rather not.
> So maybe something like
> VALUE fdt-here \ instead of fdt-struct-cur
> : fdt-allot fdt-here + to fdt-here ;
: fdt-allot ( len -- ) fdt-here + to fdt-here ;
I am missing something here, the words below assume that fdt-alloc returns
> : fdt-c, fdt-here 1 fdt-allot c! ;
> : fdt-align fdt-here 4 #aligned fdt-here - ; \ or fill with zeroes?
> : fdt-str, ( str len -- ) fdt-here over fdt-allot swap move ;
> : fdt-ztr, ( str len -- ) fdt-str, 0 fdt-c, ;
> \ and now use fdt-str, fdt-align instead of false fdt-struct-add
> \ and just fdt-ztr, fdt-align for true fdt-struct-add
>>>> +: fdt-add-long ( token -- )
>>>> + fdt-struct-cur @
>>>> + l!
>>>> + fdt-struct-cur @
>>>> + 4 +
>>>> + fdt-struct-cur !
>>> : fdt-add-long ( token -- )
>>> fdt-struct-cur @ l! /l fdt-struct-cur +! ;
> This would be fdt-l, then. Using similar names to the standard data
> space allocation words makes things easy to understand (and these are a
> known good design so you don't have to spend time designing a good
> interface :-) )
>>> So "str" is a zero-terminated string here.
>>> : zstring= ( str len zstr -- flag )
>>> 2dup + c@ IF 3drop false EXIT THEN swap comp 0= ;
>> Thanks, this is better :)
>> But - is it really necessary to make words a single line? What is exactly
>> that you win when you do this? It is really hard to read.
> You can use whatever style you like best :-) I find this more readable,
> or perhaps
> : zstring= ( str len zstr -- flag )
> 2dup + c@ IF
> 3drop false EXIT THEN
> swap comp 0= ;
> if you want to be fancy. It's a good idea to keep your words short and
> sweet, and then indent etc. doesn't matter anyway.
>> My bad, "reused" thing is a leftover from debug code which tells how many
>> strings this has saved, you can ignore it.
> Ah :-)
>>> This word is so big that it is very hard to follow, btw.
>> Hmmm. I'll split out the lookup bit. fdt-flatten-tree is even bigger but
>> you did not make the same comment about it.
> But fdt-flatten-tree is mostly a list of things to do in order.
> I didn't comment on it because I haven't reviewed the whole thing, btw.
>>> I think this takes so long on huge device trees because you do way too
>>> many string compares, but it is hard to tell.
>> I'll double check but 2.8sec on such a massive guest is really not much as
>> the rest takes lot lot more time.
> Ah, right. You have to create that tree in the first place already :-)
>> Thanks for the comment. Are you still going to do the full review or I can
>> respin? :)
> Sure, but it will be a while.
I got a bit bored on a Labour Day (today, in ACT) and posted v2 :) Do you
want to me to post v3 with fdt-allot & friends first?
>> One question while I remember: firstly I looked at how "(.properties)" and
>> ".property" work and these things do not use "next-property", it is
>> node>properties, link>, etc - is this described somewhere (1275?) or it is
>> the SLOF implementation detail? It is probably faster but definitely harder
>> to read.
> Implementation details.
> node>properties is defined in node.fs, it just gets the address of a
> struct field.
> The properties are not a linked list, but a Forth wordlist. This is a)
> faster, because wordlists are optimised a bit, say ten times faster; and
> b) this allows use to run any code we want to get a property, not just
> lookups of static data. From property.fs:
> \ Words on the property list for a node are actually executable words,
> \ that return the address and length of the property's data. Special
> \ nodes like /options can have their properties use specialized code to
> \ dynamically generate their data; most nodes just use a 2CONSTANT.
Hm. I wonder if it makes sense to rewrite fdt-properties using this.
More information about the SLOF