From a266299e208bce79b7a58f96b3bc70cd8468e25d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vytautas=20=C5=A0altenis?= Date: Wed, 20 Mar 2024 14:46:44 +0200 Subject: [PATCH] Speedup response to user input 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. --- include/asm.h | 2 ++ include/riscv.h | 11 ++++++----- include/timer.h | 1 + src/boot.S | 8 +++----- src/kernel.c | 4 ++-- src/machine/d1/timer.c | 5 +++++ src/machine/ox64/timer.c | 7 ++++++- src/machine/star64/timer.c | 7 ++++++- src/proc.c | 4 +++- src/riscv.c | 38 ++++++++++++++++++++++++++++++++++++++ src/timer.c | 8 ++++++++ 11 files changed, 80 insertions(+), 15 deletions(-) diff --git a/include/asm.h b/include/asm.h index 4db5712..882b4bd 100644 --- a/include/asm.h +++ b/include/asm.h @@ -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) @@ -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) diff --git a/include/riscv.h b/include/riscv.h index 10b47bd..b3b7e6f 100644 --- a/include/riscv.h +++ b/include/riscv.h @@ -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); @@ -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); @@ -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_ diff --git a/include/timer.h b/include/timer.h index 2fdba0c..d83c98f 100644 --- a/include/timer.h +++ b/include/timer.h @@ -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_ diff --git a/src/boot.S b/src/boot.S index a0b6468..635a7bd 100644 --- a/src/boot.S +++ b/src/boot.S @@ -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) @@ -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, diff --git a/src/kernel.c b/src/kernel.c index c343a2a..feac7a4 100644 --- a/src/kernel.c +++ b/src/kernel.c @@ -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(); @@ -145,5 +145,5 @@ void panic(char const *message) { uart_prints("panic: "); uart_prints(message); uart_prints("\n"); - park_hart(); + hard_park_hart(); } diff --git a/src/machine/d1/timer.c b/src/machine/d1/timer.c index a195da1..7ce4378 100644 --- a/src/machine/d1/timer.c +++ b/src/machine/d1/timer.c @@ -1,4 +1,5 @@ #include "mmreg.h" +#include "riscv.h" #include "timer.h" #define STIMECMP_LO 0xd000 @@ -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); +} diff --git a/src/machine/ox64/timer.c b/src/machine/ox64/timer.c index fccb7ff..48ccb78 100644 --- a/src/machine/ox64/timer.c +++ b/src/machine/ox64/timer.c @@ -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 @@ -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); +} diff --git a/src/machine/star64/timer.c b/src/machine/star64/timer.c index e3e5089..8bb75c5 100644 --- a/src/machine/star64/timer.c +++ b/src/machine/star64/timer.c @@ -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 @@ -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); +} diff --git a/src/proc.c b/src/proc.c index 775fd16..e1d01d0 100644 --- a/src/proc.c +++ b/src/proc.c @@ -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 @@ -44,7 +45,7 @@ void sleep_scheduler() { set_status_interrupt_enable_and_pending(); set_interrupt_enable_bits(); - park_hart(); + soft_park_hart(); } void scheduler() { @@ -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; diff --git a/src/riscv.c b/src/riscv.c index e99d16a..d96e72f 100644 --- a/src/riscv.c +++ b/src/riscv.c @@ -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: @@ -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(); + } + } +} diff --git a/src/timer.c b/src/timer.c index 6f828bf..53b4d80 100644 --- a/src/timer.c +++ b/src/timer.c @@ -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 +}