[SLOF] [PATCH slof 1/2] fdt: Factor out code to replace a phandle in place
David Gibson
david at gibson.dropbear.id.au
Tue Jun 12 12:58:30 AEST 2018
On Thu, Jun 07, 2018 at 03:30:20PM +1000, Alexey Kardashevskiy wrote:
> On 5/6/18 3:19 pm, David Gibson wrote:
> > On Mon, May 14, 2018 at 08:03:31PM +1000, Alexey Kardashevskiy wrote:
> >> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle
> >> with the real one (i.e. SLOF's node address) in interrupt-parent and
> >> interrupt-map properties. These properties are handled differently -
> >> the interrupt-map is fixed in place while interrupt-parent is
> >> decoded+encoded+set as a property.
> >>
> >> This changes interrupt-parent fixing code to do what the interrupt-map
> >> code does because soon we are going to have more phandles to fix and some
> >> contain an array of phandles (such as "ibm,npu").
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> >
> > Hmmmmmmmm.....
> >
> > Every time I see more of these contortions to deal with the mismatch
> > between SLOF assigned and qemu assigned phandles, I wonder anew if the
> > right solution isn't just for SLOF to keep the qemu assigned ones, and
> > only assign its own for ones that don't have them in the first place.
> >
> > I mean, yes, it's very non-traditional from an OF point of view, but I
> > really have a hard time seeing it being nastier than this growing
> > collection of ad-hoc hacks.
>
>
> I made these patches to add nvlink2 pass through support which means I have
> to have cross references between PCI device nodes in the device tree.
>
> It turns out that if I simply put unique numbers to "phandle" properties in
> QEMU and not patch them, they remain unchanged and then the NVIDIA driver
> can actually find nodes by these QEMU-made phandles. I cannot find places
> in the (guest) kernel which would rely on phandles being physical addresses
> - the client interface is used from prom_init.c and that is it.
No, the phandles definitely should be opaque to the guest. They might
not be to SLOF itself, though.
> In general, we have 2 types of phandles - "phandle" and
> "linux,phandle".
Those aren't two types, just an older and newer name for the same thing.
> If
> "phandle" is present at the prom_init stage, we keep that, if it is not,
> then prom_init creates "linux,phandle" so we end up having one of them but
> not both. We could probably change QEMU to store its phandles as
> "linux,phandle" and simply not touch "phandle" properties. Or use
> "ibm,phandle".
>
> But the comment at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/fdt.c?h=v4.16#n221
> suggests these are not interchangeable and better be equal.
Rather they *are* interchangeable. If you have both they absolutely
must be equal.
> Even though
> "ibm,phandle" values can differ from "phandle" ones. Hm.
I have no idea what "ibm,phandle" is about or where they come from.
>
> The comment came from there :)
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04b954a673dd0
>
>
> >
> >> ---
> >> board-qemu/slof/fdt.fs | 27 +++++++++++++++++++--------
> >> 1 file changed, 19 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> >> index 8e8921f..5284ae6 100644
> >> --- a/board-qemu/slof/fdt.fs
> >> +++ b/board-qemu/slof/fdt.fs
> >> @@ -279,6 +279,23 @@ fdt-claim-reserve
> >> 2drop
> >> ;
> >>
> >> +: fdt-replace-l, 4 - swap 4 + swap ;
> >> +
> >> +: (fdt-replace-phandles) ( old new propname propnamelen node )
> >> + get-property IF 2drop EXIT THEN
> >> + BEGIN
> >> + dup
> >> + WHILE ( old new prop-addr prop-len )
> >> + over l@
> >> + 4 pick = IF
> >> + 2 pick 2 pick l! \ replace old with new in place
> >> + TRUE TO (fdt-phandle-replaced)
> >> + THEN
> >> + fdt-replace-l,
> >> + REPEAT
> >> + 2drop 2drop
> >> +;
> >> +
> >> \ Replace one phandle "old" with a phandle "new" in "node" and recursively
> >> \ in its child nodes:
> >> : fdt-replace-all-phandles ( old new node -- )
> >> @@ -288,14 +305,8 @@ fdt-claim-reserve
> >> ( old new prop-addr prop-len R: node )
> >> fdt-replace-interrupt-map
> >> THEN
> >> - s" interrupt-parent" r@ get-property 0= IF
> >> - ( old new prop-addr prop-len R: node )
> >> - decode-int -rot 2drop ( old new val R: node )
> >> - 2 pick = IF ( old new R: node )
> >> - dup encode-int s" interrupt-parent" r@ set-property
> >> - TRUE TO (fdt-phandle-replaced)
> >> - THEN
> >> - THEN
> >> +
> >> + 2dup s" interrupt-parent" r@ (fdt-replace-phandles)
> >> \ ... add more properties that have to be fixed here ...
> >> r>
> >> \ Now recurse over all child nodes: ( old new node )
> >
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20180612/fee8e087/attachment.sig>
More information about the SLOF
mailing list