lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 18 Feb 2020 10:36:27 -0800 (PST) From: Palmer Dabbelt <palmer@...belt.com> To: atishp@...shpatra.org CC: Anup Patel <Anup.Patel@....com>, Paul Walmsley <paul.walmsley@...ive.com>, Damien Le Moal <Damien.LeMoal@....com>, anup@...infault.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org, Atish Patra <Atish.Patra@....com>, Alistair Francis <Alistair.Francis@....com>, linux-riscv@...ts.infradead.org, Christoph Hellwig <hch@....de> Subject: Re: [PATCH] RISC-V: Don't enable all interrupts in trap_init() On Sun, 02 Feb 2020 03:48:18 PST (-0800), atishp@...shpatra.org wrote: > On Sun, Feb 2, 2020 at 3:06 AM Anup Patel <anup.patel@....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. >> Cc: stable@...r.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. >> Signed-off-by: Anup Patel <anup.patel@....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@....com> Tested-by: Palmer Dabbelt <palmerdabbelt@...gle.com> [QMEU virt machine with SMP] Reviewed-by: Palmer Dabbelt <palmerdabbelt@...gle.com> I consider this a bugfix, so I'm targeting it for RCs. It's on fixes and should go up this week. Thanks!
Powered by blists - more mailing lists