On Tue, Feb 22, 2022 at 05:31:31PM +0100, Mark Kettenis wrote:
> > 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)
Well, that is another way to do it.
> > 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...
In kernel, tp holds curcpu. In userland, tp holds TCB pointer. The
exception entry and exit code has to switch the register's content.
curcpu is stored in kernel stack when the CPU runs userland.
I have not tested the following patch yet. It takes a while
to compile...
Index: arch/riscv64/include/frame.h
===================================================================
RCS file: src/sys/arch/riscv64/include/frame.h,v
retrieving revision 1.2
diff -u -p -r1.2 frame.h
--- arch/riscv64/include/frame.h 12 May 2021 01:20:52 -0000 1.2
+++ arch/riscv64/include/frame.h 22 Feb 2022 16:50:53 -0000
@@ -64,6 +64,7 @@ typedef struct trapframe {
register_t tf_sstatus;
register_t tf_stval;
register_t tf_scause;
+ register_t tf_pad;
} trapframe_t;
/*
@@ -85,6 +86,7 @@ struct sigframe {
struct switchframe {
register_t sf_s[12];
register_t sf_ra;
+ register_t sf_pad;
};
struct callframe {
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 16:50:53 -0000
@@ -62,6 +62,10 @@ cpu_fork(struct proc *p1, struct proc *p
struct trapframe *tf;
struct switchframe *sf;
+ /* Ensure proper stack alignment. */
+ CTASSERT((sizeof(struct trapframe) & STACKALIGNBYTES) == 0);
+ CTASSERT((sizeof(struct switchframe) & STACKALIGNBYTES) == 0);
+
/* Save FPU state to PCB if necessary. */
if (pcb1->pcb_flags & PCB_FPU)
fpu_save(p1, pcb1->pcb_tf);
