[Skiboot] [PATCH] ipmi: call check_timers() while waiting for synchronous messages to complete

[Vasant Hegde]

On 04/04/2019 09:42 PM, Cédric Le Goater wrote:
> On 4/4/19 6:01 PM, Vasant Hegde wrote:
>> On 04/04/2019 07:17 PM, Cédric Le Goater wrote:
>>> BT responses are handled using a timer doing the polling. To hope to
>>> get an answer to an IPMI synchronous message, the timer needs to run.
>>>

So even FreeBSD hit this issue [1].

[1] https://github.com/open-power/skiboot/issues/240

>>
>> timer is called from time_wait_ms() -> opal_run_pollers() -> chec_timer() right?
> 
> we are holding locks when accessing the flash from flash_load_resource().
> so we are taking the nopoll() path AFAICT.



Ah! correct. if we hold lock, it goes to nopoll() path.


.../...

>>> +     * BT response handling relies on a timer. Run timers once in
>>> +     * a while.
>>> +     *
>>> +     * TODO (clg): implement a timeout for IPMI synchronous messages
>>> +     */
>>
>> We have timeout for ipmi messages. Its handled in hw/bt.c (once we schedule message).
> 
> Yes but timers are not running in some cases (this one). If we were
> handling timeouts here, we could cleanly error out instead of looping
> forever.
> 

Handling another timeout here is tricky. We never know when messages gets queued.

With current code, timer starts after sending message to BMC. With this patch we 
will detect timeout cases. So I think we are good.

So I'd suggest to remove that "TODO" line.

Otherwise patch looks good to me.


-Vasant