Map ACPI memory in ARM64 pmap

[mst_semihalf.com]

Summary

Previously DMAP VA range was used for this purpose but the address range of ACPI tables may not be covered by DMAP.
This change is a prerequisite before switching to L2-block based DMAP.

[andrew]

Can you provide more context. If you are uploading a patch via the web interface there are instructions on the wiki to ensure this is the case at https://wiki.freebsd.org/Phabricator#Create_a_Revision_via_Web_Interface

[andrew]

sys/arm64/arm64/pmap.c

4343

Does this rely on the change to pmap_bootstrap_dmap? If not we should split this into two patches. One for pmap_mapbios/pmap_unmapbios and any related changes, and another to further restrict the DMAP region.

I'd prefer to handle the DMAP change as it will need acknowledgements to the funding agency the work was carried out under.

4437

This doesn't look right. What will happen if we have a mapping open across setting vm_initialized?

4442

You should call kva_free after removing the maps, another CPU may allocate the same virtual address creating a race.

[mst_semihalf.com]

In D15059#317300, @andrew wrote:

Can you provide more context. If you are uploading a patch via the web interface there are instructions on the wiki to ensure this is the case at https://wiki.freebsd.org/Phabricator#Create_a_Revision_via_Web_Interface

Sorry, wrong option during export, I will upload again.

[mst_semihalf.com]

sys/arm64/arm64/pmap.c

4343

Yes, I think we can split it. I'll remove pmap_bootstrap_dmap changes from this patch so that you can commit it separately after mapbios/unmapbios changes have gone in.

[mst_semihalf.com]

sys/arm64/arm64/pmap.c

4442

Good point, I'll reverse the order. I wonder if it's necessary to remove the maps at all - on amd64 for example there's only kva_free() and return.

[mst_semihalf.com]

sys/arm64/arm64/pmap.c

4437

That's a caveat I forgot to mention - it wouldn't work if the mapping is held for so long. I wanted to keep the code simple and didn't see such usage of mapbios currently in the kernel so I concluded it would be invalid to use this interface in this way. Also there's one more problem with holding a mapping across setting vm_initialized. If the mapped physical memory is included in phys_avail (shouldn't be the case for ACPI tables but that's solely dependent on what EFI provides) then 'vm_initialized=1' means VM has already added it to its pool and may overwrite data in those pages at any time. The pre-init mapping is no longer valid in such a case, when vm_initialized is set - which is done in pmap_init.

To add support for mappings held across VM initialization while keeping it simple, I suppose it would be enough to store original PA and size in the pre-init array, maybe increasing the size of the array, and always map new L2 block each time. This could lead to duplicate mappings (two VA blocks -> same PA block) but I think arm64 mandates PIPT d-caches so it shouldn't be a problem.
I can implement this if you think we should support it. That said, the caveat with VM possibly reclaiming the physical memory in between mapbios/unmapbios calls would still hold (however unlikely).

[mst_semihalf.com]

More context.

[mst_semihalf.com]

I wrote an alternative, more complete implementation of mapbios/unmapbios functions without any of the previous caveats.
Of course the code got a little more complicated but I think it's still comprehensible.
@andrew let me know what you think.

I also removed the changes to pmap_bootstrap_dmap as @andrew will commit it separately, after this patch.

[mst_semihalf.com]

Add fix for race condition in pmap_unmapbios() which I forgot to add earlier.

[andrew]

Can you update the proposed commit message. I'm happy with the change, other than we should be clearing the mapping in pmap_unmapbios.

sys/arm64/arm64/pmap.c

4392

I'd prefer we remove the mapping here. It's undefined behaviour to map the same physical address twice with different attributes.

[mst_semihalf.com]

Clear the mapping in pmap_unmapbios as suggested.