[Skiboot] [PATCH RESEND v6 2/2] SBE: Add timer support

[Vasant Hegde]

On 04/17/2018 09:10 AM, Benjamin Herrenschmidt wrote:
> On Mon, 2018-04-16 at 15:26 +0530, Vasant Hegde wrote:
>>
>>>>    	/* SBE came back from reset */
>>>>    	if (data & SBE_HOST_RESET) {
>>>>    		prlog(PR_NOTICE, "Back from reset [chip id = %x]\n", chip_id);
>>>> @@ -547,6 +586,9 @@ clr_interrupt:
>>>>    
>>>>    	p9_sbe_process_queue(sbe);
>>>>    	unlock(&sbe->lock);
>>>> +	/* Drop lock and call timers */
>>>> +	if (timer_interrupt)
>>>> +		check_timers(true);
>>>
>>> If for some reason the timer fired early, are you confident it will be
>>> re-armed ?
>>
>> Our understanding is SBE will never get back to us early. They will always come
>> after timeout.
>> So we are good here.
> 
> No, you need to ensure you are robust. It could be something as simple
> as the clock drifting between the SBE and the CPU for example.

Hmmm. Yeah. That's possible. We have two option here.
Fix check_timers code.. something like below

--- a/core/timer.c
+++ b/core/timer.c
@@ -203,8 +203,13 @@ static void __check_timers(uint64_t now)
                 t = list_top(&timer_list, struct timer, link);

                 /* Top of list not expired ? that's it ... */
-               if (!t || t->target > now)
+               if (!t || t->target > now) {
+                       if (proc_gen < proc_gen_p9)
+                               p8_sbe_update_timer_expiry(t->target);
+                       else
+                               p9_sbe_update_timer_expiry(t->target);
                         break;
+               }


.. we are already holding lock.. so should be fine. Of course I haven't tested 
this one. If you are fine  with this approach I will test and send patch.

-OR-

Take care if SBE driver itself. Fairly straight forward. In sbe_interrupt 
processing path, we just need to compare sbe_last_gen_timestamp with mftb() and 
re-arm it.

Thanks
-Vasant