Page MenuHomeFreeBSD

plic: fix context calculation
Needs ReviewPublic

Authored by mhorne on Mon, Oct 7, 10:58 PM.

Details

Reviewers
markj
br
kp
Summary

The PLIC registers are divided up by "context" which is purposefully
left ambiguous in the PLIC spec. In Sifive compatible PLIC
implementations (most existing implementations), the mapping from hart
to context is not 1-to-1. Instead each hart has an interrupt context for
machine mode and supervisor mode. Fix the calculation to always target
the supervisor contexts. The incorrect calculations went previously
unnoticed due to the way the PLIC was initialized by the bootloader
firmware.

Relevant PLIC specs:
https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc

https://sifive.cdn.prismic.io/sifive%2Fdc4980ff-17db-448b-b521-4c7ab26b7488_sifive+u54-mc+manual+v19.08.pdf (Chapter 10)

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26918
Build 25223: arc lint + arc unit

Event Timeline

mhorne created this revision.Mon, Oct 7, 10:58 PM
br added a comment.Tue, Oct 8, 1:38 PM

I was looking at Linux driver before writing this one. Why don't they do the same ?

https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-sifive-plic.c

mhorne added a comment.Tue, Oct 8, 5:05 PM
In D21927#479243, @br wrote:

I was looking at Linux driver before writing this one. Why don't they do the same ?
https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-sifive-plic.c

I found this confusing too, but I believe the same behavior is achieved by the "skip context holes" check on line 254. In this case, the holes are the machine-mode contexts. Checking their dts file seems to show why they use -1 for the check.

https://github.com/torvalds/linux/blob/master/arch/riscv/boot/dts/sifive/fu540-c000.dtsi

mhorne edited the summary of this revision. (Show Details)Tue, Oct 8, 5:16 PM
br added a comment.Tue, Oct 8, 6:02 PM

https://github.com/torvalds/linux/blob/master/arch/riscv/boot/dts/sifive/fu540-c000.dtsi

Are these example of PLIC contexts?

interrupts-extended = <
				&cpu0_intc 0xffffffff
				&cpu1_intc 0xffffffff &cpu1_intc 9
				&cpu2_intc 0xffffffff &cpu2_intc 9
				&cpu3_intc 0xffffffff &cpu3_intc 9
				&cpu4_intc 0xffffffff &cpu4_intc 9>;

In that case cpu1 supervisor context is number 2, but macro calculates 3?

mhorne added a comment.Tue, Oct 8, 7:25 PM
In D21927#479338, @br wrote:

Yes these are them.

interrupts-extended = <
				&cpu0_intc 0xffffffff
				&cpu1_intc 0xffffffff &cpu1_intc 9
				&cpu2_intc 0xffffffff &cpu2_intc 9
				&cpu3_intc 0xffffffff &cpu3_intc 9
				&cpu4_intc 0xffffffff &cpu4_intc 9>;

In that case cpu1 supervisor context is number 2, but macro calculates 3?

I think you're correct here. The Hifive Unleashed appears to be a special case since it does not have a supervisor context for hart 0. To handle this case we might have to read these device tree indices. I will rework this patch and test it further, but I might need you to test it on your board if qemu's sifive_u doesn't simulate the extra hart 0.