| Message ID | 158032207136.1409918.6540598987399205482.stgit@bahia.lan |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix creation of nodes at CAS some more | expand |
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@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... Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > 2dup find-node ?dup 0 <> IF > set-node > fdt-debug IF ." Setting node: " 2dup type cr THEN >
On Thu, 30 Jan 2020 10:21:29 +1100 Alexey Kardashevskiy <aik@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@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@ozlabs.ru> > > > > > 2dup find-node ?dup 0 <> IF > > set-node > > fdt-debug IF ." Setting node: " 2dup type cr THEN > > > > > > >
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 2dup find-node ?dup 0 <> IF set-node fdt-debug IF ." Setting node: " 2dup type cr THEN
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@kaod.org> --- board-qemu/slof/fdt.fs | 13 +++++++++++++ 1 file changed, 13 insertions(+)