On Wed, Feb 19, 2020 at 12:06 AM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote: > > On Sun, 02 Feb 2020 03:48:18 PST (-0800), atishp@xxxxxxxxxxxxxx wrote: > > On Sun, Feb 2, 2020 at 3:06 AM Anup Patel <anup.patel@xxxxxxx> wrote: > >> > >> Historically, we have been enabling all interrupts for each > >> HART in trap_init(). Ideally, we should only enable M-mode > >> interrupts for M-mode kernel and S-mode interrupts for S-mode > >> kernel in trap_init(). > >> > >> Currently, we get suprious S-mode interrupts on Kendryte K210 > >> board running M-mode NO-MMU kernel because we are enabling all > >> interrupts in trap_init(). To fix this, we only enable software > >> and external interrupt in trap_init(). In future, trap_init() > >> will only enable software interrupt and PLIC driver will enable > >> external interrupt using CPU notifiers. > > I think we should add a proper interrupt controller driver for the per-hart > interrupt controllers, as doing this within the other drivers is ugly -- for > example, there's no reason an MMIO timer or interrupt controller driver should > be toggling these bits. I have always been in support of having per-hart interrupt controller driver. I will rebase my RISC-V INTC driver upon latest kernel and send it again. Of course, now the situation has changed the RISC-V INTC driver will have to consider NOMMU kernel as well. The last version of RISC-V INTC driver can be found in riscv_intc_v2 branch of https://github.com/avpatel/linux.git > > >> Cc: stable@xxxxxxxxxxxxxxx > >> Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code) > > I'd argue this actually fixes the M-mode stuff, since that's the first place > this issue shows up. I've queued this with > > Fixes: a4c3733d32a7 ("riscv: abstract out CSR names for supervisor vs machine mode") > > instead, as that's the first commit that will actually write to MIE and > therefor the first commit that will actually exhibit bad behavior. It also has > the advantage of making the patch apply on older trees, which should make life > easier for the stable folks. Sure, no problem. > > >> Signed-off-by: Anup Patel <anup.patel@xxxxxxx> > >> --- > >> arch/riscv/kernel/traps.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > >> index f4cad5163bf2..ffb3d94bf0cc 100644 > >> --- a/arch/riscv/kernel/traps.c > >> +++ b/arch/riscv/kernel/traps.c > >> @@ -156,6 +156,6 @@ void __init trap_init(void) > >> csr_write(CSR_SCRATCH, 0); > >> /* Set the exception vector address */ > >> csr_write(CSR_TVEC, &handle_exception); > >> - /* Enable all interrupts */ > >> - csr_write(CSR_IE, -1); > >> + /* Enable interrupts */ > >> + csr_write(CSR_IE, IE_SIE | IE_EIE); > >> } > >> -- > >> 2.17.1 > >> > >> > > > > Looks good. > > Reviewed-by: Atish Patra <atish.patra@xxxxxxx> > > Tested-by: Palmer Dabbelt <palmerdabbelt@xxxxxxxxxx> [QMEU virt machine with SMP] > Reviewed-by: Palmer Dabbelt <palmerdabbelt@xxxxxxxxxx> > > I consider this a bugfix, so I'm targeting it for RCs. It's on fixes and > should go up this week. > > Thanks! Thanks, Anup
