Page MenuHomeFreeBSD

ofwfb: fix boot on LE
ClosedPublic

Authored by luporl on Dec 4 2020, 2:56 PM.

Details

Summary

Some framebuffer properties obtained from the device tree were not being properly converted to host endian.
Replace OF_getprop calls by OF_getencprop where needed to fix this.

Test Plan

These changes were tested on a Talos II (POWER9) machine, both with FreeBSD for powerpc64le as well as for powerpc64 (BE).

While older firmware revisions from Talos should be ok, newer ones need a modified 30-dtb-updates binary to workaround a Petitboot issue, where the framebuffer address set in the device tree is incorrect.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

luporl requested review of this revision.Dec 4 2020, 2:56 PM
sys/dev/vt/hw/ofwfb/ofwfb.c
111

I don't think this is correct for the case where it is an ihandle instead of a path.

402

Same here. ihandles (as used when you are dealing with a "live" OF instead of a FDT) are one of the things that need to be preserved as-is, otherwise the logic of swapping them back and forth gets pretty confusing.

sys/dev/vt/hw/ofwfb/ofwfb.c
111

I've tested your suggested changes on LE.
They worked on Talos but not on QEMU, where the kernel hangs before printing anything.

Both Talos and QEMU work fine when using OF_getencprop with stdout.
But note that on QEMU I'm using loader with D26699 and booting it from cd, as currently this seems to be the only way to boot a kernel on a PPC64LE VM.

Is there anything else that should be done to make this work without using OF_getencprop?

sys/dev/vt/hw/ofwfb/ofwfb.c
111

Debugging OF_instance_to_package(stdout) I found out the problem is that stdout ihandle is compared to phandles at OF_child_xref_phandle(), and those phandles are obtained with OF_getencprop(), so there is a clear mismatch here.

If we can't use OF_getencprop() for /chosen/stdout, to avoid breaking "live" OF, then we need to byte swap the ihandle before treating it as a phandle. The call to OF_node_from_xref() at ofw_fdt_instance_to_package() seems a good place.

sys/dev/vt/hw/ofwfb/ofwfb.c
111

I've tested this change again, with the now working PPC LE loader, and the result is the same: ofwfb doesn't work unless OF_getencprop() is used. Changing OF_child_xref_phandle() behavior to not swap phandles is risky, because existing code already relies on it.

We also have dumped and compared BE and LE FDTs and confirmed they are ok.

Because of the reasons above, I think it is better to just use OF_getencprop(). It won't break PPC64BE "live" OF and IIUC "live" OF on PPC64LE doesn't make much sense, since PPC64LE either runs on hardware or with SLOF, that doesn't support several "live" OF features.

What do you think?

OK.

Tested on PPC32.

Also, I realized that our real-mode ofw_real_instance_to_package was actually byteswapping, but ofw_std_instance_to_package() isn't.

This isn't actually a problem in practice though because ofw_std_instance_to_package is only used in virtual mode, which only happens on macintosh hw, which we don't support LE on.

I introduced the swapping in in b49db8270ae3. Another note -- in 509142e1894f where the initial endian flexibility was added to other code way back in 2015, phyp_console was changed to use OF_getencprop for the same thing (stdout on chosen).

So accepting this as is. The new standard will be to keep instance handles in native endian.

This revision is now accepted and ready to land.Mar 9 2021, 1:56 AM
This revision was automatically updated to reflect the committed changes.