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

David Gibson david at gibson.dropbear.id.au
Tue Oct 3 17:28:43 AEDT 2017


On Tue, Oct 03, 2017 at 04:15:23PM +1100, Alexey Kardashevskiy wrote:
> This creates flatten device tree and passes it to QEMU via a custom
> hypercall right before jumping to RTAS.

Uh.. "right before jumping to RTAS" isn't very clear to me.  It's
called at quiesce time, right, which we anticipate is the last thing
SLOF will do before being wiped away by the OS.  That's not really
connected to RTAS AFAICT.

> On a machine with 256 CPUs and 256 virtual Intel E1000 devices the blob
> is 360KB (356KB structs and 20KB of strings), building such a tree takes
> ~2s on a POWER8 box. A simple tree with 1 CPU and a couple of devices
> takes 38ms and creates 16KB blob.

Hrm.  360/16 * 38ms = 855ms < 1s.  Which suggests we have something
that's slower than O(n) here, which is concerning.  256 cpus and 256
devices is largish, but it's not a ludicrous size.

> This preloads strings with 40 property names from CPU and PCI device nodes
> and the strings lookup only searches within these. Without string reusing
> at all, the strings blob is 200KB and rendering time is 1.7sec; with
> unlimited reusing, the strings blob is 4KB and rendering time is
> 2.8sec.

Blech, that's a prety ugly set of tradeoffs.  When I suggested this
approach I hadn't thought it would take so long to flatten the tree in
Forth.

> 
> While we are here, fix the version handling in fdt-init. It only matters
> a little for the fdt-debug==1 case though.
> 
> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> ---
> 
> Changes:
> v3:
> * fixed stack handling after hcall returned
> * fixed format versions in both rendering and parsing paths
> * rebased on top of removed unused hvcalls
> * renamed used variables to have fdtfl- prefixes as there are already
> some for parsing the initial dt
> 
> v2:
> * fixed comments from review
> * added strings cache
> * changed last_compat_vers from 0x17 to 0x16 as suggested by dwg
> 
> ---
> 
> I tested the blob by storing it from QEMU to a file 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 at 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

The reason for this error is it's not to do with the "node name" as
given after the FDT_BEGIN_TAG, but with the "name" property.  That's
not required in FDT, but if supplied should match that node name
exactly.  IIRC, however, for human readability runtime OF treats
'name' of the root node as being '/'.

Arguably that is a bug in dtc - '/' as the name property in the root
is probably acceptable.  On the other hand, you should probably just
omit the 'name' properties when you flatten the tree.

