Skip to content

Commit

Permalink
Speedup response to user input
Browse files Browse the repository at this point in the history
There was a delay between the time the user presses Enter and the time
when the scheduler has a chance to run. That's because the timer is now
deliberately set to a very infrequent 1Hz pace, and the sleeping
scheduler didn't have a chance to know that the UART driver has marked a
process for wakeup.

This commit implements a way for the sleeping scheduler to wake up. It
is achieved by setting a global variable unsleep_scheduler to true. When
the sleeping scheduler (now implemented in soft_park_hart()) encounters
an unsleep, it induces a timer interrupt as soon as possible, which
triggers the usual scheduling workflow.

It may seem like an overkill to induce the timer interrupt when we could
simply return from the sleeper back into the scheduler, but we can't do
that. The scheduler and the interrupts use the same stack (see
set_global_stack) and interrupts corrupt the frame unwind pointers
leading to soft_park_hart(). Using separate stacks for these purposes
would help, but that would bloat the footprint on the smallest supported
systems.
  • Loading branch information
rtfb committed Mar 20, 2024
1 parent 5f8b70c commit a266299
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 15 deletions.
2 changes: 2 additions & 0 deletions include/asm.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

#if !HAS_S_MODE
#define REG_IE STR(mie)
#define REG_IP STR(mip)
#define REG_TVEC STR(mtvec)
#define REG_CAUSE STR(mcause)
#define REG_EPC STR(mepc)
Expand All @@ -44,6 +45,7 @@
#define OP_xRET STR(mret)
#else
#define REG_IE STR(sie)
#define REG_IP STR(sip)
#define REG_TVEC STR(stvec)
#define REG_CAUSE STR(scause)
#define REG_EPC STR(sepc)
Expand Down
11 changes: 6 additions & 5 deletions include/riscv.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@

#define PAGE_OFFS(addr) ((regsize_t)addr & (PAGE_SIZE - 1))

// defined in riscv.c
extern int unsleep_scheduler;

// dedicated M-Mode funcs:
unsigned int get_hartid();
void set_mscratch_csr(void* ptr);
Expand All @@ -111,9 +114,8 @@ void set_medeleg_csr(regsize_t value);
void set_mie_csr(regsize_t value);

// dedicated S-Mode funcs:
void set_sip_csr(regsize_t value);
void csr_sip_clear_flags(regsize_t flags);
regsize_t get_sip_csr();
void csr_sip_set_flags(regsize_t flags);
void set_stvec_csr(void *ptr);
void set_sscratch_csr(void* ptr);

Expand All @@ -130,8 +132,7 @@ void set_status_interrupt_pending();
void set_status_interrupt_enable_and_pending();
void clear_status_interrupt_enable();
void set_interrupt_enable_bits();

// implemented in boot.S
void park_hart();
void hard_park_hart();
void soft_park_hart();

#endif // ifndef _RISCV_H_
1 change: 1 addition & 0 deletions include/timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@ extern void* mtimertrap;
void init_timer();
void set_timer_after(uint64_t delta);
uint64_t time_get_now();
void cause_timer_interrupt_now();

#endif // ifndef _TIMER_H_
8 changes: 3 additions & 5 deletions src/boot.S
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,9 @@ trap_vector: // 3.1.20 Machine Cause Register (mcause
// set_global_stack. This can happen if the scheduler had nothing to
// schedule and the CPU was idling.
beq x0, t0, set_global_stack

// XXX: really? shouldn't sp be set to proc->ctx.sp only when a trap is
// a syscall, but not in other cases?
OP_LOAD sp, 2*REGSZ(t0) // (*process_t)[0] => lock
// (*process_t)[1] => ctx[0] (RA)
// (*process_t)[2] => ctx[1] (SP)
Expand Down Expand Up @@ -489,11 +492,6 @@ interrupt_noop:
interrupt_timer:
j interrupt_epilogue

.globl park_hart
park_hart:
loop: wfi // parked hart will sleep waiting for interrupt
j loop

//## User payload = code + readonly data for U-mode ############################
//
.section .user_text // at least in QEMU 5.1 memory protection seems to work on 4KiB page boundaries,
Expand Down
4 changes: 2 additions & 2 deletions src/kernel.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void kinit(regsize_t hartid, uintptr_t fdt_header_addr) {
if (cpu_id != BOOT_HART_ID) {
release(&init_lock);
// TODO: support multi-core
park_hart();
hard_park_hart();
}
plic_init();
drivers_init();
Expand Down Expand Up @@ -145,5 +145,5 @@ void panic(char const *message) {
uart_prints("panic: ");
uart_prints(message);
uart_prints("\n");
park_hart();
hard_park_hart();
}
5 changes: 5 additions & 0 deletions src/machine/d1/timer.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "mmreg.h"
#include "riscv.h"
#include "timer.h"

#define STIMECMP_LO 0xd000
Expand Down Expand Up @@ -27,3 +28,7 @@ void set_timer_after(uint64_t delta) {
write32(CLINT0_BASE_ADDRESS + STIMECMP_LO, future_lo);
write32(CLINT0_BASE_ADDRESS + STIMECMP_HI, future_hi);
}

void cause_timer_interrupt_now() {
csr_sip_set_flags(SIP_SSIP);
}
7 changes: 6 additions & 1 deletion src/machine/ox64/timer.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "timer.h"
#include "riscv.h"
#include "sbi.h"
#include "timer.h"

void init_timer() {
// nothing to be done on this machine
Expand All @@ -18,3 +19,7 @@ void set_timer_after(uint64_t delta) {
uint64_t now = time_get_now();
sbi_ecall(SBI_EXT_TIME, SBI_EXT_TIME_SET_TIMER, now+delta, 0, 0, 0, 0, 0);
}

void cause_timer_interrupt_now() {
csr_sip_set_flags(SIP_SSIP);
}
7 changes: 6 additions & 1 deletion src/machine/star64/timer.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "timer.h"
#include "riscv.h"
#include "sbi.h"
#include "timer.h"

// This is a verbatim copy of ox64/timer.c
// TODO: unify
Expand All @@ -21,3 +22,7 @@ void set_timer_after(uint64_t delta) {
uint64_t now = time_get_now();
sbi_ecall(SBI_EXT_TIME, SBI_EXT_TIME_SET_TIMER, now+delta, 0, 0, 0, 0, 0);
}

void cause_timer_interrupt_now() {
csr_sip_set_flags(SIP_SSIP);
}
4 changes: 3 additions & 1 deletion src/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ void init_process_table(uint32_t runflags, unsigned int hart_id) {
// means that something went terribly wrong, or all processes are sleeping. In
// which case we should simply schedule the next timer tick and do nothing.
void sleep_scheduler() {
unsleep_scheduler = 0;
#if MIXED_MODE_TIMER
csr_sip_clear_flags(SIP_SSIP);
#else
Expand All @@ -44,7 +45,7 @@ void sleep_scheduler() {
set_status_interrupt_enable_and_pending();
set_interrupt_enable_bits();

park_hart();
soft_park_hart();
}

void scheduler() {
Expand Down Expand Up @@ -614,6 +615,7 @@ void proc_mark_for_wakeup(void *chan) {
}

void update_proc_by_chan(process_t *proc, void *chan) {
unsleep_scheduler = 1;
if (proc->cond.type == PWAKE_COND_CHAN) {
proc->state = PROC_STATE_READY;
proc->chan = 0;
Expand Down
38 changes: 38 additions & 0 deletions src/riscv.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#include "asm.h"
#include "riscv.h"
#include "timer.h"

int unsleep_scheduler = 0;

void set_status_interrupt_pending() {
// set mstatus.MPIE (Machine Pending Interrupt Enable) bit to 1:
Expand Down Expand Up @@ -278,3 +281,38 @@ void csr_sip_clear_flags(regsize_t flags) {
: "r"(flags) // input in flags
);
}

// csr_sip_set_flags sets the requested bits of sip to be set to 1. Effectively,
// it does an atomic "sip |= flags".
void csr_sip_set_flags(regsize_t flags) {
asm volatile (
"csrs sip, %0;" // set requested bits of sip
: // no output
: "r"(flags) // input in flags
);
}

// hard_park_hart is an infinite loop, it will stop the calling hart forever.
// The wfi (wait for interrupt) instruction is just a hint to the core that it
// may downlock itself or otherwise save energy, not that we expect any
// interrupts to happen on it.
//
// It's meant to be called in cases like panic(), or to suspend an unused hart.
void hard_park_hart() {
while (1) {
asm volatile ("wfi");
}
}

// soft_park_hart suspends a hart, but with a way out of it. If some conditions
// set unsleep_scheduler to true, it will immediately cause a timer interrupt,
// which in turn will let the scheduler do its thing.
void soft_park_hart() {
while (1) {
asm volatile ("wfi");
if (unsleep_scheduler) {
unsleep_scheduler = 0;
cause_timer_interrupt_now();
}
}
}
8 changes: 8 additions & 0 deletions src/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,11 @@ void set_timer_after(uint64_t delta) {
uint64_t time_get_now() {
return read64(MTIME);
}

void cause_timer_interrupt_now() {
#if HAS_S_MODE
csr_sip_set_flags(SIP_SSIP);
#else
set_timer_after(0);
#endif
}

0 comments on commit a266299

Please sign in to comment.