| Message ID | 158092690859.2344237.16184914364427726409.stgit@bahia.lan |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fdt: Fix creation/deletion of nodes at CAS | expand |
On 06/02/2020 05:21, Greg Kurz wrote: > We recently fixed node creation at CAS in order to support early hotplug of > devices between boot and CAS. Let's handle node removal now to support early > hot *un*plug of devices. > > This is done in three phases: > > 1) all nodes from the boot time FDT are added a "slof,present-at-boot" > property > > 2) during pass 0 of (fdt-fix-cas-node) all nodes from the CAS FDT are added > a "slof,present-at-cas" property > > 3) loop on all nodes in the DT and delete the ones that only have the > "slof,present-at-boot" property, and any alias they might have. In > the same time, "slof,present-at-cas" properties are converted into > "slof,present-at-boot" to allow further node removal in case of a > subsequent device unplug and CAS sequence before quiesce (linux > doesn't do that but it isn't explicitely forbidden by the spec). > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > board-qemu/slof/fdt.fs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 94 insertions(+) > > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > index 66f8fe74d9dd..1125f6702435 100644 > --- a/board-qemu/slof/fdt.fs > +++ b/board-qemu/slof/fdt.fs > @@ -198,6 +198,8 @@ fdt-check-header > decode-int dup fdt-create-dec fdt-create-enc 2drop > THEN > > + 0 0 s" slof,present-at-boot" property > + > finish-device > ; > > @@ -514,6 +516,9 @@ r> drop > swap ( newnode? a1 ) > > fdt-debug IF ." Current now: " pwd get-node ." = " . cr THEN > + fdt-cas-pass 0= IF > + 0 0 s" slof,present-at-cas" property > + THEN I wanted to say this will remove "disk" nodes from SCSI hosts but then I realized that "slof,present-at-boot" protects them. May be put this in the commit log? > BEGIN > fdt-next-tag dup OF_DT_END_NODE <> > WHILE > @@ -584,8 +589,97 @@ r> drop > THEN > ; > > +: alias-dev-path ( xt -- dev-path len ) > + link> execute decode-string 2swap 2drop > +; > + > +: alias-name ( xt -- alias-name len ) > + link> >name name>string > +; > + > +: fdt-cas-alias-obsolete? ( node xt -- true|false ) > + alias-dev-path rot node>path str= > +; > + > +: (fdt-cas-delete-obsolete-aliases) ( node xt -- ) > + dup IF > + dup @ 2 pick swap ( node xt node next-xt ) > + recurse ( node xt ) > + 2dup fdt-cas-alias-obsolete? IF > + dup alias-name ( node xt alias-name len ) > + fdt-debug IF ." Deleting alias " 2dup type cr THEN > + delete-property ( node xt ) > + THEN > + THEN > + 2drop > +; > + > +: fdt-cas-delete-obsolete-aliases ( node -- ) > + s" /aliases" find-node dup IF > + ( node aliases ) > + get-node >r > + dup set-node > + > + node>properties @ cell+ @ ( node xt ) > + over swap ( node node xt ) > + (fdt-cas-delete-obsolete-aliases) ( node ) I'd suggest clearing aliases in the end - simply walk through them and delete those which we fail to "find-path". > + > + r> set-node > + THEN > + drop > +; > + > +: fdt-cas-node-obsolete? ( node -- true|false) > + s" slof,present-at-cas" 2 pick get-package-property IF > + s" slof,present-at-boot" 2 pick get-package-property IF > + \ Node not coming from the QEMU > + FALSE > + ELSE > + \ Device got removed since last boot or CAS > + 2drop > + TRUE > + THEN > + ELSE > + 2drop ( node ) > + > + get-node >r > + dup set-node > + > + \ Any device present in the CAS FDT is now considered as if it was > + \ present at boot, ie. cold plugged. This allows a subsequent CAS > + \ to remove the node from the DT if the device gets unplugged in > + \ between). > + s" slof,present-at-cas" delete-property > + 0 0 s" slof,present-at-boot" property Uff. The "fdt-cas-node-obsolete?" suggests it does checking but the code actually changes the node. For the write only language we should be extra careful :) I'd probably make it one property - "slof,from-fdt" - and store "0" for "at-boot" and "1" for "at-cas" and would not bother with switching "at-cas" -> "at-boot". If the property is present, then we preserve it, that is all. > + > + r> set-node > + FALSE > + THEN > + nip > +; > + > +: (fdt-cas-search-obsolete-nodes) ( start node -- ) > + dup IF > + dup child 2 pick swap recurse > + dup peer 2 pick swap recurse ( start node ) > + > + fdt-debug IF dup ." Inspecting node: " dup .node ." = " . cr THEN > + dup fdt-cas-node-obsolete? IF > + fdt-debug IF ." Deleting node" cr THEN > + dup fdt-cas-delete-obsolete-aliases > + dup delete-node > + THEN > + THEN > + 2drop > +; > + > +: fdt-cas-delete-obsolete-nodes ( start -- ) > + s" /" find-device get-node (fdt-cas-search-obsolete-nodes) > +; > + > : fdt-fix-cas-node ( start -- ) > 0 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Add phandles > + dup fdt-cas-delete-obsolete-nodes \ Delete removed devices > 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 0 > drop >
On Fri, 7 Feb 2020 14:41:09 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > On 06/02/2020 05:21, Greg Kurz wrote: > > We recently fixed node creation at CAS in order to support early hotplug of > > devices between boot and CAS. Let's handle node removal now to support early > > hot *un*plug of devices. > > > > This is done in three phases: > > > > 1) all nodes from the boot time FDT are added a "slof,present-at-boot" > > property > > > > 2) during pass 0 of (fdt-fix-cas-node) all nodes from the CAS FDT are added > > a "slof,present-at-cas" property > > > > 3) loop on all nodes in the DT and delete the ones that only have the > > "slof,present-at-boot" property, and any alias they might have. In > > the same time, "slof,present-at-cas" properties are converted into > > "slof,present-at-boot" to allow further node removal in case of a > > subsequent device unplug and CAS sequence before quiesce (linux > > doesn't do that but it isn't explicitely forbidden by the spec). > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > board-qemu/slof/fdt.fs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 94 insertions(+) > > > > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > > index 66f8fe74d9dd..1125f6702435 100644 > > --- a/board-qemu/slof/fdt.fs > > +++ b/board-qemu/slof/fdt.fs > > @@ -198,6 +198,8 @@ fdt-check-header > > decode-int dup fdt-create-dec fdt-create-enc 2drop > > THEN > > > > + 0 0 s" slof,present-at-boot" property > > + > > finish-device > > ; > > > > @@ -514,6 +516,9 @@ r> drop > > swap ( newnode? a1 ) > > > > fdt-debug IF ." Current now: " pwd get-node ." = " . cr THEN > > + fdt-cas-pass 0= IF > > + 0 0 s" slof,present-at-cas" property > > + THEN > > I wanted to say this will remove "disk" nodes from SCSI hosts but then I > realized that "slof,present-at-boot" protects them. May be put this in > the commit log? Yeah, the logic is to remove QEMU's FDT originated nodes only, if they have "slof,present-at-boot" but not "slof,present-at-cas"... Are you suggesting that I should emphasize this or are you thinking to something else I might have missed with SCSI hosts ? > > > > > BEGIN > > fdt-next-tag dup OF_DT_END_NODE <> > > WHILE > > @@ -584,8 +589,97 @@ r> drop > > THEN > > ; > > > > +: alias-dev-path ( xt -- dev-path len ) > > + link> execute decode-string 2swap 2drop > > +; > > + > > +: alias-name ( xt -- alias-name len ) > > + link> >name name>string > > +; > > + > > +: fdt-cas-alias-obsolete? ( node xt -- true|false ) > > + alias-dev-path rot node>path str= > > +; > > + > > +: (fdt-cas-delete-obsolete-aliases) ( node xt -- ) > > + dup IF > > + dup @ 2 pick swap ( node xt node next-xt ) > > + recurse ( node xt ) > > + 2dup fdt-cas-alias-obsolete? IF > > + dup alias-name ( node xt alias-name len ) > > + fdt-debug IF ." Deleting alias " 2dup type cr THEN > > + delete-property ( node xt ) > > + THEN > > + THEN > > + 2drop > > +; > > + > > +: fdt-cas-delete-obsolete-aliases ( node -- ) > > + s" /aliases" find-node dup IF > > + ( node aliases ) > > + get-node >r > > + dup set-node > > + > > + node>properties @ cell+ @ ( node xt ) > > + over swap ( node node xt ) > > + (fdt-cas-delete-obsolete-aliases) ( node ) > > > I'd suggest clearing aliases in the end - simply walk through them and > delete those which we fail to "find-path". > Ok, this will probably simplify the code. > > > + > > + r> set-node > > + THEN > > + drop > > +; > > + > > +: fdt-cas-node-obsolete? ( node -- true|false) > > + s" slof,present-at-cas" 2 pick get-package-property IF > > + s" slof,present-at-boot" 2 pick get-package-property IF > > + \ Node not coming from the QEMU > > + FALSE > > + ELSE > > + \ Device got removed since last boot or CAS > > + 2drop > > + TRUE > > + THEN > > + ELSE > > + 2drop ( node ) > > + > > + get-node >r > > + dup set-node > > + > > + \ Any device present in the CAS FDT is now considered as if it was > > + \ present at boot, ie. cold plugged. This allows a subsequent CAS > > + \ to remove the node from the DT if the device gets unplugged in > > + \ between). > > + s" slof,present-at-cas" delete-property > > + 0 0 s" slof,present-at-boot" property > > Uff. The "fdt-cas-node-obsolete?" suggests it does checking but the code > actually changes the node. For the write only language we should be > extra careful :) > Heh... I must confess I was kinda expecting this comment would pop up. > I'd probably make it one property - "slof,from-fdt" - and store "0" for > "at-boot" and "1" for "at-cas" and would not bother with switching > "at-cas" -> "at-boot". If the property is present, then we preserve it, > that is all. > The "at-cas" -> "at-boot" switching was done so that the deleting logic still works if the guest calls CAS again before quiesce. Linux doesn't do that but since it isn't explicitly forbidden by spec AFAIK, who knows what another OS can decide to do ? Following your suggestion, maybe increment "slof,from-fdt" at CAS and introduce a "slof,fdt-counter" in " /", set to 0 during boot and also incremented at CAS. Then we would delete nodes for which "slof,from-fdt" is less than "slof,fdt-counter" ? And of course, don't hijack the "fdt-cas-node-obsolete?" for that :) > > > > + > > + r> set-node > > + FALSE > > + THEN > > + nip > > +; > > + > > +: (fdt-cas-search-obsolete-nodes) ( start node -- ) > > + dup IF > > + dup child 2 pick swap recurse > > + dup peer 2 pick swap recurse ( start node ) > > + > > + fdt-debug IF dup ." Inspecting node: " dup .node ." = " . cr THEN > > + dup fdt-cas-node-obsolete? IF > > + fdt-debug IF ." Deleting node" cr THEN > > + dup fdt-cas-delete-obsolete-aliases > > + dup delete-node > > + THEN > > + THEN > > + 2drop > > +; > > + > > +: fdt-cas-delete-obsolete-nodes ( start -- ) > > + s" /" find-device get-node (fdt-cas-search-obsolete-nodes) > > +; > > + > > : fdt-fix-cas-node ( start -- ) > > 0 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Add phandles > > + dup fdt-cas-delete-obsolete-nodes \ Delete removed devices > > 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 0 > > drop > > >
On 07/02/2020 20:09, Greg Kurz wrote: > On Fri, 7 Feb 2020 14:41:09 +1100 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > >> >> >> On 06/02/2020 05:21, Greg Kurz wrote: >>> We recently fixed node creation at CAS in order to support early hotplug of >>> devices between boot and CAS. Let's handle node removal now to support early >>> hot *un*plug of devices. >>> >>> This is done in three phases: >>> >>> 1) all nodes from the boot time FDT are added a "slof,present-at-boot" >>> property >>> >>> 2) during pass 0 of (fdt-fix-cas-node) all nodes from the CAS FDT are added >>> a "slof,present-at-cas" property >>> >>> 3) loop on all nodes in the DT and delete the ones that only have the >>> "slof,present-at-boot" property, and any alias they might have. In >>> the same time, "slof,present-at-cas" properties are converted into >>> "slof,present-at-boot" to allow further node removal in case of a >>> subsequent device unplug and CAS sequence before quiesce (linux >>> doesn't do that but it isn't explicitely forbidden by the spec). >>> >>> Signed-off-by: Greg Kurz <groug@kaod.org> >>> --- >>> board-qemu/slof/fdt.fs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 94 insertions(+) >>> >>> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs >>> index 66f8fe74d9dd..1125f6702435 100644 >>> --- a/board-qemu/slof/fdt.fs >>> +++ b/board-qemu/slof/fdt.fs >>> @@ -198,6 +198,8 @@ fdt-check-header >>> decode-int dup fdt-create-dec fdt-create-enc 2drop >>> THEN >>> >>> + 0 0 s" slof,present-at-boot" property >>> + >>> finish-device >>> ; >>> >>> @@ -514,6 +516,9 @@ r> drop >>> swap ( newnode? a1 ) >>> >>> fdt-debug IF ." Current now: " pwd get-node ." = " . cr THEN >>> + fdt-cas-pass 0= IF >>> + 0 0 s" slof,present-at-cas" property >>> + THEN >> >> I wanted to say this will remove "disk" nodes from SCSI hosts but then I >> realized that "slof,present-at-boot" protects them. May be put this in >> the commit log? > > Yeah, the logic is to remove QEMU's FDT originated nodes only, if they have > "slof,present-at-boot" but not "slof,present-at-cas"... Are you suggesting > that I should emphasize this yup, this. > or are you thinking to something else I might > have missed with SCSI hosts ? nah, it is fine. > >> >> >> >>> BEGIN >>> fdt-next-tag dup OF_DT_END_NODE <> >>> WHILE >>> @@ -584,8 +589,97 @@ r> drop >>> THEN >>> ; >>> >>> +: alias-dev-path ( xt -- dev-path len ) >>> + link> execute decode-string 2swap 2drop >>> +; >>> + >>> +: alias-name ( xt -- alias-name len ) >>> + link> >name name>string >>> +; >>> + >>> +: fdt-cas-alias-obsolete? ( node xt -- true|false ) >>> + alias-dev-path rot node>path str= >>> +; >>> + >>> +: (fdt-cas-delete-obsolete-aliases) ( node xt -- ) >>> + dup IF >>> + dup @ 2 pick swap ( node xt node next-xt ) >>> + recurse ( node xt ) >>> + 2dup fdt-cas-alias-obsolete? IF >>> + dup alias-name ( node xt alias-name len ) >>> + fdt-debug IF ." Deleting alias " 2dup type cr THEN >>> + delete-property ( node xt ) >>> + THEN >>> + THEN >>> + 2drop >>> +; >>> + >>> +: fdt-cas-delete-obsolete-aliases ( node -- ) >>> + s" /aliases" find-node dup IF >>> + ( node aliases ) >>> + get-node >r >>> + dup set-node >>> + >>> + node>properties @ cell+ @ ( node xt ) >>> + over swap ( node node xt ) >>> + (fdt-cas-delete-obsolete-aliases) ( node ) >> >> >> I'd suggest clearing aliases in the end - simply walk through them and >> delete those which we fail to "find-path". >> > > Ok, this will probably simplify the code. > >> >>> + >>> + r> set-node >>> + THEN >>> + drop >>> +; >>> + >>> +: fdt-cas-node-obsolete? ( node -- true|false) >>> + s" slof,present-at-cas" 2 pick get-package-property IF >>> + s" slof,present-at-boot" 2 pick get-package-property IF >>> + \ Node not coming from the QEMU >>> + FALSE >>> + ELSE >>> + \ Device got removed since last boot or CAS >>> + 2drop >>> + TRUE >>> + THEN >>> + ELSE >>> + 2drop ( node ) >>> + >>> + get-node >r >>> + dup set-node >>> + >>> + \ Any device present in the CAS FDT is now considered as if it was >>> + \ present at boot, ie. cold plugged. This allows a subsequent CAS >>> + \ to remove the node from the DT if the device gets unplugged in >>> + \ between). >>> + s" slof,present-at-cas" delete-property >>> + 0 0 s" slof,present-at-boot" property >> >> Uff. The "fdt-cas-node-obsolete?" suggests it does checking but the code >> actually changes the node. For the write only language we should be >> extra careful :) >> > > Heh... I must confess I was kinda expecting this comment would pop up. > >> I'd probably make it one property - "slof,from-fdt" - and store "0" for >> "at-boot" and "1" for "at-cas" and would not bother with switching >> "at-cas" -> "at-boot". If the property is present, then we preserve it, >> that is all. >> > > The "at-cas" -> "at-boot" switching was done so that the deleting logic > still works if the guest calls CAS again before quiesce. Linux doesn't > do that but since it isn't explicitly forbidden by spec AFAIK, who knows > what another OS can decide to do ? > > Following your suggestion, maybe increment "slof,from-fdt" at CAS and > introduce a "slof,fdt-counter" in " /", set to 0 during boot and also > incremented at CAS. Then we would delete nodes for which "slof,from-fdt" > is less than "slof,fdt-counter" ? And of course, don't hijack the > "fdt-cas-node-obsolete?" for that :) Usually it is called "generation" rather then "counter" but yeah, looks simpler too, no? btw I am taking 1/3 and 2/3 now, no need to repost. Thanks, > >> >> >>> + >>> + r> set-node >>> + FALSE >>> + THEN >>> + nip >>> +; >>> + >>> +: (fdt-cas-search-obsolete-nodes) ( start node -- ) >>> + dup IF >>> + dup child 2 pick swap recurse >>> + dup peer 2 pick swap recurse ( start node ) >>> + >>> + fdt-debug IF dup ." Inspecting node: " dup .node ." = " . cr THEN >>> + dup fdt-cas-node-obsolete? IF >>> + fdt-debug IF ." Deleting node" cr THEN >>> + dup fdt-cas-delete-obsolete-aliases >>> + dup delete-node >>> + THEN >>> + THEN >>> + 2drop >>> +; >>> + >>> +: fdt-cas-delete-obsolete-nodes ( start -- ) >>> + s" /" find-device get-node (fdt-cas-search-obsolete-nodes) >>> +; >>> + >>> : fdt-fix-cas-node ( start -- ) >>> 0 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Add phandles >>> + dup fdt-cas-delete-obsolete-nodes \ Delete removed devices >>> 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 0 >>> drop >>> >> >
diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs index 66f8fe74d9dd..1125f6702435 100644 --- a/board-qemu/slof/fdt.fs +++ b/board-qemu/slof/fdt.fs @@ -198,6 +198,8 @@ fdt-check-header decode-int dup fdt-create-dec fdt-create-enc 2drop THEN + 0 0 s" slof,present-at-boot" property + finish-device ; @@ -514,6 +516,9 @@ r> drop swap ( newnode? a1 ) fdt-debug IF ." Current now: " pwd get-node ." = " . cr THEN + fdt-cas-pass 0= IF + 0 0 s" slof,present-at-cas" property + THEN BEGIN fdt-next-tag dup OF_DT_END_NODE <> WHILE @@ -584,8 +589,97 @@ r> drop THEN ; +: alias-dev-path ( xt -- dev-path len ) + link> execute decode-string 2swap 2drop +; + +: alias-name ( xt -- alias-name len ) + link> >name name>string +; + +: fdt-cas-alias-obsolete? ( node xt -- true|false ) + alias-dev-path rot node>path str= +; + +: (fdt-cas-delete-obsolete-aliases) ( node xt -- ) + dup IF + dup @ 2 pick swap ( node xt node next-xt ) + recurse ( node xt ) + 2dup fdt-cas-alias-obsolete? IF + dup alias-name ( node xt alias-name len ) + fdt-debug IF ." Deleting alias " 2dup type cr THEN + delete-property ( node xt ) + THEN + THEN + 2drop +; + +: fdt-cas-delete-obsolete-aliases ( node -- ) + s" /aliases" find-node dup IF + ( node aliases ) + get-node >r + dup set-node + + node>properties @ cell+ @ ( node xt ) + over swap ( node node xt ) + (fdt-cas-delete-obsolete-aliases) ( node ) + + r> set-node + THEN + drop +; + +: fdt-cas-node-obsolete? ( node -- true|false) + s" slof,present-at-cas" 2 pick get-package-property IF + s" slof,present-at-boot" 2 pick get-package-property IF + \ Node not coming from the QEMU + FALSE + ELSE + \ Device got removed since last boot or CAS + 2drop + TRUE + THEN + ELSE + 2drop ( node ) + + get-node >r + dup set-node + + \ Any device present in the CAS FDT is now considered as if it was + \ present at boot, ie. cold plugged. This allows a subsequent CAS + \ to remove the node from the DT if the device gets unplugged in + \ between). + s" slof,present-at-cas" delete-property + 0 0 s" slof,present-at-boot" property + + r> set-node + FALSE + THEN + nip +; + +: (fdt-cas-search-obsolete-nodes) ( start node -- ) + dup IF + dup child 2 pick swap recurse + dup peer 2 pick swap recurse ( start node ) + + fdt-debug IF dup ." Inspecting node: " dup .node ." = " . cr THEN + dup fdt-cas-node-obsolete? IF + fdt-debug IF ." Deleting node" cr THEN + dup fdt-cas-delete-obsolete-aliases + dup delete-node + THEN + THEN + 2drop +; + +: fdt-cas-delete-obsolete-nodes ( start -- ) + s" /" find-device get-node (fdt-cas-search-obsolete-nodes) +; + : fdt-fix-cas-node ( start -- ) 0 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Add phandles + dup fdt-cas-delete-obsolete-nodes \ Delete removed devices 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 0 drop
We recently fixed node creation at CAS in order to support early hotplug of devices between boot and CAS. Let's handle node removal now to support early hot *un*plug of devices. This is done in three phases: 1) all nodes from the boot time FDT are added a "slof,present-at-boot" property 2) during pass 0 of (fdt-fix-cas-node) all nodes from the CAS FDT are added a "slof,present-at-cas" property 3) loop on all nodes in the DT and delete the ones that only have the "slof,present-at-boot" property, and any alias they might have. In the same time, "slof,present-at-cas" properties are converted into "slof,present-at-boot" to allow further node removal in case of a subsequent device unplug and CAS sequence before quiesce (linux doesn't do that but it isn't explicitely forbidden by the spec). Signed-off-by: Greg Kurz <groug@kaod.org> --- board-qemu/slof/fdt.fs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)