arm64/vmm: Enable 16-bit VMIDs when in use by pmap

[jrtc27]

Summary

pmap_init always uses 16-bit VMIDs when supported, but we never enable
them in VTCR_EL2 (for ASIDs, locore enables them in TCR_EL1 and
pmap_init keys off whether they've been enabled, but the order in which
pmap_init and vmmops_modinit run is reversed). As a result, although the
full 16-bit value can be stored to VTTBR_EL2 and read back, the upper 8
bits are treated as 0, and so VMIDs that our VMID allocation believes
are distinct end up aliasing.

Obtained from: CheriBSD
Fixes: 47e073941f4e ("Import the kernel parts of bhyve/arm64")
MFC after: 1 week

[andrew]

Is there a reason to not just read the ID register in the vmm code?

[jrtc27]

In D55860#1278404, @andrew wrote:

Is there a reason to not just read the ID register in the vmm code?

Several reasons:

1. I didn't like duplicating the logic
2. If ever some new extension comes along that adds another toggle for bigger VMIDs, this will blow up (and is more likely to be noticed when grep'ing pmap.c) - I want to make sure this error never happens again, and having two parallel bits of code that don't interact with each other is still at risk of that (DS on the following lines is also a bit at risk of that and ideally would instead have been pmap_[stage2_]ds_enabled rather than peeking at TCR_EL1 to infer the expectations of pmap.c)
3. This way you can test with VS disabled on a system that supports 16-bit VMIDs by only modifying the one place in pmap.c that chooses vmids.asid_bits

My initial patch was to do just that, read the ID register and enable VS if supported, but that didn't give me warm fuzzy feelings.

[jhb]

sys/arm64/arm64/pmap.c

800

Hmm, is this function saying that VS is already enabled, or that VS needs to be enabled? Given that we use this to set the bit, I wonder if pmap_vs_needed might be a clearer name?

[jrtc27]

sys/arm64/arm64/pmap.c

800

I guess in my head the name is "pmap has logically enabled using it and vmm will realise that in VTCR_EL2", and it matched the naming for pmap_ps_enabled, though that's an x86-ism that got used as the MI name for superpages. Perhaps pmap_vmid16_enabled (c.f. FEAT_VMID16 as the extension), which more closely matches the fact that pmap has already put 16 in vmids.asid_bits (and if there were a tunable to disable it, it would just override that back to 8), and then vmm uses that to determine it needs to set VTCR_EL2.VS?

[jhb]

sys/arm64/arm64/pmap.c

800

Yes, I think that would be perfect.

[andrew]

sys/arm64/arm64/pmap.c

800

I was wondering if we should move the FEAT_VMID16 detection logic to vmm & have it tell pmap how many vmid bits to support.

[jrtc27]

sys/arm64/arm64/pmap.c

800

You could defer the pmap_init_asids until vmm is loaded, but then you'd need to think about what it would mean to unload vmm.ko and reload it, potentially with a different number of VMID bits requested. Which is perhaps a better longer-term design, but I don't particularly want to have to deal with that right now to fix this bug (that we hit in a few hours of a demo that loops booting a Morello seL4 VM).

[andrew]

sys/arm64/arm64/pmap.c

800

Something like D55961 (untested)

[jrtc27]

sys/arm64/arm64/pmap.c

800

Can we please just land this fix for now? (with pmap_vmid16_enabled as the function name)