Re: [PATCH v5 0/6] New RISC-V Local Interrupt Controller Driver

From: Palmer Dabbelt
Date: Wed May 27 2020 - 14:52:56 EST

On Thu, 21 May 2020 06:32:55 PDT (-0700), Anup Patel wrote:
This patchset provides a new RISC-V Local Interrupt Controller Driver
for managing per-CPU local interrupts. The overall approach is inspired
from the way per-CPU local interrupts are handled by Linux ARM64 and
ARM GICv3 driver.

Few advantages of this new driver over previous one are:
1. All local interrupts are registered as per-CPU interrupts
2. The RISC-V timer driver can register timer interrupt handler
using kernel irq subsystem without relying on arch/riscv to
explicitly call it's interrupt handler
3. The KVM RISC-V can use this driver to implement interrupt
handler for per-HART guest external interrupt defined by
the RISC-V H-Extension
4. In future, we can develop drivers for devices with per-HART
interrupts without changing arch code or this driver (example,
CLINT timer driver for RISC-V M-mode kernel)

With this patchset, output of "cat /proc/interrupts" looks as follows:
2: 379 0 0 0 SiFive PLIC 10 ttyS0
3: 591 0 0 0 SiFive PLIC 8 virtio0
5: 5079 10821 8435 12984 RISC-V INTC 5 riscv-timer
IPI0: 2045 2537 891 870 Rescheduling interrupts
IPI1: 9 269 91 168 Function call interrupts
IPI2: 0 0 0 0 CPU stop interrupts

The patchset is based up Linux-5.7-rc6 and can be found at riscv_intc_v5
branch of:

This series is tested on:
1. QEMU RV64 virt machine using Linux RISC-V S-mode
2. QEMU RV32 virt machine using Linux RISC-V S-mode
3. QEMU RV64 virt machine using Linux RISC-V M-mode (i.e. NoMMU)

Changes since v4:
- Rebased to Linux-5.7-rc6 and multi-PLIC improvement patches
- Added separate patch to force select RISCV_INTC for CONFIG_RISCV
- Fixed the driver for Linux RISC-V NoMMU

Changes since v3:
- Rebased to Linux-5.6-rc5 and Atish's PLIC patches
- Added separate patch to rename and move plic_find_hart_id()
to arch directory
- Use riscv_of_parent_hartid() in riscv_intc_init() instead of
atomic counter

Changes since v2:
- Dropped PATCH2 since it was merged long-time back
- Rebased series from Linux-4.19-rc2 to Linux-5.6-rc2

Changes since v1:
- Removed changes related to puggable IPI triggering
- Separate patch for self-contained IPI handling routine
- Removed patch for GENERIC_IRQ kconfig options
- Added patch to remove do_IRQ() function
- Rebased upon Atish's SMP patches

Anup Patel (6):
RISC-V: self-contained IPI handling routine
RISC-V: Rename and move plic_find_hart_id() to arch directory
irqchip: RISC-V per-HART local interrupt controller driver
clocksource/drivers/timer-riscv: Use per-CPU timer interrupt
RISC-V: Remove do_IRQ() function

arch/riscv/Kconfig | 2 +
arch/riscv/include/asm/irq.h | 5 -
arch/riscv/include/asm/processor.h | 1 +
arch/riscv/include/asm/smp.h | 3 +
arch/riscv/kernel/cpu.c | 16 +++
arch/riscv/kernel/entry.S | 4 +-
arch/riscv/kernel/irq.c | 33 +-----
arch/riscv/kernel/smp.c | 11 +-
arch/riscv/kernel/traps.c | 2 -
drivers/clocksource/timer-riscv.c | 30 ++++-
drivers/irqchip/Kconfig | 13 +++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-riscv-intc.c | 150 +++++++++++++++++++++++++
drivers/irqchip/irq-sifive-plic.c | 52 +++++----
include/linux/cpuhotplug.h | 1 +
include/linux/irqchip/irq-riscv-intc.h | 20 ++++
16 files changed, 280 insertions(+), 64 deletions(-)
create mode 100644 drivers/irqchip/irq-riscv-intc.c
create mode 100644 include/linux/irqchip/irq-riscv-intc.h

So I read through this a bit, and while I haven't gone through every line of
code I'm somewhat inclined toward taking it.

During the original RISC-V port submission we went back and forth between
having this first-level interrupt controller in arch/riscv/ vs
drivers/irqchip/. The original deciding factor was that the ISA mandated the
interrupt controller, but as that's proving to be less and less the case every
day (with the CLIC and M-mode Linux) it certainly seem sane to move all our
interrupt controller drivers out of arch/riscv/.

This is certainly a step in the right direction, and it handles some of the
more glaring issues (iscv_timer_interrupt and lacking IRQs for the CLINT). I
think we should just go ahead and merge it, even though there might be some
more refactoring to do when we eventually end up with another interrupt

I think it's best if this all goes in through a single tree, as it seems more
work than it's worth to split it up. I'm happy to take it through my tree if
that's OK with the irqchip folks?