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

[Anup Patel]

> -----Original Message-----
> From: linux-kernel-owner@xxxxxxxxxxxxxxx <linux-kernel-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Palmer Dabbelt
> Sent: 29 May 2020 09:43
> To: anup@xxxxxxxxxxxxxx
> Cc: Marc Zyngier <maz@xxxxxxxxxx>; Anup Patel <Anup.Patel@xxxxxxx>; Paul
> Walmsley <paul.walmsley@xxxxxxxxxx>; aou@xxxxxxxxxxxxxxxxx;
> daniel.lezcano@xxxxxxxxxx; tglx@xxxxxxxxxxxxx; jason@xxxxxxxxxxxxxx; Atish
> Patra <Atish.Patra@xxxxxxx>; Alistair Francis <Alistair.Francis@xxxxxxx>;
> linux-riscv@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 0/6] New RISC-V Local Interrupt Controller Driver
> 
> On Thu, 28 May 2020 20:57:26 PDT (-0700), anup@xxxxxxxxxxxxxx wrote:
> > On Thu, May 28, 2020 at 12:17 AM Palmer Dabbelt <palmer@xxxxxxxxxxx>
> wrote:
> >>
> >> 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:
> >> >            CPU0       CPU1       CPU2       CPU3
> >> >   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: https://github.com/avpatel/linux.git
> >> >
> >> > 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
> >> >   RISC-V: Force select RISCV_INTC for CONFIG_RISCV
> >> >
> >> >  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 controller.
> >>
> >> 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?
> >
> > A small heads up...
> >
> > Marc has queued a few PLIC improvement patches for Linux-next.
> > (https://patchwork.kernel.org/cover/11555051/)
> >
> > This series (particularly PATCH3) is based upon above mentioned PLIC
> > patches.
> >
> > Apart from above, I don't see any potential merge conflicts.
> 
> Thanks.  I hit some merge issues when pulling it into a staging branch, but
> nothing seemed interesting.  I think the best bet here is to just pull it in through
> the RISC-V tree.
> 
> Aside from this, I'm ready to send out my first 5.8 PR.  I'm going to put this on a
> staging branch and send it up as a second 5.8 merge window PR once
> everything else settles.  IIRC our other irqchip changes have been around for a
> while, so they should be going up early in the merge window.

Sending this as part of second 5.8 merge window PR will be perfect. It will
also take care of dependency on PLIC patches.

Let me know if you want me to rebase and send v6 before your second 5.8 PR.

Thanks,
Anup