xiaoxiang781216 commented on pull request #5782: URL: https://github.com/apache/incubator-nuttx/pull/5782#issuecomment-1077043968
> Thank you for your comments. Here are a few considerations to your questions. > > > How about we trigger software interrupt(SSIP) instead ECALL? > > This mechanism could maybe be used, but is not necessary, as the asm functions provided here do the same. Also, the SW interrupt is typically used for inter-processor communication (IPI) and I would like to keep that option open for a true SMP. Ok. > > > Can we fake CPU by modifying CSR_STATUS to match the ECALL condition and ghen jump to exception_common directly? > > Not really, in my opinion this gets extremely complicated. > > 1. Sharing the IRQ stack will break this > 2. I don't know how the mcause/scause registers will behave in this case. Are these even writable (need to check tomorrow, I don't think so). > 3. Finally, the CURRENT_REGS behavior. CURRENT_REGS != NULL is tested in many places to guess whether the SW is in IRQ or not > > I would keep the interrupt handling separate, it is quite mature code and I would rather not modify it lightly. > OK. > > It's different from the active context switch, the passive context switch(e.g. the block thread wake up by an interrupt handler) since the interrupt handle already run in S-mode: > > The exception entry pointer already save the current thread context for us so we can restore the unblock thread context directly without trap to M-mode > > Absolutely true, and this is what is done in this implementation also > > > My understanding is different from yours: user task need issue syscall here otherwise how the thread can raise the privilege to S-mode? > > This is also true. The user thread must use syscall to raise privilege to S-mode / kernel mode. But a context switch syscall is never sent by the application directly from **user mode**. This happens only when the user task is already executing kernel code in privileged mode. How the task got there can happen e.g. via sem_wait() or some other synchronization mechanism / timer interrupt / other interrupt etc. In any case, at this point the user task is already in **privileged mode**, and thus any sys_call() (including SYS_switch_context) will be executed from privileged mode. Ok, I get your point. U-mode thread never directly initiative the context switch through riscv_switch_context, instead through other normal syscall or the hardware interrupt. > If that uses ecall, then the privilege will raise from S->M which is not what we want. > > This is the only place where SYS_switch_context exists. The macro takes in tcb->xcp.regs from task that is to be preempted as arg1, and the task to be activated as arg2. Only the kernel can do this, as only the kernel knows the tcb:s. The same goes for the other context handling SYS_xxx numbers. These have been replaced by the asm functions, because e.g. in the below case sys_call2 would execute ecall. > > ``` > #define riscv_switchcontext(saveregs, restoreregs) \ > sys_call2(SYS_switch_context, (uintptr_t)saveregs, (uintptr_t)restoreregs) > ``` > > > No, all syscalls(either reserved or normal) are for user space(U-mode) to initiate the designed functionality to kernel space. > > Also true. But with the old implementation the user task is already in kernel space when e.g. sys_call(SYS_context_switch) is executed. If sys_call(SYS_context_switch) executes ecall instruction, again we raise privilege from S->M (or M->M in the case of the flat build). This works when the kernel is in M-mode, as the privilege level stays the same! This is the case for many (not all, like I said, sorry about that) reserved syscalls. > Once kernel mode is enabled, only threads in S-mode will call riscv_switch_context directly to switch the context: 1. In the flat mode, all threads call riscv_switch_context to trigger the context switch. In this case, riscv_switch_context could reuse the interrupt process logic by issuing ECALL since RISCV support ECALL from M-mode to M-mode. 2.In the kernel mode, only S-mode thread call riscv_switch_context to trigger the context switch. In this case, riscv_switch_context has do the save/restore manually, since RISCV doesn't support ECALL from S-mode to S-mode. Basically, riscv_switch_context need two different implementation. > > The suggestion look reasonable, sys_call0-6 is directly called by user space code, libc/machine is a better place. But, we can do the change for all arch in a new patch. > > I agree, a new patch is better for this. This patch is already very big. And I still have the CONFIG_BUILD_KERNEL code to give for review (it's also done) :) > > On a last note, I think I chose the naming of "ksys_call()" quite poorly. Actually, the function is just a trampoline to execute the asm function riscv_syscall_dispatch. A trampoline is required to store the original return address, so the "ksys_call()" return procedure will know where it came from originally (like with ecall you know from EPC, but emulated with SW!) Yes, it confuse me too. > > > Ok, it just has the S-mode timer interrupt pending bit(STIP), so we have: > > > Let M-mode simulate the software timer and access the functionality through ECALL and trigger the interrupt by STIP > > Use the external hardware time instead > > > To support cpuid and timer, S-mode has to talk with M-mode. How do you reduce the coupling since we can't assume M-mode is also running NuttX? Do you plan reuse OpenSBI interface? > > I don't plan to use OpenSBI. The coupling should/could be handled somehow differently. However, if NuttX is running on a hart, no other OS can run on the same hart, so using my "miniSBI" is perfectly safe, **for that hart**. Obviously tampering with other harts this will fail horribly... > But M-mode mayn't run NuttX at all, how that piece software support your private "miniSBI"? My suggestion doesn't mean to bring the whole OpenSBI package into M mode NuttX, but reuse the OpenSBI interface: NuttX in M-mode implement the minimal OpenSBI API required by NuttX in S-mode. The approach make S-mode NuttX could work with any M-mode software which compliant with OpenSBI. > NOTE: The arch specific startup routine e.g. mpfs_head.S sets mtvec = __trap_handler, so ecall to M-mode will enter the nuttx trap handler on that particular hart. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
