[SLOF] [PATCH slof] fdt: Pass the resulting device tree to QEMU

[Alexey Kardashevskiy]

On 02/10/17 21:16, Segher Boessenkool wrote:
> Hi!
> 
> 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.
> 
> Yeah.
> 
> 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 ;
may be?

I am missing something here, the words below assume that fdt-alloc returns
something :-/


> : 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.



-- 
Alexey