Add missing mutex in LPC error path

There is a pre-check to look for errors before every LPC read or
write operation.  This check was running outside of the mutex,
which meant that if an operation caused an error and then another
thread attempted a separate LPC op, the second thread would end
up reporting the error from the first operation,  This would
typically just result in a double log, but there are cases where
an error is expected (the SIO problem for the hiomap protocol)
and is deleted by the caller.  Because of the second thread
seeing the error, the error condition ends up being a visible
log.

The change here is to move the pre-check inside the mutex so that
only 1 thread can ever be checking the LPC status at a time.

Change-Id: I9ce0ce48252b7d2b271aa5dd6e0a819dfb728a25
CQ: SW450825
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/68609
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Corey V. Swenson <cswenson@us.ibm.com>
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
Reviewed-by: William G. Hoffa <wghoffa@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>

1 files changed, 60 insertions, 27 deletions

diff --git a/src/usr/lpc/lpcdd.C b/src/usr/lpc/lpcdd.C
index 48f56a1f2..558ab4d5b 100644
--- a/src/usr/lpc/lpcdd.C
+++ b/src/usr/lpc/lpcdd.C
@@ -109,13 +109,6 @@ errlHndl_t lpcRead(DeviceFW::OperationType i_opType,
     //  then we have to use our special side copy of the driver
     if( i_target == TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL )
     {
-        //First check/clear the LPC bus of errors and commit any errors found
-        l_err = Singleton<LpcDD>::instance().checkForLpcErrors();
-        if (l_err)
-        {
-            errlCommit(l_err, LPC_COMP_ID);
-        }
-
         l_err = Singleton<LpcDD>::instance().readLPC( l_type,
                                                       l_addr,
                                                       io_buffer,
@@ -126,14 +119,6 @@ errlHndl_t lpcRead(DeviceFW::OperationType i_opType,
         if( g_altLpcDD
             && (i_target == g_altLpcDD->getProc()) )
         {
-            //First check/clear the LPC bus of errors and commit
-            //   any errors found
-            l_err = g_altLpcDD->checkForLpcErrors();
-            if (l_err)
-            {
-                errlCommit(l_err, LPC_COMP_ID);
-            }
-
             l_err = g_altLpcDD->readLPC( l_type,
                                          l_addr,
                                          io_buffer,
@@ -230,14 +215,6 @@ errlHndl_t lpcWrite(DeviceFW::OperationType i_opType,
         if( g_altLpcDD
             && (i_target == g_altLpcDD->getProc()) )
         {
-            //First check/clear the LPC bus of errors and commit
-            //   any errors found
-            l_err = g_altLpcDD->checkForLpcErrors();
-            if (l_err)
-            {
-                errlCommit(l_err, LPC_COMP_ID);
-            }
-
             l_err = g_altLpcDD->writeLPC( l_type,
                                           l_addr,
                                           io_buffer,
@@ -1232,10 +1209,38 @@ errlHndl_t LpcDD::readLPC(LPC::TransType i_type,
 {
     // Grab the lock and call the internal function
     mutex_lock(ivp_mutex);
-    errlHndl_t l_err = _readLPC( i_type, i_addr, o_buffer, io_buflen );
+
+    //First check/clear the LPC bus of errors and commit any errors found
+    errlHndl_t l_err_precheck = checkForLpcErrors();
+    if (l_err_precheck)
+    {
+        l_err_precheck->setSev(ERRORLOG::ERRL_SEV_INFORMATIONAL);
+    }
+
+    // Now do the operation
+    errlHndl_t l_err_op = _readLPC( i_type, i_addr, o_buffer, io_buflen );
+
+    // If this op failed and there was something wrong before we started,
+    //  attach the logs together to aid debug
+    if( l_err_op && l_err_precheck )
+    {
+        l_err_precheck->plid(l_err_op->plid());
+        //Note-ideally we would up the severity of l_err_precheck here
+        // as well so that it would be visible everywhere, but we can't
+        // because that breaks the scenario where the caller might want
+        // to delete the log they get back (see SIO).  We don't want
+        // any visible logs in that case.
+    }
+
+    // Always just commit the log for any errors that were present
+    if( l_err_precheck )
+    {
+        errlCommit(l_err_precheck, LPC_COMP_ID);
+    }
+
     mutex_unlock(ivp_mutex);
 
-    return l_err;
+    return l_err_op;
 }
 
 /**
@@ -1248,8 +1253,36 @@ errlHndl_t LpcDD::writeLPC(LPC::TransType i_type,
 {
     // Grab the lock and call the internal function
     mutex_lock(ivp_mutex);
-    errlHndl_t l_err = _writeLPC( i_type, i_addr, i_buffer, io_buflen );
+
+    //First check/clear the LPC bus of errors and commit any errors found
+    errlHndl_t l_err_precheck = checkForLpcErrors();
+    if (l_err_precheck)
+    {
+        l_err_precheck->setSev(ERRORLOG::ERRL_SEV_INFORMATIONAL);
+    }
+
+    // Now do the operation
+    errlHndl_t l_err_op = _writeLPC( i_type, i_addr, i_buffer, io_buflen );
+
+    // If this op failed and there was something wrong before we started,
+    //  attach the logs together to aid debug
+    if( l_err_op && l_err_precheck )
+    {
+        l_err_precheck->plid(l_err_op->plid());
+        //Note-ideally we would up the severity of l_err_precheck here
+        // as well so that it would be visible everywhere, but we can't
+        // because that breaks the scenario where the caller might want
+        // to delete the log they get back (see SIO).  We don't want
+        // any visible logs in that case.
+    }
+
+    // Always just commit the log for any errors that were present
+    if( l_err_precheck )
+    {
+        errlCommit(l_err_precheck, LPC_COMP_ID);
+    }
+
     mutex_unlock(ivp_mutex);
 
-    return l_err;
+    return l_err_op;
 }