[SLOF] [PATCH slof 1/2] fdt: Factor out code to replace a phandle in place

[Alexey Kardashevskiy]

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.

In general, we have 2 types of phandles - "phandle" and "linux,phandle". 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. Even though
"ibm,phandle" values can differ from "phandle" ones. Hm.

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 )
> 


-- 
Alexey

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20180607/0a3f1e9d/attachment.sig>