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

["Nicholas Piggin" <npiggin@gmail.com>]

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