sio: Add test for availability

Some components can continue to operate in the face of the SuperIO
controller being unavailable on the LPC bus (specifically, the UART and
boot flag processing). Other components require it present (AST-based
SFC implementations and the AST mailbox). Components in the latter
category can just fail with an errl when they attempt to access the
controller, but for those in the former category we add an isAvailable()
function in the SIO namespace to sidestep dealing with errors.

Specifically, isAvailable() tests for the expected error when the
SuperIO controller is disabled, and returns an errlHndl_t if any other
error occurs. This way true LPC errors are propagated to the caller to
commit as desired.

For the moment *all* errors produced by the SIO::isAvailable() LPC bus
access will result in the SIO code assuming the device is absent. We
should be more precise about this, but the hardware behaviour seen under
hostboot currently prevents us from being more specific. This problem is
highlighted by a FIXME block in the implementation of
SIO::isAvailable().

Change-Id: Id30a09b48586d2054e0cdae625ee23df68ac2aa3
Signed-off-by: Andrew Jeffery <andrewrj@au1.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/67460
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-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: Daniel M. Crowell <dcrowell@us.ibm.com>

9 files changed, 131 insertions, 31 deletions

diff --git a/src/include/usr/sio/sio.H b/src/include/usr/sio/sio.H
index 52f752da7..ec422d8b7 100644
--- a/src/include/usr/sio/sio.H
+++ b/src/include/usr/sio/sio.H
@@ -25,6 +25,8 @@
 #ifndef __SIO_SIO_H
 #define __SIO_SIO_H
 
+#include <errl/errlentry.H>
+
 namespace SIO
 {
     /**
@@ -47,5 +49,17 @@ namespace SIO
         SIO_SCRATCH_REG1        = 0x21,     /**< Scratch Reg */
         SIO_SCRATCH_REG2        = 0x22,     /**< Scratch Reg */
     };
+
+    /**
+     * @brief Test if SuperIO is accessible on the LPC bus
+     *
+     * @param[out] o_available Set true if the SuperIO controller is available,
+     *                         false if it explicitly detected as unavailable,
+     *                         and unchanged if an unexpected error occurs.
+     *
+     * @return NULL if the test did not fail unexpectedly, otherwise a pointer
+     *         to an errorlog associated with the error.
+     */
+    errlHndl_t isAvailable(bool& o_available);
 }
 #endif
