[SLOF] [PATCH 2/2] fdt: Fix update of "interrupt-controller" node at CAS

[Greg Kurz]

On Thu, 30 Jan 2020 10:21:29 +1100
Alexey Kardashevskiy <aik at ozlabs.ru> wrote:

> 
> 
> On 30/01/2020 05:21, Greg Kurz wrote:
> > Now that QEMU passes a full FDT at CAS without rebooting, a guest that
> > has switched from XICS to XIVE ends up being presented an malformed
> > "interrupt-controller" node in the DT:
> > 
> > # dtc -I fs -O dts /proc/device-tree
> > <stdout>: Warning (unit_address_vs_reg): /interrupt-controller: node has a reg or ranges property, but no unit name
> > ...
> >         interrupt-controller {
> >                 ibm,xive-eq-sizes = <0x10>;
> >                 device_type = "power-ivpe";
> >                 ibm,interrupt-server-ranges = <0x00 0x03>;
> >                 compatible = "ibm,power-ivpe";
> >                 #interrupt-cells = <0x02>;
> >                 reg = <0x60302 0x31b0000 0x00 0x10000 0x60302 0x31a0000 0x00 0x10000>;
> >                 phandle = <0xe7448a8>;
> >                 ibm,xive-lisn-ranges = <0x00 0x03>;
> >                 interrupt-controller;
> >         };
> > 
> > The node should have its unit set to "60302031b0000" as reported by dtc.
> > Also the node still has an "ibm,interrupt-server-ranges" property which
> > only makes sense with XICS.
> > 
> > This happens because we find an existing "interrupt-controller" node,
> > which describes a XICS controller, and we _wrongly_ decide to copy
> > all the properties from the new node into it. Delete the existing node
> > instead so that we create a new node with the appropriate properties
> > and unit name.
> > 
> > Signed-off-by: Greg Kurz <groug at kaod.org>
> > ---
> >  board-qemu/slof/fdt.fs |   13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> > index 2ceb366d4f68..1e2378124b09 100644
> > --- a/board-qemu/slof/fdt.fs
> > +++ b/board-qemu/slof/fdt.fs
> > @@ -465,6 +465,19 @@ r> drop
> >      ELSE
> >  	drop
> >      THEN
> > +
> > +    fdt-cas-pass 0= IF
> > +	\ The guest might have asked to change the interrupt controller type. It doesn't
> > +	\ make sense to merge the new node and the existing "interrupt-controller" node
> > +	\ in this case. Delete the latter. A brand new one will be created with the
> > +	\ appropriate properties and unit name.
> > +	2dup " interrupt-controller" find-substr 0= IF
> > +	    " interrupt-controller" find-node ?dup 0 <> IF
> > +		fdt-debug IF ." Deleting existing node: " dup .node cr THEN
> > +		delete-node
> > +	    THEN
> > +	THEN
> > +    THEN
> 
> This does not look extremely nice but I really do not see a better way
> now. Deleting all properties instead of deleting the node may have
> problems with unit/space; deleting all nodes with QEMU-defined phandles
> may also have undesired effects...
> 

Yeah... I'm not very comfortable with a behavior change for all nodes,
just to handle this case which is very specific to "interrupt-controller"
AFAICT. I tried to come up with something more generic like detecting that
the node from the FDT has a unit name but the node in the DT hasn't, which
is exactly what happens with the XICS->XIVE switch, but the resulting
code was no nicer.

> Reviewed-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> 
> 
> 
> >      2dup find-node ?dup 0 <> IF
> >  	set-node
> >  	fdt-debug IF ." Setting node: " 2dup type cr THEN
> > 
> 
> 
> 
> 
>