[PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[BALATON Zoltan]

The FDT does not normally store name properties but reconstructs it
from path but each node in Open Firmware should at least have this
property. This is correctly handled in getprop but nextprop should
also return it even if not present as a property. This patch fixes
that and also skips phandle which does not appear in Open Firmware
and only added for internal use by VOF.

Explicit name properties are still allowed because they are needed
e.g. on the root node that guests expect to have specific names as
seen on real machines instead of being empty so sometimes the node
name may need to be overriden.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
I've tested this with pegasos2 but don't know how to test spapr.
v2:
Fixed a typo in commit message
Simplified loop to get next property name

 hw/ppc/vof.c | 51 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index 09cb77de93..790d67c096 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -353,34 +353,51 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
 {
     int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
     char prev[OF_PROPNAME_LEN_MAX + 1];
-    const char *tmp;
+    const char *tmp = NULL;
+    bool match = false;
 
     if (readstr(prevaddr, prev, sizeof(prev))) {
         return PROM_ERROR;
     }
-
-    fdt_for_each_property_offset(offset, fdt, nodeoff) {
-        if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
-            return 0;
+    /*
+     * "name" may or may not be present in fdt but we should still return it.
+     * Do that first and then skip it if seen later. Also skip phandle which is
+     * an internal value we added in vof_build_dt but should not appear here.
+     */
+    if (prev[0] == '\0') {
+        tmp = "name";
+    } else {
+        if (strcmp(prev, "name") == 0) {
+            prev[0] = '\0';
         }
-        if (prev[0] == '\0' || strcmp(prev, tmp) == 0) {
-            if (prev[0] != '\0') {
-                offset = fdt_next_property_offset(fdt, offset);
-                if (offset < 0) {
-                    return 0;
-                }
-            }
+        fdt_for_each_property_offset(offset, fdt, nodeoff) {
             if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
                 return 0;
             }
-
-            if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
-                return PROM_ERROR;
+            if (strcmp(tmp, "name") == 0 || strcmp(tmp, "phandle") == 0) {
+                continue;
+            }
+            if (match) {
+                break;
             }
-            return 1;
+            if (strcmp(prev, tmp) == 0) {
+                match = true;
+                continue;
+            }
+            if (prev[0] == '\0') {
+                break;
+            }
+        }
+        if (offset < 0) {
+            return 0;
         }
     }
-
+    if (tmp) {
+        if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
+            return PROM_ERROR;
+        }
+        return 1;
+    }
     return 0;
 }
 
-- 
2.41.3

Re: [PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[Nicholas Piggin]

On Tue Apr 1, 2025 at 12:26 AM AEST, BALATON Zoltan wrote:
> The FDT does not normally store name properties but reconstructs it
> from path but each node in Open Firmware should at least have this
> property. This is correctly handled in getprop but nextprop should
> also return it even if not present as a property. This patch fixes
> that and also skips phandle which does not appear in Open Firmware
> and only added for internal use by VOF.
>
> Explicit name properties are still allowed because they are needed
> e.g. on the root node that guests expect to have specific names as
> seen on real machines instead of being empty so sometimes the node
> name may need to be overriden.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> I've tested this with pegasos2 but don't know how to test spapr.

Boot a pseries machine with pseries (book3s 64-bit) Linux kernel
with x-vof=on option.

AFAIKS the two places Linux calls nextprop look like this

               if (call_prom("nextprop", 3, 1, node, prev_name,
                              pname) != 1)
                        break;

                /* skip "name" */
                if (prom_strcmp(pname, "name") == 0) {
                        prev_name = "name";
                        continue;
                }

So, seems like skipping name is okay?

After iterating through properties it also has this:

        /* Add a "phandle" property if none already exist */
        if (!has_phandle) {
                soff = dt_find_string("phandle");
                if (soff == 0)
                        prom_printf("WARNING: Can't find string index for <phandle> node %s\n", path);

That warning does not seem to fire after your patch.

spapr *seems* to be okay booting, but I would not be inclined to
take this for 10.0 at least without review from someone who knows
more than I do about OF since there can be subtle breakage.

What actual problem is it causing for pegasos?

Thanks,
Nick

Re: [PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[BALATON Zoltan]

On Tue, 1 Apr 2025, Nicholas Piggin wrote:
> On Tue Apr 1, 2025 at 12:26 AM AEST, BALATON Zoltan wrote:
>> The FDT does not normally store name properties but reconstructs it
>> from path but each node in Open Firmware should at least have this
>> property. This is correctly handled in getprop but nextprop should
>> also return it even if not present as a property. This patch fixes
>> that and also skips phandle which does not appear in Open Firmware
>> and only added for internal use by VOF.
>>
>> Explicit name properties are still allowed because they are needed
>> e.g. on the root node that guests expect to have specific names as
>> seen on real machines instead of being empty so sometimes the node
>> name may need to be overriden.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> I've tested this with pegasos2 but don't know how to test spapr.
>
> Boot a pseries machine with pseries (book3s 64-bit) Linux kernel
> with x-vof=on option.
>
> AFAIKS the two places Linux calls nextprop look like this
>
>               if (call_prom("nextprop", 3, 1, node, prev_name,
>                              pname) != 1)
>                        break;
>
>                /* skip "name" */
>                if (prom_strcmp(pname, "name") == 0) {
>                        prev_name = "name";
>                        continue;
>                }
>
> So, seems like skipping name is okay?

For Linux maybe OK to not have name but other OSes use it to identify 
devices so we need it. I tried to match what the real pegasos firmware 
returns but likely SLOF does the same if you do .properties in a node, I 
have not tried SLOF but I can try if that helps. I've seen some 
OpenFirmware output from Macs which also have name in property list and OF 
specification says each node should have a name property so I think it's 
not OK to skip it and not returning it from nextprop does not work for 
pegasos which now has to add explicit name properties in pegasos2.c to fix 
this.

> After iterating through properties it also has this:
>
>        /* Add a "phandle" property if none already exist */
>        if (!has_phandle) {
>                soff = dt_find_string("phandle");
>                if (soff == 0)
>                        prom_printf("WARNING: Can't find string index for <phandle> node %s\n", path);
>
> That warning does not seem to fire after your patch.

Is that good or bad? Was it firing before? If so, getting rid of it may be 
good but I can leave phandle there if it helps Linux. Other OSes don't 
seem to care but it does not seem to appear on real OF results that's why 
I also removed it but I could leave it in if you think it's better that 
way.

> spapr *seems* to be okay booting, but I would not be inclined to
> take this for 10.0 at least without review from someone who knows
> more than I do about OF since there can be subtle breakage.
>
> What actual problem is it causing for pegasos?

Sorry I did not make it clear this is for 10.1 not 10.0, that's why it 
does not say what does it fix. I want to clean up pegasos2.c a bit after 
the freeze and this allows me to remove all the explicit name properties 
which are now only needed because while the name property works with 
getprop, it is not queried due to missing from nextprop. I've just 
submitted this patch in advance to get it reviewed and hopefully merged 
after the freeze so I can submit the patches that depend on it later. I 
don't have any fixes for 10.0 so that should be fine for now, these are 
for after the freeze.

Regards,
BALATON Zoltan

Re: [PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[Alexey Kardashevskiy]

On Tue, 1 Apr 2025, at 01:26, BALATON Zoltan wrote:
> The FDT does not normally store name properties but reconstructs it
> from path but each node in Open Firmware should at least have this
> property. This is correctly handled in getprop but nextprop should
> also return it even if not present as a property. This patch fixes
> that and also skips phandle which does not appear in Open Firmware
> and only added for internal use by VOF.
> 
> Explicit name properties are still allowed because they are needed
> e.g. on the root node that guests expect to have specific names as
> seen on real machines instead of being empty so sometimes the node
> name may need to be overriden.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> I've tested this with pegasos2 but don't know how to test spapr.
> v2:
> Fixed a typo in commit message
> Simplified loop to get next property name
> 
> hw/ppc/vof.c | 51 ++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index 09cb77de93..790d67c096 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -353,34 +353,51 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
> {
>      int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
>      char prev[OF_PROPNAME_LEN_MAX + 1];
> -    const char *tmp;
> +    const char *tmp = NULL;
> +    bool match = false;
>  
>      if (readstr(prevaddr, prev, sizeof(prev))) {
>          return PROM_ERROR;
>      }
> -
> -    fdt_for_each_property_offset(offset, fdt, nodeoff) {
> -        if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> -            return 0;
> +    /*
> +     * "name" may or may not be present in fdt but we should still return it.

yeah we should, at least, to match "getprop". I also wonder if VOF does not add "name", then what would do so, do we really expect to see such properties anywhere? Because if not, then we do not need to skip it as we won't find it.

> +     * Do that first and then skip it if seen later. Also skip phandle which is

(a nit) appears to me that if handling of a missing "name" was done after the last property, the patch would look simpler, but not sure and do not insist.

> +     * an internal value we added in vof_build_dt but should not appear here.

I would not hide anything though, unless it breaks something. Thanks,

> +     */
> +    if (prev[0] == '\0') {
> +        tmp = "name";
> +    } else {
> +        if (strcmp(prev, "name") == 0) {
> +            prev[0] = '\0';
>          }
> -        if (prev[0] == '\0' || strcmp(prev, tmp) == 0) {
> -            if (prev[0] != '\0') {
> -                offset = fdt_next_property_offset(fdt, offset);
> -                if (offset < 0) {
> -                    return 0;
> -                }
> -            }
> +        fdt_for_each_property_offset(offset, fdt, nodeoff) {
>              if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
>                  return 0;
>              }
> -
> -            if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> -                return PROM_ERROR;
> +            if (strcmp(tmp, "name") == 0 || strcmp(tmp, "phandle") == 0) {
> +                continue;
> +            }
> +            if (match) {
> +                break;
>              }
> -            return 1;
> +            if (strcmp(prev, tmp) == 0) {
> +                match = true;
> +                continue;
> +            }
> +            if (prev[0] == '\0') {
> +                break;
> +            }
> +        }
> +        if (offset < 0) {
> +            return 0;
>          }
>      }
> -
> +    if (tmp) {
> +        if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> +            return PROM_ERROR;
> +        }
> +        return 1;
> +    }
>      return 0;
> }
>  
> -- 
> 2.41.3
> 
> 

Re: [PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[BALATON Zoltan]

On Fri, 4 Apr 2025, Alexey Kardashevskiy wrote:
> On Tue, 1 Apr 2025, at 01:26, BALATON Zoltan wrote:
>> The FDT does not normally store name properties but reconstructs it
>> from path but each node in Open Firmware should at least have this
>> property. This is correctly handled in getprop but nextprop should
>> also return it even if not present as a property. This patch fixes
>> that and also skips phandle which does not appear in Open Firmware
>> and only added for internal use by VOF.
>>
>> Explicit name properties are still allowed because they are needed
>> e.g. on the root node that guests expect to have specific names as
>> seen on real machines instead of being empty so sometimes the node
>> name may need to be overriden.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> I've tested this with pegasos2 but don't know how to test spapr.
>> v2:
>> Fixed a typo in commit message
>> Simplified loop to get next property name
>>
>> hw/ppc/vof.c | 51 ++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 34 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
>> index 09cb77de93..790d67c096 100644
>> --- a/hw/ppc/vof.c
>> +++ b/hw/ppc/vof.c
>> @@ -353,34 +353,51 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
>> {
>>      int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
>>      char prev[OF_PROPNAME_LEN_MAX + 1];
>> -    const char *tmp;
>> +    const char *tmp = NULL;
>> +    bool match = false;
>>
>>      if (readstr(prevaddr, prev, sizeof(prev))) {
>>          return PROM_ERROR;
>>      }
>> -
>> -    fdt_for_each_property_offset(offset, fdt, nodeoff) {
>> -        if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
>> -            return 0;
>> +    /*
>> +     * "name" may or may not be present in fdt but we should still return it.
>
> yeah we should, at least, to match "getprop". I also wonder if VOF does 
> not add "name", then what would do so, do we really expect to see such 
> properties anywhere? Because if not, then we do not need to skip it as 
> we won't find it.

I have to add it to fdt where needed. For example on pegasos MorphOS 
checks the name of "/" and expects to find bplan,Pegasos2 which is how it 
identifies the machine. So we need a specific name property there which is 
one example when there will be explicit name property in the fdt. Maybe 
it's needed on some other nodes sometimes but normally the default will be 
sufficient but not always.

>> +     * Do that first and then skip it if seen later. Also skip phandle which is
>
> (a nit) appears to me that if handling of a missing "name" was done 
> after the last property, the patch would look simpler, but not sure and 
> do not insist.

I put name first to match what OpenFirmware does which returns name first. 
SLOF seems to do everything backwards (maybe a result of parsing the fdt 
to build the device tree) and returns properties with inverted order so 
name is last on SLOF but even then the order is matched by putting name 
first when we return properties in the normal order otherwise it would not 
be in same order when reversed. I don't know if it's significant, some 
guests may expect to get a name first but most would probably look for the 
name not its position. The order now matches OpenFirmware and pegasos2 
SmartFirmware and SLOF backwards so I think name is now where it should be 
so I'd leave it first. The loop may become simpler if we don't skip 
phandle only name, the complexity is mostly from sometimes we need to skip 
both in a row.

>> +     * an internal value we added in vof_build_dt but should not appear here.
>
> I would not hide anything though, unless it breaks something. Thanks,

I did some tests with SLOF. This is what I get from SLOF:

package 0x1e64a890 /vdevice/v-scsi@71000003:
         slof,from-fdt          0
         reg                    71000003
         device_type            "vscsi"
         compatible             "IBM,v-scsi"
         interrupts             [0x8 bytes, 2 cells]
         [000] 00001103 00000000
         ibm,#dma-address-cells 2
         ibm,#dma-size-cells    2
         ibm,my-dma-window      "q"
         #address-cells         2
         #size-cells            0
         name                   "v-scsi"

This is VOF before patch:

package 0x00001122 /vdevice/v-scsi@71000003:
         phandle                1122
         #size-cells            0
         #address-cells         2
         ibm,my-dma-window      "q"
         ibm,#dma-size-cells    2
         ibm,#dma-address-cells 2
         interrupts             [0x8 bytes, 2 cells]
         [000] 00001103 00000000
         compatible             "IBM,v-scsi"
         device_type            "vscsi"
         reg                    71000003

and this is VOF after patch:

package 0x00001122 /vdevice/v-scsi@71000003:
         name                   "v-scsi"
         #size-cells            0
         #address-cells         2
         ibm,my-dma-window      "q"
         ibm,#dma-size-cells    2
         ibm,#dma-address-cells 2
         interrupts             [0x8 bytes, 2 cells]
         [000] 00001103 00000000
         compatible             "IBM,v-scsi"
         device_type            "vscsi"
         reg                    71000003

Apart from SLOF returning properties backwards this now matches better. 
SLOF or other Open Firmware implementations don't return phandle property 
because that's what you pass to nextprop or getprop to get the property in 
the first place (listed next to package above) and it is returned by 
finddevice so not normally stored as a property. But if Linux would add it 
and it helps Linux to have it there already we can keep it, it did not 
break OSes on pegasos as they only parse properties they need and ignore 
the rest. So I can skip skipping phandle and add that back if it would be 
better for Linux but didn't removing it fixed a warning about it too?

By the way, phandle is identifying the node so can't we use the fdt offset 
for it so we don't have to add phandle properties? Or does offset change 
when editing the fdt? I think libfdt also uses offsets to identify nodes 
so maybe these should be somewhat stable.

Regards,
BALATON Zoltan

Re: [PATCH v2] ppc/vof: Make nextprop behave more like Open Firmware

[Alexey Kardashevskiy]

On Sat, 5 Apr 2025, at 11:09, BALATON Zoltan wrote:
> On Fri, 4 Apr 2025, Alexey Kardashevskiy wrote:
> > On Tue, 1 Apr 2025, at 01:26, BALATON Zoltan wrote:
> >> The FDT does not normally store name properties but reconstructs it
> >> from path but each node in Open Firmware should at least have this
> >> property. This is correctly handled in getprop but nextprop should
> >> also return it even if not present as a property. This patch fixes
> >> that and also skips phandle which does not appear in Open Firmware
> >> and only added for internal use by VOF.
> >>
> >> Explicit name properties are still allowed because they are needed
> >> e.g. on the root node that guests expect to have specific names as
> >> seen on real machines instead of being empty so sometimes the node
> >> name may need to be overriden.
> >>
> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >> ---
> >> I've tested this with pegasos2 but don't know how to test spapr.
> >> v2:
> >> Fixed a typo in commit message
> >> Simplified loop to get next property name
> >>
> >> hw/ppc/vof.c | 51 ++++++++++++++++++++++++++++++++++-----------------
> >> 1 file changed, 34 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> >> index 09cb77de93..790d67c096 100644
> >> --- a/hw/ppc/vof.c
> >> +++ b/hw/ppc/vof.c
> >> @@ -353,34 +353,51 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
> >> {
> >>      int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
> >>      char prev[OF_PROPNAME_LEN_MAX + 1];
> >> -    const char *tmp;
> >> +    const char *tmp = NULL;
> >> +    bool match = false;
> >>
> >>      if (readstr(prevaddr, prev, sizeof(prev))) {
> >>          return PROM_ERROR;
> >>      }
> >> -
> >> -    fdt_for_each_property_offset(offset, fdt, nodeoff) {
> >> -        if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> >> -            return 0;
> >> +    /*
> >> +     * "name" may or may not be present in fdt but we should still return it.
> >
> > yeah we should, at least, to match "getprop". I also wonder if VOF does 
> > not add "name", then what would do so, do we really expect to see such 
> > properties anywhere? Because if not, then we do not need to skip it as 
> > we won't find it.
> 
> I have to add it to fdt where needed. For example on pegasos MorphOS 
> checks the name of "/" and expects to find bplan,Pegasos2 which is how it 
> identifies the machine. So we need a specific name property there which is 
> one example when there will be explicit name property in the fdt. Maybe 
> it's needed on some other nodes sometimes but normally the default will be 
> sufficient but not always.

Put this paragraph to the commit log, this is useful.

> >> +     * Do that first and then skip it if seen later. Also skip phandle which is
> >
> > (a nit) appears to me that if handling of a missing "name" was done 
> > after the last property, the patch would look simpler, but not sure and 
> > do not insist.
> 
> I put name first to match what OpenFirmware does which returns name first. 
> SLOF seems to do everything backwards (maybe a result of parsing the fdt 
> to build the device tree) and returns properties with inverted order so 
> name is last on SLOF but even then the order is matched by putting name 
> first when we return properties in the normal order otherwise it would not 
> be in same order when reversed. I don't know if it's significant, some 
> guests may expect to get a name first but most would probably look for the 
> name not its position. The order now matches OpenFirmware and pegasos2 
> SmartFirmware and SLOF backwards so I think name is now where it should be 
> so I'd leave it first. The loop may become simpler if we don't skip 
> phandle only name, the complexity is mostly from sometimes we need to skip 
> both in a row.
> 
> >> +     * an internal value we added in vof_build_dt but should not appear here.
> >
> > I would not hide anything though, unless it breaks something. Thanks,
> 
> I did some tests with SLOF. This is what I get from SLOF:
> 
> package 0x1e64a890 /vdevice/v-scsi@71000003:
>          slof,from-fdt          0
>          reg                    71000003
>          device_type            "vscsi"
>          compatible             "IBM,v-scsi"
>          interrupts             [0x8 bytes, 2 cells]
>          [000] 00001103 00000000
>          ibm,#dma-address-cells 2
>          ibm,#dma-size-cells    2
>          ibm,my-dma-window      "q"
>          #address-cells         2
>          #size-cells            0
>          name                   "v-scsi"
> 
> This is VOF before patch:
> 
> package 0x00001122 /vdevice/v-scsi@71000003:
>          phandle                1122
>          #size-cells            0
>          #address-cells         2
>          ibm,my-dma-window      "q"
>          ibm,#dma-size-cells    2
>          ibm,#dma-address-cells 2
>          interrupts             [0x8 bytes, 2 cells]
>          [000] 00001103 00000000
>          compatible             "IBM,v-scsi"
>          device_type            "vscsi"
>          reg                    71000003
> 
> and this is VOF after patch:
> 
> package 0x00001122 /vdevice/v-scsi@71000003:
>          name                   "v-scsi"
>          #size-cells            0
>          #address-cells         2
>          ibm,my-dma-window      "q"
>          ibm,#dma-size-cells    2
>          ibm,#dma-address-cells 2
>          interrupts             [0x8 bytes, 2 cells]
>          [000] 00001103 00000000
>          compatible             "IBM,v-scsi"
>          device_type            "vscsi"
>          reg                    71000003
> 
> Apart from SLOF returning properties backwards this now matches better. 
> SLOF or other Open Firmware implementations don't return phandle property 
> because that's what you pass to nextprop or getprop to get the property in 
> the first place (listed next to package above) and it is returned by 
> finddevice so not normally stored as a property. But if Linux would add it 
> and it helps Linux to have it there already we can keep it, it did not 
> break OSes on pegasos as they only parse properties they need and ignore 
> the rest. So I can skip skipping phandle and add that back if it would be 
> better for Linux but didn't removing it fixed a warning about it too?
> 
> By the way, phandle is identifying the node so can't we use the fdt offset 

This is sort of what SLOF does - uses physical addresses of nodes.

> for it so we don't have to add phandle properties? Or does offset change 
> when editing the fdt?

afair it does.

> I think libfdt also uses offsets to identify nodes 
> so maybe these should be somewhat stable.

tl;dr: no, we want properties.

Stable phandles (vs fdt offset or address) was one of the main triggers to write this VOF in the first place as we already have a few predefined phandles in pseries - interrupt controller and something else, my memory is weak. They go to SLOF which then tells the QEMU what the actual phandle is [1].

When a PCI host bridge is plugged into a pseries VM - there is no SLOF at this point but there an fdt node with a reference to the XICS phandle. And this ibm,client-architecture-support thing (which can enforce reduced number of threads-per-core and things like that) which causes rebuilding of the tree which involves QEMU creating a diff, feeding it back to SLOF which then would update its internal tree (which happens when SLOF is juuust about do die and it is not not really functional any more) to then build FDT (and do [1], again) - and this requires matching phandles in the nodes. 

> 
> Regards,
> BALATON Zoltan
>