diff --git a/src/usr/console/ast2400.C b/src/usr/console/ast2400.C
index 1990baf49..213ae789e 100644
--- a/src/usr/console/ast2400.C
+++ b/src/usr/console/ast2400.C
@@ -297,6 +297,18 @@ namespace CONSOLE
 
             do
             {
+                bool haveSio;
+                l_err = SIO::isAvailable(haveSio);
+                if (l_err) { break; }
+
+                if (!haveSio)
+                {
+                    // SIO may have been disabled by the BMC, in which case
+                    // assume the UART is already set up.
+                    Uart::initialize();
+                    break;
+                }
+
                 //  verify the boot flags version from register 0x28
                 l_err = deviceOp( DeviceFW::READ,
                                   TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL,
@@ -304,6 +316,7 @@ namespace CONSOLE
                                   l_len,
                                   DEVICE_SIO_ADDRESS(SIO::SUART1, 0x28));
                 if (l_err) { break; }
+
                 // is it the version we expected?
                 if( expected_version == this_version )
                 {
diff --git a/src/usr/initservice/bootconfig/bootconfig_ast2400.C b/src/usr/initservice/bootconfig/bootconfig_ast2400.C
index c7e0a3d8b..87caa32a5 100644
--- a/src/usr/initservice/bootconfig/bootconfig_ast2400.C
+++ b/src/usr/initservice/bootconfig/bootconfig_ast2400.C
@@ -212,6 +212,21 @@ errlHndl_t AST2400BootConfig::readAndProcessBootConfig()
     size_t l_len = sizeof(uint8_t);
     do
     {
+        // The BMC may have disabled SIO, in which case we use a default set of
+        // boot flags
+        bool haveSio;
+        l_err = SIO::isAvailable(haveSio);
+        if (l_err)
+        {
+            break;
+        }
+
+        if (!haveSio)
+        {
+            processBootFlagsV1(0);
+            break;
+        }
+
         // read the register holding the agreed upon magic
         // number to indicate registers have been configured
 
diff --git a/src/usr/lpc/lpcdd.C b/src/usr/lpc/lpcdd.C
index efcfa0346..48f56a1f2 100644
--- a/src/usr/lpc/lpcdd.C
+++ b/src/usr/lpc/lpcdd.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2014,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2014,2018                        */
 /* [+] Google Inc.                                                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
@@ -217,15 +217,13 @@ errlHndl_t lpcWrite(DeviceFW::OperationType i_opType,
     {
         //First check/clear the LPC bus of errors and commit any errors found
         l_err = Singleton<LpcDD>::instance().checkForLpcErrors();
-        if (l_err)
+        if (!l_err)
         {
-            errlCommit(l_err, LPC_COMP_ID);
+            l_err = Singleton<LpcDD>::instance().writeLPC( l_type,
+                                                           l_addr,
+                                                           io_buffer,
+                                                           io_buflen );
         }
-
-        l_err = Singleton<LpcDD>::instance().writeLPC( l_type,
-                                                       l_addr,
-                                                       io_buffer,
-                                                       io_buflen );
     }
     else
     {
diff --git a/src/usr/pnor/ast_mboxdd.C b/src/usr/pnor/ast_mboxdd.C
index b7504e934..5b6e58300 100644
--- a/src/usr/pnor/ast_mboxdd.C
+++ b/src/usr/pnor/ast_mboxdd.C
@@ -365,6 +365,11 @@ errlHndl_t astMbox::initializeMbox(void)
 
     do
     {
+        // The BMC may have disabled SIO access, but there's not much point in
+        // testing whether it's available as there's no alternative action if
+        // it is not. Instead, just try the SIO accesses and bail out with the
+        // errl if they fail.
+
         //First disable SIO Mailbox engine to configure it
         // 0x30 - Enable/Disable Reg
         l_data = SIO::DISABLE_DEVICE;
diff --git a/src/usr/pnor/sfc_ast2400.C b/src/usr/pnor/sfc_ast2400.C
index e8e612efa..aa0e2bb07 100644
--- a/src/usr/pnor/sfc_ast2400.C
+++ b/src/usr/pnor/sfc_ast2400.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2014,2016                        */
+/* Contributors Listed Below - COPYRIGHT 2014,2018                        */
 /* [+] Google Inc.                                                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
@@ -95,6 +95,11 @@ errlHndl_t SfcAST2400::hwInit( )
     uint32_t l_lpc_addr;
     do
     {
+        // The BMC may have disabled SIO access, but there's not much point in
+        // testing whether it's available as there's no alternative action if
+        // it is not. Instead, just try the SIO accesses and bail out with the
+        // errl if they fail.
+
         /* Enable device 0x0D*/
         uint8_t l_data = SIO::ENABLE_DEVICE;
         size_t l_len = sizeof(l_data);
diff --git a/src/usr/pnor/sfc_ast2500.C b/src/usr/pnor/sfc_ast2500.C
index 39465f9b2..9f4d88ae1 100644
--- a/src/usr/pnor/sfc_ast2500.C
+++ b/src/usr/pnor/sfc_ast2500.C
@@ -94,6 +94,11 @@ errlHndl_t SfcAST2500::hwInit( )
     uint32_t l_lpc_addr;
     do
     {
+        // The BMC may have disabled SIO access, but there's not much point in
+        // testing whether it's available as there's no alternative action if
+        // it is not. Instead, just try the SIO accesses and bail out with the
+        // errl if they fail.
+
         /* Enable device 0x0D*/
         uint8_t l_data = SIO::ENABLE_DEVICE;
         size_t l_len = sizeof(l_data);
diff --git a/src/usr/sio/siodd.C b/src/usr/sio/siodd.C
index 5301d9e1c..8bbc59f5b 100755
--- a/src/usr/sio/siodd.C
+++ b/src/usr/sio/siodd.C
@@ -37,6 +37,9 @@
 #include <hbotcompid.H>
 #include <stdarg.h>
 #include <targeting/common/target.H>
+#include <lpc/lpc_reasoncodes.H>
+
+#include <console/consoleif.H>
 
 // Trace definition
 trace_desc_t* g_trac_sio = NULL;
@@ -191,32 +194,64 @@ DEVICE_REGISTER_ROUTE( DeviceFW::WRITE,
                        TARGETING::TYPE_PROC,
                        ahbSioWriteDD );
 
-//function to unlock superIO password register
-void SioDD::unlock_SIO(TARGETING::Target* i_target)
+errlHndl_t SIO::isAvailable(bool& available)
 {
     uint8_t l_byte = SIO::SIO_PASSWORD_REG;
-    errlHndl_t l_err = NULL;
-    do
-    {
-        // Unlock the SIO registers
-        //  (write 0xA5 password to offset 0x2E two times)
     size_t l_len = sizeof(uint8_t);
-    l_err = deviceWrite(i_target,
-                        &l_byte,
-                        l_len,
-                        DEVICE_LPC_ADDRESS(LPC::TRANS_IO, SIO::SIO_ADDR_REG_2E));
-    if(l_err) { break; }
-    l_err = deviceWrite(i_target,
-                        &l_byte,
-                        l_len,
+    errlHndl_t l_err = NULL;
+
+    l_err = deviceWrite(TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL,
+                        &l_byte, l_len,
                         DEVICE_LPC_ADDRESS(LPC::TRANS_IO, SIO::SIO_ADDR_REG_2E));
-    } while(0);
 
     if (l_err)
     {
-        TRACFCOMP(g_trac_sio,"Error in unlocking SIO password register\n");
-        errlCommit(l_err, SIO_COMP_ID);
+        /* FIXME: The implementation assumes that any error indicates that the
+         * SIO device is not available. This, generally, is a terribe
+         * assumption. We should instead look for the specific failure, which
+         * is a LPC SYNC No Response (and this is what we see in skiboot).
+         * Currently there are open questions about the hardware behaviour as
+         * observed by hostboot: We see an OPBM Error Acknowledge state when we
+         * try to access an absent SIO device, but no error state is present in
+         * the LPCHC status register.
+         *
+         * We retain the interface of returning an errlHndl_t to future-proof
+         * the code. The caller should commit the errl if it is valid.
+         */
+        TRACFCOMP(g_trac_sio,
+                  "Received error during SIO availability test, assuming "
+                  "absent. Reason code: 0x%x, user data: [0x%8x, 0x%8x]",
+                  l_err->reasonCode(), l_err->getUserData1(),
+                  l_err->getUserData2());
+        available = false;
+        delete l_err;
+        l_err = NULL;
+    }
+    else
+    {
+        available = true;
     }
+
+    return l_err;
+}
+
+//function to unlock superIO password register
+errlHndl_t SioDD::unlock_SIO(TARGETING::Target* i_target)
+{
+    uint8_t l_byte = SIO::SIO_PASSWORD_REG;
+    size_t l_len = sizeof(uint8_t);
+    errlHndl_t l_err = NULL;
+    int again = 1;
+
+    do
+    {
+    // Unlock the SIO registers (write 0xA5 password to offset 0x2E two times)
+    l_err = deviceWrite(i_target, &l_byte, l_len,
+                        DEVICE_LPC_ADDRESS(LPC::TRANS_IO,
+                                           SIO::SIO_ADDR_REG_2E));
+    } while(!l_err && again--);
+
+    return l_err;
 }
 
 //SioDD constructor
@@ -225,7 +260,16 @@ SioDD::SioDD(TARGETING::Target* i_target)
     assert(i_target == TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL);
     mutex_init(&iv_sio_mutex);
     iv_prev_dev = 0x00;
-    unlock_SIO(i_target);
+
+    errlHndl_t err = unlock_SIO(i_target);
+    bool failed = (err != NULL);
+    delete err;
+
+    /* Unlocked very early, so make some noise if we fail */
+    if (failed)
+    {
+        printk("SuperIO unlock failed! Expect future errors\n");
+    }
 }
 
 //SioDD destructor
@@ -279,8 +323,7 @@ errlHndl_t SioDD::_readSIO(TARGETING::Target* i_target,
 //function to change logical device in SIO
 errlHndl_t SioDD::changeDevice(TARGETING::Target* i_target, uint8_t i_dev)
 {
-    uint8_t l_reg = SIO::SIO_DEVICE_SELECT_REG;
-    return _writeSIO(i_target, l_reg, &i_dev);
+    return _writeSIO(i_target, SIO::SIO_DEVICE_SELECT_REG, &i_dev);
 }
 
 //function to read from SIO register
diff --git a/src/usr/sio/siodd.H b/src/usr/sio/siodd.H
index 664935921..86821d466 100644
--- a/src/usr/sio/siodd.H
+++ b/src/usr/sio/siodd.H
@@ -167,8 +167,10 @@ class SioDD
         /**
          * @brief Unlock SIO password register
          * @param[in] i_target: SIO target
+         *
+         * @return Error from operation
          */
-        void unlock_SIO(TARGETING::Target* i_target);
+        errlHndl_t unlock_SIO(TARGETING::Target* i_target);
 
         /**
          * @brief Previous device accessed by SIO