[Skiboot] [PATCH 12/14] core/opal.c: Add FWTS annotations for pollers and missing OPAL_CHECK_TOKEN

Stewart Smith stewart at linux.vnet.ibm.com
Mon Jun 27 15:32:10 AEST 2016


For the missing OPAL_CHECK_TOKEN call, it means we catch kernel bugs in
the firmware test suite, which I guess is a valid thing to do, if slightly
odd. Unfortunately, kernels for POWER8 systems shipped with this (totally
harmless) bug, so it's possible that if FWTS is run on older POWER8 PowerNV
kernels, this warning will be hit. On POWER9 and above though, this
warning should never be hit.

The annotations for pollers should also never be hit, although I'm not
convinced we don't have a spot or two in OPAL where this could still be
the case (specifically, under certain error conditions).

Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
 core/opal.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/core/opal.c b/core/opal.c
index a9d3c41e4cd3..f48e6adcff25 100644
--- a/core/opal.c
+++ b/core/opal.c
@@ -70,7 +70,16 @@ long opal_bad_token(uint64_t token);
 
 long opal_bad_token(uint64_t token)
 {
-	prerror("OPAL: Called with bad token %lld !\n", token);
+	/**
+	 * @fwts-label OPALBadToken
+	 * @fwts-advice OPAL was called with a bad token. On POWER8 and
+	 * earlier, Linux kernels had a bug where they wouldn't check
+	 * if firmware supported particular OPAL calls before making them.
+	 * It is, in fact, harmless for these cases. On systems newer than
+	 * POWER8, this should never happen and indicates a kernel bug
+	 * where OPAL_CHECK_TOKEN isn't being called where it should be.
+	 */
+	prlog(PR_ERR, "OPAL: Called with bad token %lld !\n", token);
 
 	return OPAL_PARAMETER;
 }
@@ -295,6 +304,12 @@ void opal_run_pollers(void)
 
 	/* Don't re-enter on this CPU */
 	if (this_cpu()->in_poller) {
+		/**
+		 * @fwts-label OPALPollerRecursion
+		 * @fwts-advice Recursion detected in opal_run_pollers(). This
+		 * indicates a bug in OPAL where a poller ended up running
+		 * pollers, which doesn't lead anywhere good.
+		 */
 		prlog(PR_ERR, "OPAL: Poller recursion detected.\n");
 		backtrace();
 		return;
@@ -302,12 +317,25 @@ void opal_run_pollers(void)
 	this_cpu()->in_poller = true;
 
 	if (this_cpu()->lock_depth && pollers_with_lock_warnings < 64) {
+		/**
+		 * @fwts-label OPALPollerWithLock
+		 * @fwts-advice opal_run_pollers() was called with a lock
+		 * held, which could lead to deadlock if not excessively
+		 * lucky/careful.
+		 */
 		prlog(PR_ERR, "Running pollers with lock held !\n");
 		backtrace();
 		pollers_with_lock_warnings++;
-		if (pollers_with_lock_warnings == 64)
+		if (pollers_with_lock_warnings == 64) {
+			/**
+			 * @fwts-label OPALPollerWithLock64
+			 * @fwts-advice Your firmware is buggy, see the 64
+			 * messages complaining about opal_run_pollers with
+			 * lock held.
+			 */
 			prlog(PR_ERR, "opal_run_pollers with lock run 64 "
 			      "times, disabling warning.\n");
+		}
 	}
 
 	/* We run the timers first */
-- 
2.1.4



More information about the Skiboot mailing list