-
Notifications
You must be signed in to change notification settings - Fork 6.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] risc-v: drivers: intc_plic: enabled interrupts to be handled by all cores #65922
[RFC] risc-v: drivers: intc_plic: enabled interrupts to be handled by all cores #65922
Conversation
671969f
to
84d25b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this PR!
uint32_t en_value; | ||
uint32_t key; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these 2 line changes are probably not necessary
I think it would be important for @luchnikov to review as well. |
2ee7256
to
58cd3d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general, just some nits
58cd3d2
to
1abdf05
Compare
Ugh... @con-pax - may need to rebase and resolve conflicts. |
644efbc
1abdf05
to
644efbc
Compare
@cfriedt rebased and pushed. |
@ycsin - can you do a bisect on the test that's failing? I would guess it's probably from one of the ones that were just merged ahead of this. |
@cfriedt Hey Chris, apologies, this was my bad: I see something that in my haste, my rebase wasn't successful: I will rebase again from the point of previous approvals |
644efbc
to
4ac438c
Compare
The plic uses contexts to seperate irq enables, threshold priority and claim complete registers from each core for a given platform. As well as this, each privilege level has its own context. for multi-core platform's, we need to be able to enable/ disable a global interrupt for all the cores that are associated with Zephyr. To do this, we need to make some assumptions: 1. The privilege contexts are contiguous 2. M mode context is first, followed by S mode. We know how many cpus are used in an application and each cpu's hartid, thanks to some very handy inline functions. So we iterate through each cpu and use the hartid of a cpu in the calculation of the context. While we are at it, In an effort to make the driver more readable, allign with the macro naming convention outlined in Linux's PLIC driver Signed-off-by: Conor Paxton <conor.paxton@microchip.com>
The plic has a very simple mechanism to claim an interrupt as well as to complete and clear it. The same register is read from/ written to to achieve this. Get the ID of the HART that serviced the interrupt and write to the claim complete register in the correct context Signed-off-by: Conor Paxton <conor.paxton@microchip.com>
Hey @ycsin @fkokosinski |
@ycsin hey, apologies for the delay, I was off. I can test this out. AFAIK, a similar situation occurs in Linux: though each HART in the Affinity pool can service IRQ's, the same HART normally wins the race (though I know you can mask certain IRQ's to certain HARTS) |
I'm reducing the scope down to the main core so the feature added by this PR may be out of the scope of what I'm trying to do for now. Currently I'm trying to trigger the PLIC to interrupt its 2nd target (i.e. 0x09 for qemu_riscv64), it doens't work on Any idea on how to get the qemu to work would be nice, I guess I should probably open a discussion thread in the Q&A instead of spamming this closed PR. |
This patch set is to fix an issue #65489.
The problem to fix is that the first context of the PLIC was hardcoded into the driver. The side effect of that was to disable other cores in a multi-core complex from handling interrupts.
The solution presented here started off life as a cut and past from what is done in Linux, however A slightly more elegant solution raised its head during the process: basically, We know how many CPU's will be associated with a Zephyr application prior to compile time. Also, if its an SMP set up we have a mapping of the hartid to the cpu right from the start.
We can leverage this list of CPU's to enable only the contexts of the harts that are required to run in Zephyr, allowing Zephyr to also play nicely in an AMP setup.