> Date: Tue, 22 Feb 2022 15:58:31 +0000 > From: Visa Hankala <v...@hankala.org> > > The standard RISC-V calling convention says that the stack pointer > should be 16-byte aligned. > > The patch below corrects the alignment of the kernel stack in context > switching and exception handling. > > OK?
Is there a reason why we don't simply extend struct switchframe to include another register_t such that it is a multiple of 16 bytes? (and then add a compile-time assertion somewhere to make sure we don't mess that up again) > The diff reveals that curcpu is stored in an unnamed spot near the upper > end of u-area when running in user mode. Should struct trapframe contain > a field where curcpu is stored, so that the usage would become more > apparent? I guess the pointer could also be stored in one of the > existing fields with an appropriate comment in the struct definition. Sorry, but I don't understand you; curpcu is kept in the tp register... > Index: arch/riscv64/include/param.h > =================================================================== > RCS file: src/sys/arch/riscv64/include/param.h,v > retrieving revision 1.4 > diff -u -p -r1.4 param.h > --- arch/riscv64/include/param.h 16 Jun 2021 12:00:15 -0000 1.4 > +++ arch/riscv64/include/param.h 22 Feb 2022 15:29:05 -0000 > @@ -75,6 +75,7 @@ > > #define STACKALIGNBYTES (16 - 1) > #define STACKALIGN(p) ((u_long)(p) &~ STACKALIGNBYTES) > +#define FRAMESZ(sz) (((sz) + STACKALIGNBYTES) & > ~STACKALIGNBYTES) > > #define __HAVE_FDT > > Index: arch/riscv64/riscv64/cpuswitch.S > =================================================================== > RCS file: src/sys/arch/riscv64/riscv64/cpuswitch.S,v > retrieving revision 1.6 > diff -u -p -r1.6 cpuswitch.S > --- arch/riscv64/riscv64/cpuswitch.S 22 Feb 2022 07:47:46 -0000 1.6 > +++ arch/riscv64/riscv64/cpuswitch.S 22 Feb 2022 15:29:05 -0000 > @@ -17,8 +17,9 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > -#include "machine/asm.h" > #include "assym.h" > +#include <machine/asm.h> > +#include <machine/param.h> > > /* > * cpu_switchto(struct proc *oldproc, struct proc *newproc) > @@ -30,7 +31,7 @@ ENTRY(cpu_switchto_asm) > beqz a0, 1f > > // create switchframe > - addi sp, sp, -SWITCHFRAME_SIZEOF > + addi sp, sp, -FRAMESZ(SWITCHFRAME_SIZEOF) > sd s0, (SF_S + 0 * 8)(sp) > sd s1, (SF_S + 1 * 8)(sp) > sd s2, (SF_S + 2 * 8)(sp) > @@ -86,7 +87,7 @@ ENTRY(cpu_switchto_asm) > ld ra, SF_RA(sp) > > RETGUARD_CALC_COOKIE(a7) > - addi sp, sp, SWITCHFRAME_SIZEOF > + addi sp, sp, FRAMESZ(SWITCHFRAME_SIZEOF) > RETGUARD_CHECK(cpu_switchto, a7) > ret > END(cpu_switch_asm) > Index: arch/riscv64/riscv64/exception.S > =================================================================== > RCS file: src/sys/arch/riscv64/riscv64/exception.S,v > retrieving revision 1.5 > diff -u -p -r1.5 exception.S > --- arch/riscv64/riscv64/exception.S 20 May 2021 04:22:33 -0000 1.5 > +++ arch/riscv64/riscv64/exception.S 22 Feb 2022 15:29:05 -0000 > @@ -36,11 +36,12 @@ > > #include "assym.h" > #include <machine/asm.h> > +#include <machine/param.h> > #include <machine/trap.h> > #include <machine/riscvreg.h> > > .macro save_registers mode > - addi sp, sp, -(TRAPFRAME_SIZEOF) > + addi sp, sp, -FRAMESZ(TRAPFRAME_SIZEOF) > > sd ra, (TF_RA)(sp) > > @@ -54,7 +55,7 @@ > > /* Load our pcpu */ > sd tp, (TF_TP)(sp) > - ld tp, (TRAPFRAME_SIZEOF)(sp) > + ld tp, (FRAMESZ(TRAPFRAME_SIZEOF))(sp) > .endif > > sd t0, (TF_T + 0 * 8)(sp) > @@ -89,7 +90,7 @@ > > .if \mode == 1 > /* Store kernel sp */ > - li t1, TRAPFRAME_SIZEOF > + li t1, FRAMESZ(TRAPFRAME_SIZEOF) > add t0, sp, t1 > sd t0, (TF_SP)(sp) > .else > @@ -142,7 +143,7 @@ > csrw sscratch, t0 > > /* Store our pcpu */ > - sd tp, (TRAPFRAME_SIZEOF)(sp) > + sd tp, (FRAMESZ(TRAPFRAME_SIZEOF))(sp) > ld tp, (TF_TP)(sp) > > /* And restore the user's global pointer */ > @@ -181,7 +182,7 @@ > ld a6, (TF_A + 6 * 8)(sp) > ld a7, (TF_A + 7 * 8)(sp) > > - addi sp, sp, (TRAPFRAME_SIZEOF) > + addi sp, sp, FRAMESZ(TRAPFRAME_SIZEOF) > .endm > > .macro do_ast > Index: arch/riscv64/riscv64/vm_machdep.c > =================================================================== > RCS file: src/sys/arch/riscv64/riscv64/vm_machdep.c,v > retrieving revision 1.8 > diff -u -p -r1.8 vm_machdep.c > --- arch/riscv64/riscv64/vm_machdep.c 22 Feb 2022 07:47:46 -0000 1.8 > +++ arch/riscv64/riscv64/vm_machdep.c 22 Feb 2022 15:29:05 -0000 > @@ -91,7 +91,8 @@ cpu_fork(struct proc *p1, struct proc *p > tf->tf_sstatus |= (SSTATUS_SPIE); /* Enable interrupts. */ > tf->tf_sstatus &= ~(SSTATUS_SPP); /* Enter user mode. */ > > - sf = (struct switchframe *)tf - 1; > + sf = (struct switchframe *)STACKALIGN((u_long)tf > + - sizeof(struct switchframe)); > sf->sf_s[0] = 0; /* Terminate chain of call frames. */ > sf->sf_s[1] = (uint64_t)func; > sf->sf_s[2] = (uint64_t)arg; > >
