YosysHQ / picorv32

PicoRV32 - A Size-Optimized RISC-V CPU
ISC License
2.86k stars 715 forks source link

picorv32_waitirq_insn #221

Open htminuslab opened 1 year ago

htminuslab commented 1 year ago

Hi All,

I am after a HLT (HALT) kind of wait function and was happy to find the "picorv32_waitirq_insn" function. However, I am not sure I understand what the attended behavior should be. In the PicoRV32 the picorv32_waitirq_insn function triggers on any interrupt even if it is masked out. This makes the function somewhat useless?

Possible fix is to include the mask in line 1544:

1543  if (ENABLE_IRQ && (decoder_trigger || do_waitirq) && instr_waitirq) begin
1544    if (irq_pending) begin

change to:

1543  if (ENABLE_IRQ && (decoder_trigger || do_waitirq) && instr_waitirq) begin
1544    if (irq_pending & ~irq_mask) begin   // add mask

Have I missed anything?

Thanks, Hans.

jix commented 1 year ago

The current behavior matches the instruction's description "Pause execution until an interrupt becomes pending.". When a temporarily masked interrupt occurs, it is still pending, despite being masked. This is intended, see also https://github.com/YosysHQ/picorv32/issues/12#issuecomment-228058541.

htminuslab commented 1 year ago

Hi Jix, thanks I missed the #12 comment although no explanation is given why one wants to trigger on a disabled interrupt. My use model is to enable the interrupt(s) and then issue the picorv32_waitirq_insn function. During that time I use the "do_waitirq" signal to lower the system clock. I have 15 int sources and without the mask the picorv32_waitirq_insn function would just fall through as in my system there is always a pending (masked) interrupt.