> ---
>  lib/libhvcall/libhvcall.h |   3 +-
>  board-qemu/slof/fdt.fs    | 284 +++++++++++++++++++++++++++++++++++++++++++++-
>  board-qemu/slof/rtas.fs   |   7 ++
>  lib/libhvcall/hvcall.code |   5 +
>  lib/libhvcall/hvcall.in   |   1 +
>  5 files changed, 297 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/libhvcall/libhvcall.h b/lib/libhvcall/libhvcall.h
> index 6356a62..3fa4398 100644
> --- a/lib/libhvcall/libhvcall.h
> +++ b/lib/libhvcall/libhvcall.h
> @@ -24,7 +24,8 @@
>  #define KVMPPC_H_LOGICAL_MEMOP  (KVMPPC_HCALL_BASE + 0x1)
>  /* Client Architecture support */
>  #define KVMPPC_H_CAS            (KVMPPC_HCALL_BASE + 0x2)
> -#define KVMPPC_HCALL_MAX        KVMPPC_H_CAS
> +#define KVMPPC_H_UPDATE_DT      (KVMPPC_HCALL_BASE + 0x3)
> +#define KVMPPC_HCALL_MAX        KVMPPC_H_UPDATE_DT
>  
>  #ifndef __ASSEMBLY__
>  
> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> index 851645e..548cc25 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
> @@ -69,7 +69,7 @@ fdt-start fdt-init
>          dup >fdth_version l@ 3 >= IF
>              ."  strings size     : 0x" dup >fdth_string_size l@ . cr
>          THEN
> -        dup >fdth_version l@ 17 >= IF
> +        dup >fdth_version l@ 11 >= IF
>              ."  struct size      : 0x" dup >fdth_struct_size l@ . cr
>          THEN
>      THEN
> @@ -439,4 +439,284 @@ r> drop
>      fdt-cas-fix?
>  ;
>  
> +VARIABLE fdtfl-struct
> +VARIABLE fdtfl-struct-here
> +VARIABLE fdtfl-strings
> +VARIABLE fdtfl-strings-cache
> +VARIABLE fdtfl-strings-here
> +VARIABLE fdtfl-strings-reused \ debug only
> +VARIABLE fdt-ms \ debug only
> +
> +: fdt-skip-string ( cur -- cur )
> +    BEGIN
> +        dup c@
> +    WHILE
> +        1+
> +    REPEAT
> +    4 + -4 and
> +;
> +
> +: zstring=  ( str len zstr -- flag )
> +    2dup + c@ 0<> IF
> +        3drop false
> +        EXIT
> +    THEN
> +    swap comp 0=
> +;
> +
> +: fdt-find-string ( name namelen -- nameoff true | false )
> +    fdtfl-strings @
> +    BEGIN
> +        dup fdtfl-strings-cache @ <
> +    WHILE
> +        3dup zstring= IF
> +            fdtfl-strings @ -
> +            -rot
> +            2drop
> +            true
> +            EXIT
> +        THEN
> +        fdt-skip-string
> +    REPEAT
> +    3drop
> +    false
> +;
> +
> +: fdt-str-allot ( len -- ) fdtfl-strings-here @ + to fdtfl-strings-here ;
> +: fdt-str-c, ( char -- ) fdtfl-strings-here @ 1 fdt-str-allot c! ;
> +: fdt-str-align  ( -- )
> +    fdtfl-strings-here @
> +    dup dup 4 #aligned swap -   ( here bytes-to-erase )
> +    dup -rot
> +    erase
> +    fdt-str-allot
> +;
> +: fdt-str-bytes, ( data len -- ) fdtfl-strings-here @ over fdt-str-allot swap move ;
> +: fdt-str-ztr, ( str len -- ) fdt-str-bytes, 0 fdt-str-c, ;
> +
> +: fdt-add-string ( name namelen -- nameoff )
> +    fdtfl-strings-here @ -rot
> +    fdt-str-ztr,
> +    fdt-str-align
> +    fdtfl-strings @ -
> +;
> +
> +: fdt-get-string ( name namelen -- nameoff )
> +    2dup fdt-find-string IF
> +        -rot 2drop
> +        fdt-debug IF
> +           1 fdtfl-strings-reused +!
> +        THEN
> +        EXIT
> +    THEN
> +    fdt-add-string
> +;
> +
> +: fdt-allot ( len -- ) fdtfl-struct-here @ + to fdtfl-struct-here ;
> +: fdt-c, ( char -- ) fdtfl-struct-here @ 1 fdt-allot c! ;
> +: fdt-align  ( -- )
> +    fdtfl-struct-here @
> +    dup dup 4 #aligned swap -   ( here bytes-to-erase )
> +    dup -rot
> +    erase
> +    fdt-allot
> +;
> +: fdt-bytes, ( data len -- ) fdtfl-struct-here @ over fdt-allot swap move ;
> +: fdt-ztr, ( str len -- ) fdt-bytes, 0 fdt-c, ;
> +: fdt-l, ( token -- ) fdtfl-struct-here @ l! /l fdt-allot ;
> +
> +: fdt-begin-node ( name namelen -- )
> +    OF_DT_BEGIN_NODE fdt-l,
> +    2dup 1 = swap c@ [char] / = and  \ is it "/"?
> +    IF
> +        2drop s" " \ dtc is still unhappy though
> +    THEN
> +    fdt-ztr,
> +    fdt-align
> +;
> +
> +: fdt-end-node ( -- ) OF_DT_END_NODE fdt-l, ;
> +
> +: fdt-prop ( prop len name namelen -- )
> +    OF_DT_PROP fdt-l,
> +
> +    \ get string offset
> +    fdt-get-string      ( prop len nameoff )
> +
> +    \ store len and nameoff
> +    over fdt-l,
> +    fdt-l,              ( prop len )
> +
> +    \ now store the bytes
> +    fdt-bytes,
> +    fdt-align
> +;
> +
> +: fdt-end ( -- ) OF_DT_END fdt-l, ;
> +
> +: fdt-properties ( phandle -- )
> +    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 1 > 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
> +;
> +
> +: fdtfl-strings-preload ( -- )
> +    s" reg" fdt-add-string drop
> +    s" status" fdt-add-string drop
> +    s" 64-bit" fdt-add-string drop
> +    s" phandle" fdt-add-string drop
> +    s" ibm,vmx" fdt-add-string drop
> +    s" ibm,dfp" fdt-add-string drop
> +    s" slb-size" fdt-add-string drop
> +    s" ibm,purr" fdt-add-string drop
> +    s" vendor-id" fdt-add-string drop
> +    s" device-id" fdt-add-string drop
> +    s" min-grant" fdt-add-string drop
> +    s" class-code" fdt-add-string drop
> +    s" compatible" fdt-add-string drop
> +    s" interrupts" fdt-add-string drop
> +    s" cpu-version" fdt-add-string drop
> +    s" #size-cells" fdt-add-string drop
> +    s" ibm,req#msi" fdt-add-string drop
> +    s" revision-id" fdt-add-string drop
> +    s" device_type" fdt-add-string drop
> +    s" max-latency" fdt-add-string drop
> +    s" ibm,chip-id" fdt-add-string drop
> +    s" ibm,pft-size" fdt-add-string drop
> +    s" ibm,slb-size" fdt-add-string drop
> +    s" devsel-speed" fdt-add-string drop
> +    s" ibm,loc-code" fdt-add-string drop
> +    s" subsystem-id" fdt-add-string drop
> +    s" d-cache-size" fdt-add-string drop
> +    s" i-cache-size" fdt-add-string drop
> +    s" #address-cells" fdt-add-string drop
> +    s" clock-frequency" fdt-add-string drop
> +    s" cache-line-size" fdt-add-string drop
> +    s" ibm,pa-features" fdt-add-string drop
> +    s" ibm,my-drc-index" fdt-add-string drop
> +    s" d-cache-line-size" fdt-add-string drop
> +    s" i-cache-line-size" fdt-add-string drop
> +    s" assigned-addresses" fdt-add-string drop
> +    s" d-cache-block-size" fdt-add-string drop
> +    s" i-cache-block-size" fdt-add-string drop
> +    s" timebase-frequency" fdt-add-string drop
> +    s" subsystem-vendor-id" fdt-add-string drop
> +    s" ibm,segment-page-sizes" fdt-add-string drop
> +    s" ibm,ppc-interrupt-server#s" fdt-add-string drop
> +    s" ibm,processor-segment-sizes" fdt-add-string drop
> +    s" ibm,ppc-interrupt-gserver#s" fdt-add-string drop
> +;
> +
> +: fdt-append-blob ( bytes cur blob -- cur )
> +    3dup -rot swap move
> +    drop +
> +;
> +
> +: fdt-flatten-tree ( root -- tree )
> +    200000 alloc-mem dup fdtfl-struct-here ! fdtfl-struct !
> +    200000 alloc-mem dup fdtfl-strings-here ! fdtfl-strings !
> +
> +    fdt-debug IF
> +        0 fdtfl-strings-reused !
> +        milliseconds fdt-ms !
> +    THEN
> +
> +    \ Preload strings cache
> +    fdtfl-strings-preload
> +    fdtfl-strings-here @ fdtfl-strings-cache !
> +    \ Render the blobs
> +    fdt-flatten-node
> +    fdt-end
> +
> +    \ Calculate strings and struct sizes
> +    fdtfl-struct-here @ fdtfl-struct @ -
> +    fdtfl-strings-here @ fdtfl-strings @ - ( struct-len strings-len )
> +
> +    2dup + /fdth +
> +    10 + \ Reserve 16 bytes and an empty reserved block
> +
> +    fdt-debug IF
> +        3dup
> +        ." FDT flat size=" .d cr
> +        ." Strings size=" .d cr
> +        ." Struct size=" .d cr
> +        ." Reused strings=" fdtfl-strings-reused @ .d cr
> +        milliseconds fdt-ms @ -
> +        ." Took " .d ." ms" cr
> +    THEN
> +
> +    \ Allocate flatten DT blob
> +    dup alloc-mem                   ( struct-len strings-len total-len fdt )
> +    >r                              ( struct-len strings-len total-len 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!
> +    11                  r@ >fdth_version l!
> +    10                  r@ >fdth_compat_vers l!
> +    0                   r@ >fdth_boot_cpu l!
> +    over                r@ >fdth_string_size l!
> +    2 pick              r@ >fdth_struct_size l!
> +                                    ( struct-len strings-len total-len r: fdt )
> +
> +    drop                            ( struct-len strings-len r: fdt )
> +    r@ /fdth +                      ( struct-len strings-len cur r: fdt )
> +
> +    \ Write the reserved entry
> +    0 over !
> +    cell+
> +    0 over !
> +    cell+                           ( struct-len strings-len cur r: fdt )
> +
> +    \ Write strings and struct blobs
> +    fdtfl-strings @ fdt-append-blob
> +    fdtfl-struct @ fdt-append-blob
> +    drop
> +
> +    \ Free temporary blobs
> +    fdtfl-struct @ 200000 free-mem
> +    fdtfl-strings @ 200000 free-mem
> +
> +    \ Return fdt
> +    r>
> +;
> +
> +: fdt-flatten-tree-free ( tree )
> +    dup >fdth_tsize l@ free-mem
> +;
> +
> +: fdt ( -- )
> +    " /" find-node
> +    fdt-flatten-tree
> +;
> +
>  s" /" find-node fdt-fix-phandles
> diff --git a/board-qemu/slof/rtas.fs b/board-qemu/slof/rtas.fs
> index b17157e..219bcda 100644
> --- a/board-qemu/slof/rtas.fs
> +++ b/board-qemu/slof/rtas.fs
> @@ -98,6 +98,13 @@ find-qemu-rtas
>  ;
>  
>  : rtas-quiesce ( -- )
> +    " /" find-node
> +    fdt-flatten-tree
> +    dup hv-update-dt ?dup IF
> +        \ Ignore hcall not implemented error, print error otherwise
> +        dup -2 <> IF ." HV-UPDATE-DT error: " . cr ELSE drop THEN
> +    THEN
> +    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 744469f..5918c90 100644
> --- a/lib/libhvcall/hvcall.code
> +++ b/lib/libhvcall/hvcall.code
> @@ -123,3 +123,8 @@ PRIM(check_X2d_and_X2d_patch_X2d_sc1)
>  
>  	patch_broken_sc1((void*)start, (void*)end, (void*)patch_ins);
>  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 e99d6d1..9193162 100644
> --- a/lib/libhvcall/hvcall.in
> +++ b/lib/libhvcall/hvcall.in
> @@ -30,4 +30,5 @@ cod(RX!)
>  
>  cod(hv-logical-memop)
>  cod(hv-cas)
> +cod(hv-update-dt)
>  cod(get-print-version)

-- 
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/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20171003/74fdd37f/attachment.sig>


More information about the SLOF mailing list