This will eventually need to be a dynamic property based on the page table format in use. Would it not be better to just make pmap_fault verify that the address is suitably sign-extended and return 0 if not?

In D31208#702538, @jrtc27 wrote:

This will eventually need to be a dynamic property based on the page table format in use. Would it not be better to just make pmap_fault verify that the address is suitably sign-extended and return 0 if not?

I mostly prefer the explicitness in this approach, but I could be convinced otherwise. Is your concern about performance?

In my mind, we would #define VM_MIN_KERNEL_ADDRESS VM_MIN_KERNEL_ADDRESS_SV48 when we add that support, even when running sv39. This isn't the only check against these VM_...ADDRESS values, so I don't think this change introduces any new challenges in that regard.

I was thinking we should add an assertion for the sign-extension to pmap_l1(), which should catch most violations. Something similar to D31179. While this would also catch the issue presented here, I don't think it's wrong to handle this case specifically.

Hm. it was mainly based on the fact that the if (stval >= VM_MAX_USER_ADDRESS) { } else { } pattern is common to a lot of architectures whereas this way of writing it is not. Though arm64 for example puts the logic in the trap handler rather than the pmap too by virtue of checking ADDR_IS_CANONICAL first, then using code that mirrors what we currently have for riscv. I quite like that separation of separating out the validity check from the address space check. But perhaps pmap_fault could still benefit from a KASSERT that the passed address is valid for the current translation mode?