plic: support irq distribution

[mhorne]

Summary

Our PLIC implementation only enables interrupts on the boot hart.
Implement plic_bind_intr() so that they can be redistributed near the
end of boot during intr_irq_shuffle().

This also slightly modifies how enable bits are handled in an attempt to
better fit the PIC interface. plic_enable_intr/plic_disable_intr are
converted to manage an interrupt source's threshold value, since this
value can be used as to globally enable/disable an irq. All handing of the
per-context enable bits is moved to the new methods plic_setup_intr()
and plic_bind_intr().

Test Plan

The system can boot successfully to multi-user mode with both BBL and OpenSBI with any number of CPUs.
PLIC interrupts can be bound using cpuset(1).

[mhorne]

Rebase on top of changes in D21927.

[mhorne]

Upload correct diff.

[br]

Why do we need to enable interrupts for all CPUs and not only current cpu?
where an interrupt will be signaled in that case?

[mhorne]

In D21928#484089, @br wrote:

Why do we need to enable interrupts for all CPUs and not only current cpu?
where an interrupt will be signaled in that case?

PIC_ENABLE_INTR is called very seldomly, often only once while setting up the PIC during intr_setup_irq by the BSP. Since it is not called per-cpu, this function should handle enabling the interrupt for all active CPUs.

BBL sets all PLIC enable bits to 1 by default, whereas OpenSBI sets them to 0, which is how I noticed that APs weren't having their interrupts enabled.

[br]

In D21928#484096, @mhorne wrote:

In D21928#484089, @br wrote:

Why do we need to enable interrupts for all CPUs and not only current cpu?
where an interrupt will be signaled in that case?

PIC_ENABLE_INTR is called very seldomly, often only once while setting up the PIC during intr_setup_irq by the BSP. Since it is not called per-cpu, this function should handle enabling the interrupt for all active CPUs.

BBL sets all PLIC enable bits to 1 by default, whereas OpenSBI sets them to 0, which is how I noticed that APs weren't having their interrupts enabled.

It looks like we are missing SMP method pic_bind_intr() which should configure which CPUs to send the interrupt to.
Look at gic_v3.c

[br]

I also have a comment from jhb@:
@br typically we only route device interrupts to a single core since interrupt handlers generally assume they are not run concurrently (interrupt filters, ithreads will be single-threaded even if the interrupt is broadcast)
as Andy noted, you want to handle interrupt binding requests, but you also need to do something for the “default” case.
Not sure what INTRNG does, but on x86 we round-robin interrupts among CPUs when they are first setup.

[mhorne]

In D21928#484625, @br wrote:

I also have a comment from jhb@:
@br typically we only route device interrupts to a single core since interrupt handlers generally assume they are not run concurrently (interrupt filters, ithreads will be single-threaded even if the interrupt is broadcast)
as Andy noted, you want to handle interrupt binding requests, but you also need to do something for the “default” case.
Not sure what INTRNG does, but on x86 we round-robin interrupts among CPUs when they are first setup.

Okay, this makes a lot of sense actually, and clears up some of my misconceptions. I did a little more reading on how INTRNG uses the PIC interface, and how arm64 implements it for gicv3.
Below I have described my current understanding of the various PIC methods and how I propose our RISC-V PLIC should implement them. Let me know if there are gaps or errors in my current understanding.

PLIC Functions

The RISC-V PLIC has the following three types of registers for managing interrupts:

1. PLIC_PRIORITY(irq): These registers control the priority level of interrupt events for irq (priority 0 disables the interrupt)
2. PLIC_ENABLE(irq,cpu): These registers control whether an event for irq can trigger an interrupt on cpu
3. PLIC_THRESHOLD(cpu): These registers control which priority levels can interrupt cpu (we just set these to zero during plic_attach(), allowing all sources)

PIC Functions

PIC_SETUP_INTR()

• Purpose: perform any platform specific initialization for the designated irq
• Called once during per-irq in intr_setup_irq()
• Current behaviour: none
• Proposed behaviour: We should disable the enable bits for all CPUs other than the boot CPU for the designated irq. This will ensure a consistent startup state.

PIC_ENABLE_INTR()

• Purpose: globally enable/disable the interrupt source for the designated irq
• Called once immediately following PIC_SETUP_INTR() in intr_setup_irq or during teardown for PIC_DISABLE_INTR()
• Current behaviour: sets the PLIC_ENABLE bit for the current CPU (which is always the boot CPU)
• Proposed behaviour: set the PLIC_PRIORITY of irq to at least 1, to ensure that the PLIC source will generate interrupts. Don't touch the PLIC_ENABLE bits, that's the job of PIC_SETUP_INTR() and PIC_BIND_INTR()

PIC_BIND_INTR()

• Purpose: Set/choose the CPU for which the designated interrupt source should be enabled
• Called once for each irq in intr_irq_shuffle() to assign interrupts at SI_SUB_SMP, and during intr_isrc_assign_cpu when explicitly binding an interrupt
• Current behaviour: none
• Proposed behaviour: This function should set/unset the PLIC_ENABLE bits depending on the selected CPU.

PIC_PRE_ITHREAD()

• Purpose: globally disable the interrupt source while it is being serviced, but ensure that the CPU can receive interrupts from other sources (this is described explicitly in sys/interrupt.h)
• Called from intr_isrc_pre_ithread()
• Current PLIC behaviour: sets the irq threshold to zero, disabling it
• Proposed behaviour: keep it the same, but perform the PLIC_CLAIM here instead of in plic_intr() I'm unsure about this part, it's hard to tell if a PLIC claim is analogous to an EOI on other architectures.

[mhorne]

Rework the changes as described. This moves all PLIC_ENABLE bit handling to the new plic_bind_intr function.
Interrupts are enabled/disabled otherwise by adjusting their priority.

I think this version still has room for improvement but is closer to what we need to implement. One flaw with it currently is that it won't work properly with EARLY_AP_STARTUP, as intr_irq_shuffle will be called before the PLIC is initialized.

[mhorne]

Rebase.

[mhorne]

Whoops. Upload the correct commit.

[mhorne]

Rebase.

[br]

sys/riscv/riscv/plic.c

86

Is it static?

271

from style(9):
/* Insert an empty line if the function has no local variables. */

[mhorne]

Address review comments.

[mhorne]

sys/riscv/riscv/plic.c

271

This was made optional recently, but I do prefer this style.

[mhorne]

@br thanks for your help and patience in reviewing these PLIC changes. I learned a lot while writing them!