Messages in this thread | | | Date | Thu, 05 Mar 2020 10:51:09 -0800 (PST) | Subject | Re: [PATCH] RISC-V: Don't enable all interrupts in trap_init() | From | Palmer Dabbelt <> |
| |
On Tue, 18 Feb 2020 19:28:38 PST (-0800), anup@brainfault.org wrote: > On Wed, Feb 19, 2020 at 12:06 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> >> On Sun, 02 Feb 2020 03:48:18 PST (-0800), atishp@atishpatra.org wrote: >> > On Sun, Feb 2, 2020 at 3:06 AM Anup Patel <anup.patel@wdc.com> 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
Thanks. I think I saw some patches go by, so let's talk over there.
> >> >> >> Cc: stable@vger.kernel.org >> >> 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@wdc.com> >> >> --- >> >> 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@wdc.com> >> >> Tested-by: Palmer Dabbelt <palmerdabbelt@google.com> [QMEU virt machine with SMP] >> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> >> >> 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
| |