Switch to 16-byte stack alignment on N32 and N64.

[jhb]

Summary

Add a STACK_ALIGN constant rather than hardcoding sizeof(int64_t) when
aligning the stack during thread and process creation as well as when
constructing a signal frame.

Use a value of 16 for STACK_ALIGN on N32 and N64 and a value of 8 for
O32.

Test Plan

• booted o32, n32, and n64 kernels under qemu

[jmallett]

Also fix alignment in cpu_set_upcall in vm_machdep.c?

[jmallett]

Also, lib/libc/mips/gen/makecontext.c uses its own constants, and should likely be pulled out of the conditional, after subtracting enough registers' worth o' room.

[jhb]

• Use STACK_ALIGN in cpu_set_upcall() as well.
• Update comments in __makecontext().

[jhb]

I'm not quite sure what you would like to do for makecontext. It is already using the right value (16 for newabi), but I don't know if it's really safe to expose STACK_ALIGN to userland (perhaps I can include <machine/md_var.h> to get it in this file?) If the comment is really stale and there is no gap for newabi, then I don't think we can collapse the #ifdef's down any further in makecontext() aside from perhaps using STACK_ALIGN.

lib/libc/mips/gen/makecontext.c

108 ↗

(On Diff #38022)

Hopefully this can be resolved during this review? Given the logic earlier for subtracting from 'sp' to make room, I think the code is correct and that the comment on line 106 should not talk about a gap but just say "pass remaining arguments on the stack".

[jhb]

Ping. Anyone have any thoughts on my last questions?

[jmallett]

In D13875#292023, @jhb wrote:

I'm not quite sure what you would like to do for makecontext. It is already using the right value (16 for newabi), but I don't know if it's really safe to expose STACK_ALIGN to userland (perhaps I can include <machine/md_var.h> to get it in this file?)

I feel like the stack alignment ought to be derived from some shared definition for consistency, and used wherever needed. <machine/asm.h> has lots of similar/related things (which could perhaps be used instead? see ALMASK), but that's for assembly. I don't know if we have a consistent header for use in C (and perhaps also used in assembly.) I feel like a new header along the lines of <machine/abi.h> might be something we want to contain a bunch of things into.

I'm fine with this going in as is, but definitely feel like unification and consistency has benefits if we can do it right. Possibly in such a way as sets an example for adding new ABIs to other architectures, which I imagine you have some thoughts and insights into for, say, x32.

lib/libc/mips/gen/makecontext.c

108 ↗

(On Diff #38022)

Yeah, I can't reason about the comment and think your interpretation is correct.

[jhb]

I've done some more reading and I think I have a grip on the makecontext() stuff, but it's turned into a bigger ball of wax so I'm going to defer fixes there until later.

[jhb]

I did look at ALMASK since we also use CALLFRAME_SIZ in C. I don't like that ALMASK use '-' instead of '~' though. (And in fact, I can't find anything in the tree that uses ALSK or ALMASK currently). -8 and -16 would work though. That bug goes back to when these cosntants were added in NetBSD's asm.h. I feel like I'd rather just add the single STACK_ALIGN perhaps in machine/asm.h and drop the ALSK and ALMASK constants as they are named poorly.

[jmallett]

I'd concur, and I had already pared down from NetBSD some in what's in that file when fixing various ABIs, IIRC, and I think it's reasonable to remove other things that are wrong or misleading.

[jhb]

• Remove duplicate STACK_ALIGN and remove banal comment.
• Add <machine/abi.h> and move some bits of <machine/asm.h> + STACK_ALIGN to it.

[jmallett]

Strong approve; this is a great improvement. Thank you!