Re: COMPAT_NETBSD32 broken on aarch64 due to UVM bug

[Chuck Silvers <chuq%chuq.com@localhost>]

On Wed, Oct 02, 2019 at 06:07:09PM +0900, Rin Okuyama wrote:
> On 2019/10/01 18:46, Jared McNeill wrote:
> > On Mon, 30 Sep 2019, Rin Okuyama wrote:
> > 
> > > http://gnats.netbsd.org/54395
> > > 
> > > With the patch attached in the PR, that KASSERT does no longer fire on
> > > aarch64 with 32-bit binaries, as far as I can see. Also, the patched
> > > kernels just work for me on amd64, earm, and m68k.
> > 
> > I have been running with the patch in this PR doing bulk builds on arm64 for both arm64 and armv7 in parallel and the KASSERT no longer fires for me either. Nicely done!
> 
> Thank you for testing the patch!
> 
> I updated the patch in accordance with comments by uwe and joerg:
> 
> http://www.netbsd.org/~rin/uvm_map_20191002.patch
> 
> Here's a summary of changes --- mostly no functional changes:
> 
> - Turn ALIGN_VA() macro into inline function align_va().
> - Use proper macros, round{up,down}2(9).
> - Make sure that alignment is power of 2 in uvm_map_space_avail().
> 
> I will commit the revised patch, and send a pullup request in
> the weekend, if there's no objections.
> 
> Thanks,
> rin


I looked at this a while and it looks generally good to me also.
I'd like a few small changes though, please:

 - name the new function something less generic,
   perhaps uvm_map_align_va()

 - put the assertion about the alignment being a power of 2
   into the new function rather than in the callers.
   (or both places if you really want)

 - add an assertion at the top of uvm_map() that either the UVM_FLAG_FIXED flag
   is not set or *startp is already aligned.  uvm_mmap() has a real check
   to enforce this, so the assertion would just be to make sure that
   some other caller of uvm_map() in the kernel doesn't start using
   the align stuff incorrectly.

thanks,
-Chuck