[Skiboot] [PATCH] slw: do SLW timer testing while holding xscom lock

Stewart Smith stewart at linux.vnet.ibm.com
Thu Nov 24 15:08:19 AEDT 2016


We add some routines that let a caller get the xscom lock once and
then do a bunch of xscoms while holding it.

In some situations without this, it could take long enough to get
the xscom lock that the 1ms timeout would expire and we'd falsely
think the SLW timer didn't work when in fact it did.

Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
 hw/slw.c        | 12 +++++++++---
 hw/xscom.c      | 26 ++++++++++++++++++++------
 include/xscom.h | 17 +++++++++++++++--
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/hw/slw.c b/hw/slw.c
index b4fb6ec2e6f2..32481fb2751f 100644
--- a/hw/slw.c
+++ b/hw/slw.c
@@ -1242,11 +1242,13 @@ void slw_update_timer_expiry(uint64_t new_target)
 
 	do {
 		/* Grab generation and spin if odd */
+		_xscom_lock();
 		for (;;) {
-			rc = xscom_read(slw_timer_chip, 0xE0006, &gen);
+			rc = _xscom_read(slw_timer_chip, 0xE0006, &gen, false);
 			if (rc) {
 				prerror("SLW: Error %lld reading tmr gen "
 					" count\n", rc);
+				_xscom_unlock();
 				return;
 			}
 			if (!(gen & 1))
@@ -1271,25 +1273,29 @@ void slw_update_timer_expiry(uint64_t new_target)
 				 */
 				prerror("SLW: timer stuck, falling back to OPAL pollers. You will likely have slower I2C and may have experienced increased jitter.\n");
 				prlog(PR_DEBUG, "SLW: Stuck with odd generation !\n");
+				_xscom_unlock();
 				slw_has_timer = false;
 				slw_dump_timer_ffdc();
 				return;
 			}
 		}
 
-		rc = xscom_write(slw_timer_chip, 0x5003A, req);
+		rc = _xscom_write(slw_timer_chip, 0x5003A, req, false);
 		if (rc) {
 			prerror("SLW: Error %lld writing tmr request\n", rc);
+			_xscom_unlock();
 			return;
 		}
 
 		/* Re-check gen count */
-		rc = xscom_read(slw_timer_chip, 0xE0006, &gen2);
+		rc = _xscom_read(slw_timer_chip, 0xE0006, &gen2, false);
 		if (rc) {
 			prerror("SLW: Error %lld re-reading tmr gen "
 				" count\n", rc);
+			_xscom_unlock();
 			return;
 		}
+		_xscom_unlock();
 	} while(gen != gen2);
 
 	/* Check if the timer is working. If at least 1ms has elapsed
diff --git a/hw/xscom.c b/hw/xscom.c
index e51f182744de..9afa9523520b 100644
--- a/hw/xscom.c
+++ b/hw/xscom.c
@@ -402,10 +402,20 @@ static uint32_t xscom_decode_chiplet(uint32_t partid, uint64_t *pcb_addr)
 	return gcid;
 }
 
+void _xscom_lock(void)
+{
+	lock(&xscom_lock);
+}
+
+void _xscom_unlock(void)
+{
+	unlock(&xscom_lock);
+}
+
 /*
  * External API
  */
-int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
+int _xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val, bool take_lock)
 {
 	uint32_t gcid;
 	int rc;
@@ -445,7 +455,8 @@ int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
 	}
 
 	/* HW822317 requires us to do global locking */
-	lock(&xscom_lock);
+	if (take_lock)
+		lock(&xscom_lock);
 
 	/* Direct vs indirect access */
 	if (pcb_addr & XSCOM_ADDR_IND_FLAG)
@@ -454,13 +465,14 @@ int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
 		rc = __xscom_read(gcid, pcb_addr & 0x7fffffff, val);
 
 	/* Unlock it */
-	unlock(&xscom_lock);
+	if (take_lock)
+		unlock(&xscom_lock);
 	return rc;
 }
 
 opal_call(OPAL_XSCOM_READ, xscom_read, 3);
 
-int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
+int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_lock)
 {
 	uint32_t gcid;
 	int rc;
@@ -488,7 +500,8 @@ int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
 	}
 
 	/* HW822317 requires us to do global locking */
-	lock(&xscom_lock);
+	if (take_lock)
+		lock(&xscom_lock);
 
 	/* Direct vs indirect access */
 	if (pcb_addr & XSCOM_ADDR_IND_FLAG)
@@ -497,7 +510,8 @@ int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
 		rc = __xscom_write(gcid, pcb_addr & 0x7fffffff, val);
 
 	/* Unlock it */
-	unlock(&xscom_lock);
+	if (take_lock)
+		unlock(&xscom_lock);
 	return rc;
 }
 opal_call(OPAL_XSCOM_WRITE, xscom_write, 3);
diff --git a/include/xscom.h b/include/xscom.h
index f9550218d4f5..bad9140cf79b 100644
--- a/include/xscom.h
+++ b/include/xscom.h
@@ -208,9 +208,22 @@
  * Error codes TBD, 0 = success
  */
 
+/* Use only in select places where multiple SCOMs are time/latency sensitive */
+extern void _xscom_lock(void);
+extern int _xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val, bool take_lock);
+extern int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_lock);
+extern void _xscom_unlock(void);
+
+
 /* Targeted SCOM access */
-extern int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val);
-extern int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val);
+static inline int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
+{
+	return _xscom_read(partid, pcb_addr, val, true);
+}
+static inline int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val) {
+	return _xscom_write(partid, pcb_addr, val, true);
+}
+
 
 /* This chip SCOM access */
 extern int xscom_readme(uint64_t pcb_addr, uint64_t *val);
-- 
2.1.4



More information about the Skiboot mailing list