Page MenuHomeFreeBSD

powerpc64: fix OFWFB with Radix MMU

Authored by luporl on Jul 20 2021, 1:16 PM.



Current implementation of Radix MMU doesn't support mapping
arbitrary virtual addresses, such as the ones generated by
"direct mapping" I/O addresses. This caused the system to hang, when
early I/O addresses, such as those used by OpenFirmware Frame Buffer,
were remapped after the MMU was up.

To avoid having to modify mmu_radix_kenter_attr just to support this
use case, this change makes early I/O map use virtual addresses from
KVA area instead (similar to what mmu_radix_mapdev_attr does), as
these can be safely remapped later.

I'm not really sure if this is the best way to fix this, so suggestions with better approaches are welcome!

Test Plan

Tested on a Talos II machine, with built in VGA used through OFWFB.

Diff Detail

rG FreeBSD src repository
Lint Not Applicable
Tests Not Applicable

Event Timeline

Great job luporl!

I tested this patch on Raptor Blackbird, as well with D30626 and D30797 (modified version) applied and they all work fine together.


I'd prefer to have "radix_mmu" available for user tunning independent of ISA version.
And use D30797 to set "radix_mmu=1" if PPC_FEATURE2_ARCH_3_00 bit is set.


The problem is we can't allow radix_mmu=1 if ARCH is lower than 3.0 (before POWER9).
Radix is not supported on these architectures and there are some parts in trap.c that checks radix_mmu to decide how to handle exceptions.

Also, this is basically what previous code was doing, just on an earlier spot.
D30797 would remain almost the same too. We would just need to set radix_mmu to 1 on initialization or right before fetching the tunable.
Either way, this part that sets radix_mmu to 0 if !ARCH_3_00 would guarantee it to be off when not supported.

alfredo added inline comments.

Yes, what I meant to say was to leave "radix_mmu" available for user hacking, that could be useful to workaround some problem with CPU feature detection or mismatch in future, custom or virtualized CPUs. Anyway this doesn't make sense of there are other parts that could conflict with with.

This revision is now accepted and ready to land.Aug 2 2021, 12:30 PM

Since you're doing this work here, can you simplify aim_cpu_init() above? The end of aim_cpu_init() could change to something like:

if (radix_mmu)
else if (64-bit)...

Address reviewer's comments

This revision now requires review to proceed.Sep 28 2021, 6:16 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 14 2021, 1:46 PM
This revision was automatically updated to reflect the committed changes.