[Skiboot] [PATCH 1/2] SBE: Check timer state before scheduling timer

Vasant Hegde hegdevasant at linux.vnet.ibm.com
Thu Dec 17 17:59:28 AEDT 2020


Timer flow:
  - OPAL sends timer chip-op to SBE and waits for ACK
  - Until we get ACK interrupt from SBE we will not schedule any new timer
  - Once we get ACK either we wait for timer expiry -OR- schedule
    new one if new-timer-request < inflight-timer-timeout value.
  - If we get new timer request while processing current one
    p9_sbe_update_timer_expiry code sets `has_new_target` and we
    schedule it in ACK path (p9_sbe_timer_resp()).

p9_sbe_timer_resp() is callback handler and its called without lock.
It does not check whether timer message is busy or not (timer_ctrl_msg).
So in theory we may hit below scenario and corrupt msg_list.

CPU 1 -> Timer ACK (callback handler) -- its not holding any lock
 CPU 2 -> Grabbed sbe_timer_lock -> scheduled timer --> done
   CPU 3 -> p9_sbe_update_timer_expiry() -> see timer is busy -> sets has_new_timer -> done
      CPU 1 -> gets chance to grab sbe_timer_lock -> saw has_new_timer -> Called p9_sbe_timer_schedule() --> List  corrupted !

This patch adds timer message busy check in p9_sbe_timer_resp().

Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
---
 hw/sbe-p9.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/sbe-p9.c b/hw/sbe-p9.c
index ae5aacb12..75dc37aaa 100644
--- a/hw/sbe-p9.c
+++ b/hw/sbe-p9.c
@@ -768,8 +768,10 @@ static void p9_sbe_timer_resp(struct p9_sbe_msg *msg)
 
 	lock(&sbe_timer_lock);
 	if (has_new_target) {
-		has_new_target = false;
-		p9_sbe_timer_schedule();
+		if (!p9_sbe_msg_busy(timer_ctrl_msg)) {
+			has_new_target = false;
+			p9_sbe_timer_schedule();
+		}
 	}
 	unlock(&sbe_timer_lock);
 }
-- 
2.26.2



More information about the Skiboot mailing list