Skip to content
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

[sival,tests] Allow excessive csrng irqs in plic_all_irqs_test #20758

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

matutem
Copy link
Contributor

@matutem matutem commented Jan 4, 2024

Carefully allow these excessive irqs for the correct portion of
the tests, addressing #20747 so this test passes.

Add a TODO against the issue above so the code is adjusted
depending on how the issue is addressed.

Enable these tests in fpga_cw310_sival_rom_ext.

@matutem matutem requested a review from msfschaffner as a code owner January 4, 2024 05:48
@matutem matutem requested review from nbdd0121, msfschaffner and HU90m and removed request for msfschaffner January 4, 2024 05:49
@matutem matutem force-pushed the fpga_plic_all_irqs branch from 7fdee1b to ebaa22c Compare January 11, 2024 00:52
@matutem matutem requested a review from a team as a code owner January 11, 2024 00:52
@matutem matutem force-pushed the fpga_plic_all_irqs branch 2 times, most recently from fc72610 to f6a0906 Compare January 11, 2024 01:05
@matutem matutem changed the title [sival,tests] Enable plic_all_irqs test in fpga_cw310_sival_rom_ext [sival,tests] Allow unexpected csrng irqs in plic_all_irqs_test Jan 11, 2024
@matutem matutem requested a review from moidx January 11, 2024 01:07
@msfschaffner msfschaffner requested a review from cfrantz January 13, 2024 00:41
@msfschaffner
Copy link
Contributor

Should we fix this in the ROM_EXT instead?
Or maybe add a comment in the code that the spurious IRQs are likely due to ROM_EXT behavior?

peripheral_expected, peripheral);
if (allow_csrng_irq && kBootStage == kBootStageOwner &&
peripheral != peripheral_expected &&
peripheral == kTopEarlgreyPlicPeripheralCsrng) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we refer to #20747 at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added references at other points. A reference here will not hurt. Done.

@matutem matutem force-pushed the fpga_plic_all_irqs branch from f6a0906 to 61db38a Compare January 13, 2024 01:09
@matutem matutem changed the title [sival,tests] Allow unexpected csrng irqs in plic_all_irqs_test [sival,tests] Allow excessive csrng irqs in plic_all_irqs_test Jan 13, 2024
@matutem matutem force-pushed the fpga_plic_all_irqs branch from 61db38a to ddd5680 Compare January 13, 2024 01:12
Carefully allow these excessive irqs for the correct portion of
the tests, addressing lowRISC#20747 so this test passes.

Add a TODO against the issue above so the code is adjusted
depending on how the issue is addressed.

Enable these tests in fpga_cw310_sival_rom_ext.

Signed-off-by: Guillermo Maturana <[email protected]>
@matutem matutem force-pushed the fpga_plic_all_irqs branch from ddd5680 to 93cdae2 Compare January 13, 2024 01:19
@moidx moidx removed request for nbdd0121, HU90m and a team January 13, 2024 21:03
@moidx
Copy link
Contributor

moidx commented Jan 13, 2024

We should consider writing an entropy complex test to tweak the EDN settings and monitors this interrupt. I am not sure which EDN endpoint is causing the interrupt to trigger so often. Maybe enabling the EDN edn_cmd_req_done interrupt may help to root cause the issue?

@matutem
Copy link
Contributor Author

matutem commented Jan 14, 2024

The failure is unrelated, so this is merged.

@matutem matutem merged commit b5d15bc into lowRISC:master Jan 14, 2024
29 of 32 checks passed
@matutem matutem deleted the fpga_plic_all_irqs branch January 14, 2024 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants