Support timer interrupt pending clear in SBI

[atishp04]

As per the privileged spec, timer interrupt pending bit should
be cleared by calling into SEE. Currently, SBI doesn't support
a method to achieve that.

Introduce a method to clear the timer interrupt pending bit in
interrupt pending csr.

Signed-off-by: Atish Patra [email protected]

[aswaterman]

sbi_set_timer is the intended way to do this. I'm not sure why you'd want to clear the current timer interrupt without scheduling another one. If you did, though, there are two ways to do this:

• Simply disable timer interrupts by clearing sie.STIE. Then sip.STIP is a don't-care.
• Schedule a timer interrupt infinitely far in the future (sbi_set_timer((uint64_t)-1)).

[atishp04]

Clearing the timer interrupt bit in sbi_set_timer creates issues in nohz mode. As we are in nohz mode, the timer is not reprogrammed, the pending bits are not cleared. Thus timer interrupt kept on firing resulting cpu stalls.

Here are more details on the cpu stalls.
riscvarchive/riscv-linux#127

[aswaterman]

I'm still not clear on why disabling the timer interrupt enable, or scheduling a timer interrupt infinitely far in the future, isn't sufficient.

[palmer-dabbelt]

I agree with Andrew.

[atishp04]

I am trying to think how to do that without introducing a hack in kernel. Because nohz mode should work seamlessly without any arch specific things.
I couldn't find any genuine arch hookup to do these two.

tick_nohz_handler() just returns without setting the timer again.
https://github.com/riscv/riscv-linux/blob/master/kernel/time/tick-sched.c#L1176

We can do checks in the timer interrupt handler to verify if nohz mode is set but I believe that may conflict with timer management.

Any suggestions?

[atishp04]

And just for my knowledge, Why it is not recommended to clear the timer interrupt pending bit?
I couldn't find anything specific in the spec except this line.
"An SBI call to the SEE may be used to clear the pending timer interrupt."

That's what lead to me believe that we can introduce an SBI call to do that.

[aswaterman]

I don't think it requires a hack... wherever you had planned to call sbi_clear_tip, you can call sbi_set_timer((uint64_t)-1) instead.

I consider adding an SBI call to be as serious as adding a new instruction to the ISA. It's a permanent interface. So we don't want to add SBI calls without being certain they are necessary.

[atishp04]

Hmm. I get your point about adding an additional SBI call.
My current implementation was clearing the pending bit in timer interrupt handler which probably might not be a very good idea in this case.

I am going to look for any possible alternate solution. Let me check how it is done in ARM.

In case anybody wants to check the linux patch:
atishp04/riscv-linux@b9769cb

[aswaterman]

Cool, let us know how it goes...

[atishp04]

I might have misinterpreted something here..But here is my understanding of the current infrastructure in ARM.

Global timer:
https://github.com/riscv/riscv-linux/blob/master/drivers/clocksource/arm_global_timer.c#L162

But arch timer doesn't seem to clear any pending bits!!.
I discussed this with Palmer in the morning. Disabling/enabling timer interrupt seems to be best solution for the time being.

Here is the new patch that doesn't require any additional SBI calls.
atishp04/riscv-linux@fd1e893
@palmer-dabbelt @aswaterman
Thank you for your inputs.

[aswaterman]

Sounds reasonable to me. Thanks for the update.

[atishp04]

Thank you :).
Closing the PR