| Message ID | 20170929090729.19871-1-aik@ozlabs.ru |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fdt: Pass the resulting device tree to QEMU | expand |
Forgot to cc: Balbir, he _wants_ to learn Forth/SLOF :-D On 29/09/17 19:07, Alexey Kardashevskiy wrote: > This creates flatten device tree and passes it to QEMU via a custom > hypercall right before jumping to RTAS. > > On a machine with 256 CPUs and 256 virtual Intel E1000 devices the blob > is 360KB (356KB structs and 4KB of strings), building such a tree takes > 2.8s on a POWER8 box. A simple tree with 1 CPU and a couple of devices > takes 43ms and creates 16KB blob. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > I tested the blob by storing it from QEMU to a file (RFC patch is > coming next) and decompiling it; this produces error which I do not really > understand as the name of the root is an empty string (literaly: > 00 00 00 01 00 00 00 00) and yet this error: > > aik@fstn1-p1:~$ dtc -f -I dtb -O dts -o dbg.dts dbg.dtb > ERROR (name_properties): "name" property in / is incorrect ("/" instead of base node name) > Warning: Input tree has errors, output forced > --- > lib/libhvcall/libhvcall.h | 1 + > board-qemu/slof/fdt.fs | 224 +++++++++++++++++++++++++++++++++++++++++++++- > board-qemu/slof/rtas.fs | 4 + > lib/libhvcall/hvcall.code | 5 ++ > lib/libhvcall/hvcall.in | 1 + > 5 files changed, 234 insertions(+), 1 deletion(-) > > diff --git a/lib/libhvcall/libhvcall.h b/lib/libhvcall/libhvcall.h > index 5776a2b..1c2d31b 100644 > --- a/lib/libhvcall/libhvcall.h > +++ b/lib/libhvcall/libhvcall.h > @@ -26,6 +26,7 @@ > #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2) > #define KVMPPC_H_RTAS_UPDATE (KVMPPC_HCALL_BASE + 0x3) > #define KVMPPC_H_UPDATE_PHANDLE (KVMPPC_HCALL_BASE + 0x4) > +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x5) > > #ifndef __ASSEMBLY__ > > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > index a24e344..b99746a 100644 > --- a/board-qemu/slof/fdt.fs > +++ b/board-qemu/slof/fdt.fs > @@ -27,7 +27,7 @@ struct > 4 field >fdth_boot_cpu > 4 field >fdth_string_size > 4 field >fdth_struct_size > -drop > +constant /fdth > > h# d00dfeed constant OF_DT_HEADER > h# 1 constant OF_DT_BEGIN_NODE > @@ -449,4 +449,226 @@ r> drop > fdt-cas-fix? > ; > > +VARIABLE fdt-struct > +VARIABLE fdt-struct-cur > +VARIABLE fdt-strings > +VARIABLE fdt-strings-cur > +VARIABLE fdt-strings-reused > +VARIABLE fdt-ms > + > +: 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 ! > +; > + > +: fdt-add-long ( token -- ) > + fdt-struct-cur @ > + l! > + fdt-struct-cur @ > + 4 + > + fdt-struct-cur ! > +; > + > +: skip-to-next ( cur -- cur ) > + BEGIN > + dup c@ > + WHILE > + 1+ > + REPEAT > + 4 + -4 and > +; > + > +: string-equal ( name namelen str -- true | false ) > + 3dup swap comp 0 <> IF > + 3drop > + false > + EXIT > + THEN > + + c@ 0 = nip > + > + dup IF > + fdt-strings-reused @ 1+ fdt-strings-reused ! > + THEN > +; > + > +: fdt-get-string ( name namelen -- nameoff ) > + \ lookup > + fdt-strings @ > + BEGIN > + dup fdt-strings-cur @ > + = not > + WHILE > + 3dup string-equal IF > + \ found it, no need to add new one > + fdt-strings @ - > + -rot > + 2drop > + EXIT > + THEN > + skip-to-next > + REPEAT > + drop > + > + \ store cur offset > + fdt-strings-cur @ fdt-strings @ - >r > + > + \ store len > + dup >r > + > + \ copy string > + fdt-strings-cur @ swap ( name cur namelen ) > + move > + > + \ increment cur and null terminate > + fdt-strings-cur @ > + r> + > + 0 over c! > + 4 + -4 and > + fdt-strings-cur ! > + > + \ return offset > + r> > +; > + > +: fdt-begin-node ( name namelen -- ) > + OF_DT_BEGIN_NODE fdt-add-long > + 2dup 1 = swap c@ 2F = and \ is it "/"? > + IF > + 2drop s" " \ dtc is still unhappy though > + THEN > + true fdt-struct-add > +; > + > +: fdt-end-node ( -- ) > + OF_DT_END_NODE fdt-add-long > +; > + > +: fdt-prop ( prop len name namelen -- ) > + OF_DT_PROP fdt-add-long > + > + \ get string offset > + fdt-get-string ( prop len nameoff ) > + > + \ store len and nameoff > + over fdt-add-long > + fdt-add-long ( prop len ) > + > + \ now add the bytes > + false fdt-struct-add > +; > + > +: fdt-end ( -- ) > + OF_DT_END fdt-add-long > +; > + > +: fdt-properties ( phandle -- ) > + >r > + s" " > + BEGIN > + r@ next-property > + WHILE > + 2dup > + 2dup r@ get-property > + not IF > + 2swap fdt-prop > + THEN > + REPEAT > + r> > + drop > +; > + > +: fdt-flatten-node ( node -- ) > +\ fdt-debug IF dup node>path type cr THEN > + dup node>qname fdt-begin-node > + dup fdt-properties > + child > + BEGIN > + dup > + WHILE > + dup recurse > + peer > + REPEAT > + drop > + fdt-end-node > +; > + > +: fdt-flatten-tree ( root -- tree ) > + 200000 alloc-mem dup fdt-struct-cur ! fdt-struct ! > + 10000 alloc-mem dup fdt-strings-cur ! fdt-strings ! > + 0 fdt-strings-reused ! > + milliseconds fdt-ms ! > + > + fdt-flatten-node > + fdt-end > + > + fdt-struct-cur @ fdt-struct @ - > + fdt-strings-cur @ fdt-strings @ - > + fdt-debug IF > + 2dup > + ." Strings=" .d cr > + ." Struct=" .d cr > + ." Reused strings=" fdt-strings-reused @ .d cr > + milliseconds fdt-ms @ - > + ." Took " .d ." ms" cr > + THEN > + > + 2dup + /fdth + > + 10 + \ reserved block > + dup > + fdt-debug IF > + dup ." FDT flat size=" .d cr > + THEN > + alloc-mem ( struct-len strings-len totallen fdt ) > + >r ( struct-len strings-len totallen r: fdt ) > + > + \ write header > + OF_DT_HEADER r@ >fdth_magic l! > + dup r@ >fdth_tsize l! > + /fdth 10 + 2 pick + r@ >fdth_struct_off l! > + /fdth 10 + r@ >fdth_string_off l! > + /fdth r@ >fdth_rsvmap_off l! > + 17 r@ >fdth_version l! > + 17 r@ >fdth_compat_vers l! > + 0 r@ >fdth_boot_cpu l! > + over r@ >fdth_string_size l! > + 2 pick r@ >fdth_struct_size l! > + > + drop > + r@ /fdth + ( struct-len strings-len curfdt r: fdt ) > + > + \ write reserve entry > + 0 over ! > + cell+ > + 0 over ! > + cell+ > + > + \ copy string > + 2dup fdt-strings @ swap rot move > + + ( struct-len curfdt r: fdt ) > + > + \ copy struct > + fdt-struct @ swap rot move ( r: fdt ) > + > + \ cleanup > + fdt-struct @ 200000 free-mem > + fdt-strings @ 10000 free-mem > + > + \ return fdt > + r> > +; > + > +: fdt-flatten-tree-free ( tree ) > + dup >fdth_tsize l@ free-mem > +; > + > s" /" find-node fdt-fix-phandles > diff --git a/board-qemu/slof/rtas.fs b/board-qemu/slof/rtas.fs > index 54d3929..5beb079 100644 > --- a/board-qemu/slof/rtas.fs > +++ b/board-qemu/slof/rtas.fs > @@ -98,6 +98,10 @@ find-qemu-rtas > ; > > : rtas-quiesce ( -- ) > + " /" find-node > + fdt-flatten-tree > + dup hv-update-dt > + fdt-flatten-tree-free > " quiesce" rtas-get-token rtas-cb rtas>token l! > 0 rtas-cb rtas>nargs l! > 0 rtas-cb rtas>nret l! > diff --git a/lib/libhvcall/hvcall.code b/lib/libhvcall/hvcall.code > index 8349748..6ff5715 100644 > --- a/lib/libhvcall/hvcall.code > +++ b/lib/libhvcall/hvcall.code > @@ -136,3 +136,8 @@ PRIM(hv_X2d_update_X2d_phandle) > uint32_t old_phandle = TOS.u; > TOS.u = hv_generic(KVMPPC_H_UPDATE_PHANDLE, old_phandle, new_phandle); > MIRP > + > +PRIM(hv_X2d_update_X2d_dt) > + unsigned long dt = TOS.u; > + TOS.u = hv_generic(KVMPPC_H_UPDATE_DT, dt); > +MIRP > diff --git a/lib/libhvcall/hvcall.in b/lib/libhvcall/hvcall.in > index ab7513a..b59e3f7 100644 > --- a/lib/libhvcall/hvcall.in > +++ b/lib/libhvcall/hvcall.in > @@ -32,4 +32,5 @@ cod(hv-logical-memop) > cod(hv-cas) > cod(hv-rtas-update) > cod(hv-update-phandle) > +cod(hv-update-dt) > cod(get-print-version) >
On 29/09/17 19:07, Alexey Kardashevskiy wrote: > This creates flatten device tree and passes it to QEMU via a custom > hypercall right before jumping to RTAS. > > On a machine with 256 CPUs and 256 virtual Intel E1000 devices the blob > is 360KB (356KB structs and 4KB of strings), building such a tree takes > 2.8s on a POWER8 box. A simple tree with 1 CPU and a couple of devices > takes 43ms and creates 16KB blob. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > I tested the blob by storing it from QEMU to a file (RFC patch is > coming next) and decompiling it; this produces error which I do not really > understand as the name of the root is an empty string (literaly: > 00 00 00 01 00 00 00 00) and yet this error: > > aik@fstn1-p1:~$ dtc -f -I dtb -O dts -o dbg.dts dbg.dtb > ERROR (name_properties): "name" property in / is incorrect ("/" instead of base node name) > Warning: Input tree has errors, output forced Forgot to mention - the resulting tree does not phandles (which is the whole purpose of the patch), I'll have to add them manually :)
On 29/09/17 19:07, Alexey Kardashevskiy wrote: > This creates flatten device tree and passes it to QEMU via a custom > hypercall right before jumping to RTAS. > > On a machine with 256 CPUs and 256 virtual Intel E1000 devices the blob > is 360KB (356KB structs and 4KB of strings), building such a tree takes > 2.8s on a POWER8 box. A simple tree with 1 CPU and a couple of devices > takes 43ms and creates 16KB blob. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > I tested the blob by storing it from QEMU to a file (RFC patch is > coming next) and decompiling it; this produces error which I do not really > understand as the name of the root is an empty string (literaly: > 00 00 00 01 00 00 00 00) and yet this error: > > aik@fstn1-p1:~$ dtc -f -I dtb -O dts -o dbg.dts dbg.dtb > ERROR (name_properties): "name" property in / is incorrect ("/" instead of base node name) > Warning: Input tree has errors, output forced > --- > lib/libhvcall/libhvcall.h | 1 + > board-qemu/slof/fdt.fs | 224 +++++++++++++++++++++++++++++++++++++++++++++- > board-qemu/slof/rtas.fs | 4 + > lib/libhvcall/hvcall.code | 5 ++ > lib/libhvcall/hvcall.in | 1 + > 5 files changed, 234 insertions(+), 1 deletion(-) > > diff --git a/lib/libhvcall/libhvcall.h b/lib/libhvcall/libhvcall.h > index 5776a2b..1c2d31b 100644 > --- a/lib/libhvcall/libhvcall.h > +++ b/lib/libhvcall/libhvcall.h > @@ -26,6 +26,7 @@ > #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2) > #define KVMPPC_H_RTAS_UPDATE (KVMPPC_HCALL_BASE + 0x3) > #define KVMPPC_H_UPDATE_PHANDLE (KVMPPC_HCALL_BASE + 0x4) > +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x5) > > #ifndef __ASSEMBLY__ > > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > index a24e344..b99746a 100644 > --- a/board-qemu/slof/fdt.fs > +++ b/board-qemu/slof/fdt.fs > @@ -27,7 +27,7 @@ struct > 4 field >fdth_boot_cpu > 4 field >fdth_string_size > 4 field >fdth_struct_size > -drop > +constant /fdth > > h# d00dfeed constant OF_DT_HEADER > h# 1 constant OF_DT_BEGIN_NODE > @@ -449,4 +449,226 @@ r> drop > fdt-cas-fix? > ; > > +VARIABLE fdt-struct > +VARIABLE fdt-struct-cur > +VARIABLE fdt-strings > +VARIABLE fdt-strings-cur > +VARIABLE fdt-strings-reused > +VARIABLE fdt-ms > + > +: 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 ! > +; > + > +: fdt-add-long ( token -- ) > + fdt-struct-cur @ > + l! > + fdt-struct-cur @ > + 4 + > + fdt-struct-cur ! > +; > + > +: skip-to-next ( cur -- cur ) > + BEGIN > + dup c@ > + WHILE > + 1+ > + REPEAT > + 4 + -4 and > +; > + > +: string-equal ( name namelen str -- true | false ) > + 3dup swap comp 0 <> IF > + 3drop > + false > + EXIT > + THEN > + + c@ 0 = nip > + > + dup IF > + fdt-strings-reused @ 1+ fdt-strings-reused ! > + THEN > +; > + > +: fdt-get-string ( name namelen -- nameoff ) > + \ lookup > + fdt-strings @ > + BEGIN > + dup fdt-strings-cur @ > + = not > + WHILE > + 3dup string-equal IF > + \ found it, no need to add new one > + fdt-strings @ - > + -rot > + 2drop > + EXIT > + THEN > + skip-to-next > + REPEAT > + drop > + > + \ store cur offset > + fdt-strings-cur @ fdt-strings @ - >r > + > + \ store len > + dup >r > + > + \ copy string > + fdt-strings-cur @ swap ( name cur namelen ) > + move > + > + \ increment cur and null terminate > + fdt-strings-cur @ > + r> + > + 0 over c! > + 4 + -4 and > + fdt-strings-cur ! > + > + \ return offset > + r> > +; > + > +: fdt-begin-node ( name namelen -- ) > + OF_DT_BEGIN_NODE fdt-add-long > + 2dup 1 = swap c@ 2F = and \ is it "/"? > + IF > + 2drop s" " \ dtc is still unhappy though > + THEN > + true fdt-struct-add > +; > + > +: fdt-end-node ( -- ) > + OF_DT_END_NODE fdt-add-long > +; > + > +: fdt-prop ( prop len name namelen -- ) > + OF_DT_PROP fdt-add-long > + > + \ get string offset > + fdt-get-string ( prop len nameoff ) > + > + \ store len and nameoff > + over fdt-add-long > + fdt-add-long ( prop len ) > + > + \ now add the bytes > + false fdt-struct-add > +; > + > +: fdt-end ( -- ) > + OF_DT_END fdt-add-long > +; > + > +: fdt-properties ( phandle -- ) Missing here: dup encode-int s" phandle" fdt-prop > + >r > + s" " > + BEGIN > + r@ next-property > + WHILE > + 2dup > + 2dup r@ get-property > + not IF > + 2swap fdt-prop > + THEN > + REPEAT > + r> > + drop > +; > + > +: fdt-flatten-node ( node -- ) > +\ fdt-debug IF dup node>path type cr THEN > + dup node>qname fdt-begin-node > + dup fdt-properties > + child > + BEGIN > + dup > + WHILE > + dup recurse > + peer > + REPEAT > + drop > + fdt-end-node > +; > + > +: fdt-flatten-tree ( root -- tree ) > + 200000 alloc-mem dup fdt-struct-cur ! fdt-struct ! > + 10000 alloc-mem dup fdt-strings-cur ! fdt-strings ! > + 0 fdt-strings-reused ! > + milliseconds fdt-ms ! > + > + fdt-flatten-node > + fdt-end > + > + fdt-struct-cur @ fdt-struct @ - > + fdt-strings-cur @ fdt-strings @ - > + fdt-debug IF > + 2dup > + ." Strings=" .d cr > + ." Struct=" .d cr > + ." Reused strings=" fdt-strings-reused @ .d cr > + milliseconds fdt-ms @ - > + ." Took " .d ." ms" cr > + THEN > + > + 2dup + /fdth + > + 10 + \ reserved block > + dup > + fdt-debug IF > + dup ." FDT flat size=" .d cr > + THEN > + alloc-mem ( struct-len strings-len totallen fdt ) > + >r ( struct-len strings-len totallen r: fdt ) > + > + \ write header > + OF_DT_HEADER r@ >fdth_magic l! > + dup r@ >fdth_tsize l! > + /fdth 10 + 2 pick + r@ >fdth_struct_off l! > + /fdth 10 + r@ >fdth_string_off l! > + /fdth r@ >fdth_rsvmap_off l! > + 17 r@ >fdth_version l! > + 17 r@ >fdth_compat_vers l! > + 0 r@ >fdth_boot_cpu l! > + over r@ >fdth_string_size l! > + 2 pick r@ >fdth_struct_size l! > + > + drop > + r@ /fdth + ( struct-len strings-len curfdt r: fdt ) > + > + \ write reserve entry > + 0 over ! > + cell+ > + 0 over ! > + cell+ > + > + \ copy string > + 2dup fdt-strings @ swap rot move > + + ( struct-len curfdt r: fdt ) > + > + \ copy struct > + fdt-struct @ swap rot move ( r: fdt ) > + > + \ cleanup > + fdt-struct @ 200000 free-mem > + fdt-strings @ 10000 free-mem > + > + \ return fdt > + r> > +; > + > +: fdt-flatten-tree-free ( tree ) > + dup >fdth_tsize l@ free-mem > +; > + > s" /" find-node fdt-fix-phandles > diff --git a/board-qemu/slof/rtas.fs b/board-qemu/slof/rtas.fs > index 54d3929..5beb079 100644 > --- a/board-qemu/slof/rtas.fs > +++ b/board-qemu/slof/rtas.fs > @@ -98,6 +98,10 @@ find-qemu-rtas > ; > > : rtas-quiesce ( -- ) > + " /" find-node > + fdt-flatten-tree > + dup hv-update-dt > + fdt-flatten-tree-free > " quiesce" rtas-get-token rtas-cb rtas>token l! > 0 rtas-cb rtas>nargs l! > 0 rtas-cb rtas>nret l! > diff --git a/lib/libhvcall/hvcall.code b/lib/libhvcall/hvcall.code > index 8349748..6ff5715 100644 > --- a/lib/libhvcall/hvcall.code > +++ b/lib/libhvcall/hvcall.code > @@ -136,3 +136,8 @@ PRIM(hv_X2d_update_X2d_phandle) > uint32_t old_phandle = TOS.u; > TOS.u = hv_generic(KVMPPC_H_UPDATE_PHANDLE, old_phandle, new_phandle); > MIRP > + > +PRIM(hv_X2d_update_X2d_dt) > + unsigned long dt = TOS.u; > + TOS.u = hv_generic(KVMPPC_H_UPDATE_DT, dt); > +MIRP > diff --git a/lib/libhvcall/hvcall.in b/lib/libhvcall/hvcall.in > index ab7513a..b59e3f7 100644 > --- a/lib/libhvcall/hvcall.in > +++ b/lib/libhvcall/hvcall.in > @@ -32,4 +32,5 @@ cod(hv-logical-memop) > cod(hv-cas) > cod(hv-rtas-update) > cod(hv-update-phandle) > +cod(hv-update-dt) > cod(get-print-version) >
Hi Alexey, Just some random comments, not a full review: 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, and you probably want a convenience word to do the zero-termination (e.g. fdt-struct-add-zero-terminated). > +: 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 +! ; "add" is not such a great name, maybe you should name this like property encoding (and maybe structure it like it even), so fdt-encode-int etc.? > +: skip-to-next ( cur -- cur ) > + BEGIN > + dup c@ > + WHILE > + 1+ > + REPEAT > + 4 + -4 and > +; : skip-to-next ( addr -- addr ) dup zcount + 1+ 4 #aligned ; Skip to next what? "fdt-skip-string" perhaps? > +: string-equal ( name namelen str -- true | false ) > + 3dup swap comp 0 <> IF > + 3drop > + false > + EXIT > + THEN > + + c@ 0 = nip > + > + dup IF > + fdt-strings-reused @ 1+ fdt-strings-reused ! > + THEN > +; So "str" is a zero-terminated string here. : zstring= ( str len zstr -- flag ) 2dup + c@ IF 3drop false EXIT THEN swap comp 0= ; \ Not a great name, it does this "reused" thing, too? : string-equal ( name namelen str -- true | false ) zstring= dup IF 1 fdt-strings-reused +! ; > +: fdt-get-string ( name namelen -- nameoff ) > + \ lookup > + fdt-strings @ > + BEGIN > + dup fdt-strings-cur @ > + = not Don't use NOT, it does not do what you think (it is INVERT, but you want 0= here; could just say <> of course). This word is so big that it is very hard to follow, btw. > +: fdt-begin-node ( name namelen -- ) > + OF_DT_BEGIN_NODE fdt-add-long > + 2dup 1 = swap c@ 2F = and \ is it "/"? This would be ideal for COMPARE except that was never implemented ;-) s" /" compare (It's also probably more readable to write [char] / instead of 2F, if you don't want to use the string words). I think this takes so long on huge device trees because you do way too many string compares, but it is hard to tell. Segher
On 02/10/17 06:34, Segher Boessenkool wrote: > Hi Alexey, > > Just some random comments, not a full review: > > 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. > 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. > >> +: 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 +! ; > > "add" is not such a great name, maybe you should name this like property > encoding (and maybe structure it like it even), so fdt-encode-int etc.? > >> +: skip-to-next ( cur -- cur ) >> + BEGIN >> + dup c@ >> + WHILE >> + 1+ >> + REPEAT >> + 4 + -4 and >> +; > > : skip-to-next ( addr -- addr ) dup zcount + 1+ 4 #aligned ; > > Skip to next what? "fdt-skip-string" perhaps? > >> +: string-equal ( name namelen str -- true | false ) >> + 3dup swap comp 0 <> IF >> + 3drop >> + false >> + EXIT >> + THEN >> + + c@ 0 = nip >> + >> + dup IF >> + fdt-strings-reused @ 1+ fdt-strings-reused ! >> + THEN >> +; > > 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. > \ Not a great name, it does this "reused" thing, too? > : string-equal ( name namelen str -- true | false ) > zstring= dup IF 1 fdt-strings-reused +! ; My bad, "reused" thing is a leftover from debug code which tells how many strings this has saved, you can ignore it. >> +: fdt-get-string ( name namelen -- nameoff ) >> + \ lookup >> + fdt-strings @ >> + BEGIN >> + dup fdt-strings-cur @ >> + = not > > Don't use NOT, it does not do what you think (it is INVERT, but you want > 0= here; could just say <> of course). Yup, a bug, thanks. > > 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. > >> +: fdt-begin-node ( name namelen -- ) >> + OF_DT_BEGIN_NODE fdt-add-long >> + 2dup 1 = swap c@ 2F = and \ is it "/"? > > This would be ideal for COMPARE except that was never implemented ;-) > s" /" compare > (It's also probably more readable to write [char] / instead of 2F, > if you don't want to use the string words). Sure, it has just been a while since I touched forth :) > > 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. Thanks for the comment. Are you still going to do the full review or I can respin? :) 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.
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-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. > 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. Segher
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.
On Mon, Oct 02, 2017 at 10:09:52PM +1100, Alexey Kardashevskiy wrote: > > 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 I don't see it, but I haven't tested it either, just typed it :-) : fdt-str, ( str len -- ) fdt-here over ( str len fhere len ) fdt-allot ( str len fhere ) swap ( str fhere len ) move ( -- ) ; Looks fine to me? > >> 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? If you think those suggestions help, then yes please. > > 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. You might need to make a case-sensitive wordlist search then? Segher
On 03/10/17 02:12, Segher Boessenkool wrote: > On Mon, Oct 02, 2017 at 10:09:52PM +1100, Alexey Kardashevskiy wrote: >>> 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 > > I don't see it, but I haven't tested it either, just typed it :-) > > : fdt-str, ( str len -- ) > fdt-here over ( str len fhere len ) > fdt-allot ( str len fhere ) > swap ( str fhere len ) > move ( -- ) > ; > > Looks fine to me? > >>>> 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? > > If you think those suggestions help, then yes please. I'd rather follow the style and who knows it better than you :) v3 is coming soon. >>> 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. > > You might need to make a case-sensitive wordlist search then? I have no idea, any example? I'll probably skip this bit this time though.
On Tue, Oct 03, 2017 at 03:33:16PM +1100, Alexey Kardashevskiy wrote: > On 03/10/17 02:12, Segher Boessenkool wrote: > > On Mon, Oct 02, 2017 at 10:09:52PM +1100, Alexey Kardashevskiy wrote: > >>> : 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 > > > > I don't see it, but I haven't tested it either, just typed it :-) > > > > : fdt-str, ( str len -- ) > > fdt-here over ( str len fhere len ) > > fdt-allot ( str len fhere ) > > swap ( str fhere len ) > > move ( -- ) > > ; > > > > Looks fine to me? Well, : fdt-str, ( str len -- ) dup fdt-allot fdt-here swap move ; is probably clearer :-) > >>> 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. > > > > You might need to make a case-sensitive wordlist search then? > > I have no idea, any example? I'll probably skip this bit this time though. Currently deep down all wordlists use string=ci (that is, case-insensitive string compare). I think for the FDT string table you will want case- sensitive instead. (When searching the Forth dictionaries Open Firmware requires case- insensitive; and upper case is not allowed in property names, so doing a case-insensitive search works there, too (at least in practice, even though people *do* use upper case in property names). One option is to set some global flag before finding a word; another is to have a separate word for case-sentive finding; yet another is to set a flag in the wordlist itself saying if it should be case-sensitive. That last one is probably the most useful for other purposes, too. Segher
On Wed, Oct 04, 2017 at 08:39:59AM -0500, Segher Boessenkool wrote: > On Tue, Oct 03, 2017 at 03:33:16PM +1100, Alexey Kardashevskiy wrote: > > On 03/10/17 02:12, Segher Boessenkool wrote: > > > On Mon, Oct 02, 2017 at 10:09:52PM +1100, Alexey Kardashevskiy wrote: > > >>> : 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 > > > > > > I don't see it, but I haven't tested it either, just typed it :-) > > > > > > : fdt-str, ( str len -- ) > > > fdt-here over ( str len fhere len ) > > > fdt-allot ( str len fhere ) > > > swap ( str fhere len ) > > > move ( -- ) > > > ; > > > > > > Looks fine to me? > > Well, > > : fdt-str, ( str len -- ) dup fdt-allot fdt-here swap move ; > > is probably clearer :-) > > > >>> 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. > > > > > > You might need to make a case-sensitive wordlist search then? > > > > I have no idea, any example? I'll probably skip this bit this time though. > > Currently deep down all wordlists use string=ci (that is, case-insensitive > string compare). I think for the FDT string table you will want case- > sensitive instead. Yes, absolutely. > (When searching the Forth dictionaries Open Firmware requires case- > insensitive; and upper case is not allowed in property names, so doing > a case-insensitive search works there, too (at least in practice, even > though people *do* use upper case in property names). Right. There are, alas, a number of things that aren't allowed which are nonetheless found in the wild. Plus, a case insensitve comparison should be faster. > One option is to set some global flag before finding a word; another is > to have a separate word for case-sentive finding; yet another is to set > a flag in the wordlist itself saying if it should be case-sensitive. > That last one is probably the most useful for other purposes, too. > > > Segher > _______________________________________________ > SLOF mailing list > SLOF@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/slof
diff --git a/lib/libhvcall/libhvcall.h b/lib/libhvcall/libhvcall.h index 5776a2b..1c2d31b 100644 --- a/lib/libhvcall/libhvcall.h +++ b/lib/libhvcall/libhvcall.h @@ -26,6 +26,7 @@ #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2) #define KVMPPC_H_RTAS_UPDATE (KVMPPC_HCALL_BASE + 0x3) #define KVMPPC_H_UPDATE_PHANDLE (KVMPPC_HCALL_BASE + 0x4) +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x5) #ifndef __ASSEMBLY__ diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs index a24e344..b99746a 100644 --- a/board-qemu/slof/fdt.fs +++ b/board-qemu/slof/fdt.fs @@ -27,7 +27,7 @@ struct 4 field >fdth_boot_cpu 4 field >fdth_string_size 4 field >fdth_struct_size -drop +constant /fdth h# d00dfeed constant OF_DT_HEADER h# 1 constant OF_DT_BEGIN_NODE @@ -449,4 +449,226 @@ r> drop fdt-cas-fix? ; +VARIABLE fdt-struct +VARIABLE fdt-struct-cur +VARIABLE fdt-strings +VARIABLE fdt-strings-cur +VARIABLE fdt-strings-reused +VARIABLE fdt-ms + +: 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 ! +; + +: fdt-add-long ( token -- ) + fdt-struct-cur @ + l! + fdt-struct-cur @ + 4 + + fdt-struct-cur ! +; + +: skip-to-next ( cur -- cur ) + BEGIN + dup c@ + WHILE + 1+ + REPEAT + 4 + -4 and +; + +: string-equal ( name namelen str -- true | false ) + 3dup swap comp 0 <> IF + 3drop + false + EXIT + THEN + + c@ 0 = nip + + dup IF + fdt-strings-reused @ 1+ fdt-strings-reused ! + THEN +; + +: fdt-get-string ( name namelen -- nameoff ) + \ lookup + fdt-strings @ + BEGIN + dup fdt-strings-cur @ + = not + WHILE + 3dup string-equal IF + \ found it, no need to add new one + fdt-strings @ - + -rot + 2drop + EXIT + THEN + skip-to-next + REPEAT + drop + + \ store cur offset + fdt-strings-cur @ fdt-strings @ - >r + + \ store len + dup >r + + \ copy string + fdt-strings-cur @ swap ( name cur namelen ) + move + + \ increment cur and null terminate + fdt-strings-cur @ + r> + + 0 over c! + 4 + -4 and + fdt-strings-cur ! + + \ return offset + r> +; + +: fdt-begin-node ( name namelen -- ) + OF_DT_BEGIN_NODE fdt-add-long + 2dup 1 = swap c@ 2F = and \ is it "/"? + IF + 2drop s" " \ dtc is still unhappy though + THEN + true fdt-struct-add +; + +: fdt-end-node ( -- ) + OF_DT_END_NODE fdt-add-long +; + +: fdt-prop ( prop len name namelen -- ) + OF_DT_PROP fdt-add-long + + \ get string offset + fdt-get-string ( prop len nameoff ) + + \ store len and nameoff + over fdt-add-long + fdt-add-long ( prop len ) + + \ now add the bytes + false fdt-struct-add +; + +: fdt-end ( -- ) + OF_DT_END fdt-add-long +; + +: fdt-properties ( phandle -- ) + >r + s" " + BEGIN + r@ next-property + WHILE + 2dup + 2dup r@ get-property + not IF + 2swap fdt-prop + THEN + REPEAT + r> + drop +; + +: fdt-flatten-node ( node -- ) +\ fdt-debug IF dup node>path type cr THEN + dup node>qname fdt-begin-node + dup fdt-properties + child + BEGIN + dup + WHILE + dup recurse + peer + REPEAT + drop + fdt-end-node +; + +: fdt-flatten-tree ( root -- tree ) + 200000 alloc-mem dup fdt-struct-cur ! fdt-struct ! + 10000 alloc-mem dup fdt-strings-cur ! fdt-strings ! + 0 fdt-strings-reused ! + milliseconds fdt-ms ! + + fdt-flatten-node + fdt-end + + fdt-struct-cur @ fdt-struct @ - + fdt-strings-cur @ fdt-strings @ - + fdt-debug IF + 2dup + ." Strings=" .d cr + ." Struct=" .d cr + ." Reused strings=" fdt-strings-reused @ .d cr + milliseconds fdt-ms @ - + ." Took " .d ." ms" cr + THEN + + 2dup + /fdth + + 10 + \ reserved block + dup + fdt-debug IF + dup ." FDT flat size=" .d cr + THEN + alloc-mem ( struct-len strings-len totallen fdt ) + >r ( struct-len strings-len totallen r: fdt ) + + \ write header + OF_DT_HEADER r@ >fdth_magic l! + dup r@ >fdth_tsize l! + /fdth 10 + 2 pick + r@ >fdth_struct_off l! + /fdth 10 + r@ >fdth_string_off l! + /fdth r@ >fdth_rsvmap_off l! + 17 r@ >fdth_version l! + 17 r@ >fdth_compat_vers l! + 0 r@ >fdth_boot_cpu l! + over r@ >fdth_string_size l! + 2 pick r@ >fdth_struct_size l! + + drop + r@ /fdth + ( struct-len strings-len curfdt r: fdt ) + + \ write reserve entry + 0 over ! + cell+ + 0 over ! + cell+ + + \ copy string + 2dup fdt-strings @ swap rot move + + ( struct-len curfdt r: fdt ) + + \ copy struct + fdt-struct @ swap rot move ( r: fdt ) + + \ cleanup + fdt-struct @ 200000 free-mem + fdt-strings @ 10000 free-mem + + \ return fdt + r> +; + +: fdt-flatten-tree-free ( tree ) + dup >fdth_tsize l@ free-mem +; + s" /" find-node fdt-fix-phandles diff --git a/board-qemu/slof/rtas.fs b/board-qemu/slof/rtas.fs index 54d3929..5beb079 100644 --- a/board-qemu/slof/rtas.fs +++ b/board-qemu/slof/rtas.fs @@ -98,6 +98,10 @@ find-qemu-rtas ; : rtas-quiesce ( -- ) + " /" find-node + fdt-flatten-tree + dup hv-update-dt + fdt-flatten-tree-free " quiesce" rtas-get-token rtas-cb rtas>token l! 0 rtas-cb rtas>nargs l! 0 rtas-cb rtas>nret l! diff --git a/lib/libhvcall/hvcall.code b/lib/libhvcall/hvcall.code index 8349748..6ff5715 100644 --- a/lib/libhvcall/hvcall.code +++ b/lib/libhvcall/hvcall.code @@ -136,3 +136,8 @@ PRIM(hv_X2d_update_X2d_phandle) uint32_t old_phandle = TOS.u; TOS.u = hv_generic(KVMPPC_H_UPDATE_PHANDLE, old_phandle, new_phandle); MIRP + +PRIM(hv_X2d_update_X2d_dt) + unsigned long dt = TOS.u; + TOS.u = hv_generic(KVMPPC_H_UPDATE_DT, dt); +MIRP diff --git a/lib/libhvcall/hvcall.in b/lib/libhvcall/hvcall.in index ab7513a..b59e3f7 100644 --- a/lib/libhvcall/hvcall.in +++ b/lib/libhvcall/hvcall.in @@ -32,4 +32,5 @@ cod(hv-logical-memop) cod(hv-cas) cod(hv-rtas-update) cod(hv-update-phandle) +cod(hv-update-dt) cod(get-print-version)
This creates flatten device tree and passes it to QEMU via a custom hypercall right before jumping to RTAS. On a machine with 256 CPUs and 256 virtual Intel E1000 devices the blob is 360KB (356KB structs and 4KB of strings), building such a tree takes 2.8s on a POWER8 box. A simple tree with 1 CPU and a couple of devices takes 43ms and creates 16KB blob. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- I tested the blob by storing it from QEMU to a file (RFC patch is coming next) and decompiling it; this produces error which I do not really understand as the name of the root is an empty string (literaly: 00 00 00 01 00 00 00 00) and yet this error: aik@fstn1-p1:~$ dtc -f -I dtb -O dts -o dbg.dts dbg.dtb ERROR (name_properties): "name" property in / is incorrect ("/" instead of base node name) Warning: Input tree has errors, output forced --- lib/libhvcall/libhvcall.h | 1 + board-qemu/slof/fdt.fs | 224 +++++++++++++++++++++++++++++++++++++++++++++- board-qemu/slof/rtas.fs | 4 + lib/libhvcall/hvcall.code | 5 ++ lib/libhvcall/hvcall.in | 1 + 5 files changed, 234 insertions(+), 1 deletion(-)