[SLOF] [PATCH slof] fdt: Fix creating new nodes at H_CAS
Greg Kurz
groug at kaod.org
Thu Jan 30 19:13:49 AEDT 2020
On Thu, 30 Jan 2020 10:09:56 +1100
Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
>
>
> On 29/01/2020 22:33, Greg Kurz wrote:
> > On Wed, 29 Jan 2020 13:23:16 +1100
> > Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
> >
> >> So far we only allowed new ibm,dynamic-reconfiguration-memory and memory
> >> nodes in the FDT update blob at ibm,client-architecture-support (CAS).
> >> DRC do not have unit addresses and are easy, for memory nodee we use
> >> an address from the node name.
> >>
> >> For early hot plugged PCI devices (plugged after reset but before CAS)
> >> we have to have a similar hack as for memory@ but parse the address
> >> differently because of different binding.
> >>
> >> Instead, this changes new nodes creation. At pass#0 when we copy phandles
> >> from the FDT update blob to SLOF, we create new nodes with all
> >> new properties and call "finish-device" only after all properties are
> >> copied to the new nodes. At this point we particularly care about "reg"
> >> as this is the unit address which SLOF parses for us and sets the unit
> >> address in "finish-device"; we could skip other properties for later
> >
> > "finish-device" only sets the first entry of the "reg" property as a
> > fallback. This will be an issue when we start seeing new nodes with
> > bigger unit values (eg, a new PHB).
> >
> > The setting of the unit from "reg" is actually handled by
> > "fdt-unflatten-node" which calls "fdt-reg-unit":
> >
> > 2dup s" reg" str= IF
> > 2swap 2dup fdt-reg-unit 2swap
> > THEN
> >
> > Something similar could be done...
>
> Huh. Should not we then fix "finish-device"?
>
According to the commit that adds unit setting to "finish-device":
commit 82d381b5cffbeac520b33e2c5f907fd018226696
Author: Thomas Huth <thuth at linux.vnet.ibm.com>
Date: Thu Dec 15 15:41:34 2011 +0100
Two minor improvements for the device tree node code
...
Second fix is about "my-unit". According to IEEE1275, this value should be
initialized to the first component of the "reg" property in case it has not
been specified by other means.
So I'm not sure we should replace this fallback behavior described in
the spec with anything else.
>
> >
> >> passes.
> >>
> >> Note this creates naked nodes with no methods normally added to the nodes
> >> as this bypasses normal discovery which SLOF performs at start. So
> >> if pass#1 does not find the node created in pass#0, this points to
> >> missing "decode-unit" at the new node parent (happens when adding bridge-
> >> under-bridge) and this prints a message and resets.
> >>
> >> While at this, fix few trailing spaces and comments.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> >> ---
> >> board-qemu/slof/fdt.fs | 100 ++++++++++++++++++++++-------------------
> >> 1 file changed, 54 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> >> index fc39a82a10b5..eb726cc990ae 100644
> >> --- a/board-qemu/slof/fdt.fs
> >> +++ b/board-qemu/slof/fdt.fs
> >> @@ -112,7 +112,7 @@ fdt-check-header
> >> ;
> >>
> >> \ Lookup a string by index
> >> -: fdt-fetch-string ( index -- str-addr str-len )
> >> +: fdt-fetch-string ( index -- str-addr str-len )
> >> fdt-strings + dup from-cstring
> >> ;
> >>
> >> @@ -128,7 +128,7 @@ fdt-check-header
> >> ;
> >>
> >> \ Encode fdt property to OF property
> >> -: fdt-encode-prop ( addr len -- )
> >> +: fdt-encode-prop ( addr len -- pa ps )
> >> 2dup fdt-prop-is-string? IF
> >> 1- encode-string
> >> ELSE
> >> @@ -443,33 +443,6 @@ r> drop
> >> device-end
> >> ;
> >>
> >> -: fdt-create-cas-node ( name -- )
> >> - 2dup
> >> - 2dup " memory@" find-substr 0 = IF
> >> - fdt-debug IF ." Creating memory@ " cr THEN
> >> - new-device
> >> - 2dup " @" find-substr nip device-name \ Parse the node name
> >> - 2dup
> >> - 2dup " @" find-substr rot over + 1 + -rot - 1 - \ Jump to addr afte "@"
> >> - parse-2int nip xlsplit set-unit \ Parse and set unit
> >> - finish-device
> >> - ELSE
> >> - 2dup " ibm,dynamic-reconfiguration-memory" find-substr 0 = IF
> >> - fdt-debug IF ." Creating ibm,dynamic-reconfiguration-memory " cr THEN
> >> - new-device
> >> - device-name
> >> - finish-device
> >> - ELSE
> >> - 2drop 2drop
> >> - false to fdt-cas-fix?
> >> - ." Node not supported " cr
> >> - EXIT
> >> - THEN
> >> - THEN
> >> -
> >> - find-node ?dup 0 <> IF set-node THEN
> >> -;
> >> -
> >> : str=phandle? ( s len -- true|false )
> >> 2dup s" phandle" str= >r
> >> s" linux,phandle" str=
> >> @@ -483,7 +456,7 @@ r> drop
> >> false to fdt-cas-fix?
> >> EXIT
> >> THEN drop
> >> - fdt-fetch-unit
> >> + fdt-fetch-unit ( a1 $name )
> >> dup 0 = IF drop drop " /" THEN
> >> 40 left-parse-string
> >> 2swap ?dup 0 <> IF
> >> @@ -492,29 +465,52 @@ r> drop
> >> ELSE
> >> drop
> >> THEN
> >> - fdt-debug IF ." Setting node: " 2dup type cr THEN
> >> 2dup find-node ?dup 0 <> IF
> >> - set-node 2drop
> >> + set-node
> >> + fdt-debug IF ." Setting node: " 2dup type cr THEN
> >> + 2drop
> >> + \ newnode?=0: updating the existing node, i.e. pass1 adds only phandles
> >> + 0
> >> ELSE
> >> + fdt-cas-pass 0 <> IF
> >> + \ We could not find the node added in the previous pass,
> >> + \ most likely because it is hotplug-under-hotplug case
> >> + \ (such as PCI brigde under bridge) when missing new node methods
> >> + \ such as "decode-unit" are critical.
> >> + \ Reboot when detect such case which is expected as it is a part of
> >> + \ ibm,client-architecture-support.
> >> + ." Cannot handle FDT update for the " 2dup type
> >> + ." node, rebooting" cr
> >> + reset-all
> >> + THEN
> >> fdt-debug IF ." Creating node: " 2dup type cr THEN
> >> - fdt-create-cas-node
> >> + new-device
> >> + 2dup " @" find-substr nip
> >> + device-name
> >> + \ newnode?=1: adding new node, i.e. pass1 adds all properties,
> >> + \ most importantly "reg". After reading properties, we call
> >> + \ "finish-device" which sets the unit address from "reg".
> >> + 1
> >> THEN
> >> - fdt-debug IF ." Current now: " pwd cr THEN
> >> + swap ( newnode? a1 )
> >> +
> >> + fdt-debug IF ." Current now: " pwd get-node ." = " . cr THEN
> >> BEGIN
> >> fdt-next-tag dup OF_DT_END_NODE <>
> >> WHILE
> >> + ( newnode? a1 tag )
> >> dup OF_DT_PROP = IF
> >> - drop dup ( drop tag, dup addr : a1 a1 )
> >> - dup l@ dup rot 4 + ( fetch size, stack is : a1 s s a2)
> >> - dup l@ swap 4 + ( fetch nameid, stack is : a1 s s i a3 )
> >> - rot ( we now have: a1 s i a3 s )
> >> - fdt-encode-prop rot ( a1 s pa ps i)
> >> - fdt-fetch-string ( a1 s pa ps na ns )
> >> + drop dup ( newnode? a1 a1 )
> >> + dup l@ dup rot 4 + ( newnode? a1 s s a2)
> >> + dup l@ swap 4 + ( newnode? a1 s s i a3 )
> >> + rot ( newnode? a1 s i a3 s )
> >> + fdt-encode-prop rot ( newnode? a1 s pa ps i)
> >> + fdt-fetch-string ( newnode? a1 s pa ps na ns )
> >>
> >> fdt-cas-pass CASE
> >> 0 OF
> >> - 2dup str=phandle? IF
> >> - fdt-debug IF 4dup ." Phandle: " type ." =" swap ." @" . ." " .d ." bytes" cr THEN
> >> + 2dup str=phandle? 7 pick or IF
> >> + fdt-debug IF 4dup ." Property: " type ." =" swap ." @" . ." " .d ." bytes" cr THEN
> >> property
> >> ELSE
> >> 4drop
> >> @@ -541,8 +537,14 @@ r> drop
> >> ENDCASE
> >>
> >> + 8 + 3 + fffffffc and
> >> - ELSE dup OF_DT_BEGIN_NODE = IF
> >> - drop ( drop tag )
> >> + ELSE ( newnode? a1 tag )
> >> + dup OF_DT_BEGIN_NODE = IF
> >> + 2 pick IF
> >> + rot drop 0 -rot
>
>
> The proposed change from below is also needed here then, or since it has
> grown enough, this can make a separate function then.
>
Yeah, I realized that after posting this patch. We can probably come up
with a word that can be used in both the _node_ and _subnode_ cases.
> Can you please take over this and finish the change (merge yours 1/2
> into this one and I will comment on 2/2)? Thanks,
>
Sure. This will give you cycles to make progress on killing SLOF :-P
>
> >> + get-node finish-device set-node
> >> + fdt-debug IF ." Finished node: " pwd get-node ." = " . cr THEN
> >> + THEN
> >> + drop ( a1 )
> >> 4 -
> >> (fdt-fix-cas-node)
> >> get-parent set-node
> >> @@ -554,13 +556,19 @@ r> drop
> >> THEN
> >> THEN
> >> REPEAT
> >> - drop \ drop tag
> >> + ( newnode? a1 tag )
> >> + drop
> >> + swap ( a1 newnode? )
> >> + IF
> >
> > ... here.
> >
> > " reg" get-node get-package-property IF ELSE fdt-reg-unit THEN
> >
> > This doesn't address the case of a hotplug-over-hotplug since the
> > new parent node doesn't have a "decode-unit" method, but at least
> > it covers all cases where the parent node was created at boot time.
> >
> >> + get-node finish-device set-node
> >> + fdt-debug IF ." Finished subnode: " pwd get-node ." = " . cr THEN
> >> + THEN
> >> ;
> >>
> >> : fdt-fix-cas-node ( start -- )
> >> 0 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Add phandles
> >> 1 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Patch+add other properties
> >> - 2 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Delete phandles from pass 1
> >> + 2 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Delete phandles from pass 0
> >> drop
> >> ;
> >>
> >
>
More information about the SLOF
mailing list