[SLOF] [PATCH 3/3] fdt: Delete nodes of devices removed between boot and CAS

[Greg Kurz]

On Fri, 7 Feb 2020 14:41:09 +1100
Alexey Kardashevskiy <aik at 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 at 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
> > 
>