Re: COMPAT_NETBSD32 broken on aarch64 due to UVM bug

[Rin Okuyama <rokuyama.rk%gmail.com@localhost>]

Oops, I forgot to CC this message to tech-kern.

-------- Forwarded Message --------
Subject: Re: COMPAT_NETBSD32 broken on aarch64 due to UVM bug
Date: Sun, 27 Oct 2019 18:00:19 +0900
From: Rin Okuyama <rokuyama.rk%gmail.com@localhost>

On 2019/10/27 2:49, Martin Husemann wrote:
> May I ask if there is a chance to get this finished, commited and pulled
> up to netbsd-9 sometime soon? We are mostly ready for 9.0 RC1 (and already
> pretty late).

Sorry for the long silence. I've been almost beaten by day job ;-(.

I've updated the patch:

	http://www.netbsd.org/~rin/uvm_map_20191027.patch

And I'll commit it in few days if there's no further inputs.

On 2019/10/06 9:43, Chuck Silvers wrote:
> I looked at this a while and it looks generally good to me also.
> I'd like a few small changes though, please:

Thank you Chuck for your kind review!

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

I've renamed align_va() to 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)

I've added KASSERT in uvm_map_align_va(). The assertion in
uvm_map_space_avail() is left untouched for symmetry; we have a similar
consistency check in another code path:

	https://nxr.netbsd.org/xref/src/sys/uvm/uvm_map.c#1789

>   - 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.

I've added a stronger assertion,

	KASSERT(flags & UVM_FLAG_FIXED) == 0 || align == 0);

as done in uvm_map_findspace():

	https://nxr.netbsd.org/xref/src/sys/uvm/uvm_map.c#1866

For concern by Joerg,

On 2019/10/02 19:27, Joerg Sonnenberger wrote:
> On Wed, Oct 02, 2019 at 05:59:22PM +0900, Rin Okuyama wrote:
> > Thank you for your comments! Yes, we can use round{down,up}2() here.
>
> Looks good. There is one final concern that just hit me. Do we have to
> care about overflow in the topdown case? Would it be useful to assert
> that?

I think that integer over/underflow as well as out of range b/w
vm_map_min and vm_map_max can be detected in callers.

Is this correct? > everyone

Thanks,
rin