Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 28 May 2020 21:13:27 -0700 (PDT)
Subject:     Re: [PATCH v5 0/6] New RISC-V Local Interrupt Controller Driver
In-Reply-To: <CAAhSdy0zXh46P5WPZHmQ_PjwfOEWkh77EZ-_CroH1Eb1c3fDJg@mail.gmail.com>
CC:     Marc Zyngier <maz@kernel.org>, Anup Patel <Anup.Patel@wdc.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        aou@eecs.berkeley.edu, daniel.lezcano@linaro.org,
        tglx@linutronix.de, jason@lakedaemon.net,
        Atish Patra <Atish.Patra@wdc.com>,
        Alistair Francis <Alistair.Francis@wdc.com>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
From:   Palmer Dabbelt <palmer@dabbelt.com>
To:     anup@brainfault.org
Message-ID: <mhng-69ba2cf1-862f-49fc-ab79-eda329f69aee@palmerdabbelt-glaptop1>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 28 May 2020 20:57:26 PDT (-0700), anup@brainfault.org wrote:
> On Thu, May 28, 2020 at 12:17 AM Palmer Dabbelt <palmer@dabbelt.com> 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.

LMK if anyone has issues with that.

>
> Regards,
> Anup