NVDIMM: Fix issue where a deviceRead of a register is returning bad data

The reason the deviceRead is returning bad data, from an NVDIMM register,
is because the register data is segmented across several pages.  One must
first set the page correctly before attempting to do a read of a register.
This fix will set the page up correctly before doing a read/write to an
NVDIMM register.

Change-Id: Ic63c8671c0083f1ca243f255a29d1ef4606b7ce5
CQ: SW465917
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/81542
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Matthew Raybuck <matthew.raybuck@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>
Reviewed-by: Matt Derksen <mderkse1@us.ibm.com>
Reviewed-by: Daniel M Crowell <dcrowell@us.ibm.com>

5 files changed, 139 insertions, 7 deletions

diff --git a/src/include/usr/devicefw/userif.H b/src/include/usr/devicefw/userif.H
index d626d49c0..772511566 100644
--- a/src/include/usr/devicefw/userif.H
+++ b/src/include/usr/devicefw/userif.H
@@ -68,7 +68,8 @@ namespace DeviceFW
         AHB_SIO,      // AST Hostbridge via SIO
         DVPD,         // Direct access memory VPD
         NODECOMM,     // Internode communication
-        NVDIMM,       // Non-volatile DIMM controller access
+        NVDIMM,       // Routes message to non-volatile DIMM controller access
+        NVDIMM_RAW,   // Non-volatile DIMM controller access
         FAPI_I2C,     // FAPI2-triggered i2c accesses
         MMIO,         // Memory Mapped I/O
         IDEC,         // Read and set EC and CHIPID values
@@ -379,12 +380,30 @@ namespace DeviceFW
             static_cast<uint64_t>(( i_mailbox_id ))
 
     /**
-     * Construct the device addressing parameters for the NVDIMM device ops.
-     * @param[i] i_address - NVDIMM address to internal register
+     * @brief Construct the device addressing parameters for the
+     *        NVDIMM device ops.
+     * @details This call includes setting the page based off the address
+     *          and then performing the read/write of that NVDIMM address.
+     * @see DEVICE_NVDIMM_RAW_ADDRESS for an NVDIMM read/write call without
+     *                                page setting.
+     * @param[i] i_address - NVDIMM address to an internal register
      */
     #define DEVICE_NVDIMM_ADDRESS(i_address)\
         DeviceFW::NVDIMM, static_cast<uint64_t>((i_address))
 
+    /**
+     * @brief Construct the device addressing parameters for the
+     *        NVDIMM RAW device ops.
+     * @details This a raw call to read/write a NVDIMM address which means it
+     *          will not set the page before it does the read/write call. Hence,
+     *          for this call to work properly, the page must have been set
+     *          properly beforehand.
+     * @see DEVICE_NVDIMM_ADDRESS for a NVDIMM read/write call with
+     *                            page setting.
+     * @param[i] i_address - NVDIMM address to an internal register
+     */
+    #define DEVICE_NVDIMM_RAW_ADDRESS(i_address)\
+        DeviceFW::NVDIMM_RAW, static_cast<uint64_t>((i_address))
 
     /**
      * Construct the device addressing parameters for the FAPI I2C operation
diff --git a/src/usr/isteps/nvdimm/nvdimm.C b/src/usr/isteps/nvdimm/nvdimm.C
index 852a65a88..db26eb184 100644
--- a/src/usr/isteps/nvdimm/nvdimm.C
+++ b/src/usr/isteps/nvdimm/nvdimm.C
@@ -264,7 +264,7 @@ errlHndl_t nvdimmReadReg(Target* i_nvdimm,
                                     i_nvdimm,
                                     &o_data,
                                     l_numBytes,
-                                    DEVICE_NVDIMM_ADDRESS(l_reg_addr));
+                                    DEVICE_NVDIMM_RAW_ADDRESS(l_reg_addr));
     }while(0);
 
     TRACUCOMP(g_trac_nvdimm, EXIT_MRK"NVDIMM Read HUID 0x%X, page 0x%X, addr 0x%X = 0x%X",
@@ -334,7 +334,7 @@ errlHndl_t nvdimmWriteReg(Target* i_nvdimm,
                                     i_nvdimm,
                                     &i_data,
                                     l_numBytes,
-                                    DEVICE_NVDIMM_ADDRESS(l_reg_addr));
+                                    DEVICE_NVDIMM_RAW_ADDRESS(l_reg_addr));
     }while(0);
 
     TRACUCOMP(g_trac_nvdimm, EXIT_MRK"NVDIMM Write HUID 0x%X, page = 0x%X, addr 0x%X = 0x%X",
diff --git a/src/usr/isteps/nvdimm/nvdimm_update.C b/src/usr/isteps/nvdimm/nvdimm_update.C
index 3a732ef66..1f42228ce 100644
--- a/src/usr/isteps/nvdimm/nvdimm_update.C
+++ b/src/usr/isteps/nvdimm/nvdimm_update.C
@@ -1432,7 +1432,7 @@ errlHndl_t NvdimmInstalledImage::byteRegionBlockTransfer(const uint8_t * i_data,
                                         iv_dimm,
                                         pCurrentBlockData,
                                         l_numBytes,
-                                        DEVICE_NVDIMM_ADDRESS(l_reg_addr) );
+                                        DEVICE_NVDIMM_RAW_ADDRESS(l_reg_addr) );
             if (l_err)
             {
                 TRACFCOMP(g_trac_nvdimm_upd, ERR_MRK"byteRegionBlockTransfer: "
diff --git a/src/usr/isteps/nvdimm/nvdimmdd.C b/src/usr/isteps/nvdimm/nvdimmdd.C
index b2c8909d3..695a60b93 100755
--- a/src/usr/isteps/nvdimm/nvdimmdd.C
+++ b/src/usr/isteps/nvdimm/nvdimmdd.C
@@ -97,13 +97,83 @@ static bool errorIsRetryable(uint16_t reasonCode)
 namespace NVDIMM
 {
 
-// Register the perform Op with the routing code for DIMMs.
+// Register the perform Op router with the routing code for DIMMs.
 DEVICE_REGISTER_ROUTE( DeviceFW::WILDCARD,
                        DeviceFW::NVDIMM,
                        TARGETING::TYPE_DIMM,
+                       nvdimmPerformOpRouter  );
+
+// Register the perform Op with the routing code for DIMMs.
+DEVICE_REGISTER_ROUTE( DeviceFW::WILDCARD,
+                       DeviceFW::NVDIMM_RAW,
+                       TARGETING::TYPE_DIMM,
                        nvdimmPerformOp );
 
 // ------------------------------------------------------------------
+// nvdimmPerformOpRouter
+// ------------------------------------------------------------------
+errlHndl_t nvdimmPerformOpRouter( DeviceFW::OperationType i_opType,
+                                  TARGETING::Target * i_target,
+                                  void * io_buffer,
+                                  size_t & io_buflen,
+                                  int64_t i_accessType,
+                                  va_list i_args )
+{
+    errlHndl_t l_err(nullptr);
+
+    TRACDCOMP( g_trac_nvdimm,
+               ENTER_MRK"nvdimmPerformOpRouter()" );
+
+    // Get the NVDIMM register's address, where data will be accessed from
+    // Although the data is being retrieved as a 64 bit value
+    // it is really a 16 bit value.  Data passed via an arg list
+    // are retrieved in 64 bit chunks.
+    uint16_t l_registerAddress =
+            static_cast<uint16_t>(va_arg(i_args, uint64_t));
+
+    // Get a handle to the data buffer for easy referencing
+    uint8_t* l_data = static_cast<uint8_t*>(io_buffer);
+
+    TRACUCOMP(g_trac_nvdimm, INFO_MRK"nvdimmPerformOpRouter(): "
+              "operation type=%d, target HUID=0x%.8X, access type=%d, "
+              "buffer length=%d, buffer data=0x%.8X, register address=0x%.8X",
+              static_cast<uint64_t>(i_opType), get_huid(i_target), i_accessType,
+              io_buflen, *l_data, l_registerAddress);
+
+    // Make the right read/write call based on operation type
+    if( i_opType == DeviceFW::READ )
+    {
+        l_err = nvdimmReadReg( i_target,
+                               l_registerAddress,
+                               *l_data,
+                               PAGE_VERIFY);
+        if (!l_err)
+        {
+            TRACUCOMP (g_trac_nvdimm, INFO_MRK"nvdimmPerformOpRouter(): "
+                       "Read data(0x%X) from register(0x%X)",
+                       *l_data, l_registerAddress);
+        }
+    }
+    else if( i_opType == DeviceFW::WRITE )
+    {
+        TRACUCOMP (g_trac_nvdimm, INFO_MRK"nvdimmPerformOpRouter(): "
+                   "Writing data(0x%X) to register(0x%X) ...",
+                   *l_data, l_registerAddress);
+
+        l_err = nvdimmWriteReg( i_target,
+                                l_registerAddress,
+                                *l_data,
+                                PAGE_VERIFY);
+    }
+
+    TRACDCOMP(g_trac_nvdimm,
+              EXIT_MRK"nvdimmPerformOpRouter() returning with %s",
+              (l_err == nullptr ? "no error, success" : "an error, failure") );
+
+    return l_err;
+}
+
+// ------------------------------------------------------------------
 // nvdimmPerformOp
 // ------------------------------------------------------------------
 errlHndl_t nvdimmPerformOp( DeviceFW::OperationType i_opType,
diff --git a/src/usr/isteps/nvdimm/nvdimmdd.H b/src/usr/isteps/nvdimm/nvdimmdd.H
index 4d599b38a..1e299f2de 100755
--- a/src/usr/isteps/nvdimm/nvdimmdd.H
+++ b/src/usr/isteps/nvdimm/nvdimmdd.H
@@ -138,6 +138,49 @@ errlHndl_t nvdimmPerformOp( DeviceFW::OperationType i_opType,
                             int64_t i_accessType,
                             va_list i_args );
 
+/**
+*
+* @brief Route the read/write operator (i_opType) to the correct
+*        nvdimmReadReg/nvdimmWriteReg call.
+*
+* @details This is essentially a wrapper around the nvdimmPerformOp method
+*          which is called via the nvdimmReadReg/nvdimmWriteReg call. This
+*          ensures that the page is set correctly whenever a NVDIMM register
+*          is accessed.
+*
+* @param[in] i_opType - Operation Type - See DeviceFW::OperationType in
+*       driververif.H
+*
+* @param[in] i_target - Target device.
+*
+* @param[in/out] io_buffer
+*       INPUT: Pointer to the data that will be  written to the target
+*           device.
+*       OUTPUT: Pointer to the data that was read from the target device.
+*
+* @param[in/out] io_buflen
+*       INPUT: Length of the buffer to be written to target device.
+*       OUTPUT: Length of buffer that was written, or length of buffer
+*           to be read from target device.
+*
+* @param [in] i_accessType - Access Type - See DeviceFW::AccessType in
+*       usrif.H
+*
+* @param [in] i_args - This is an argument list for the device driver
+*       framework.  This argument list consists of the internal offset
+*       to use on the slave I2C device.
+*
+* @return errlHndl_t - NULL if successful, otherwise a pointer to the
+*       error log.
+*
+*/
+errlHndl_t nvdimmPerformOpRouter( DeviceFW::OperationType i_opType,
+                                  TARGETING::Target * i_target,
+                                  void * io_buffer,
+                                  size_t & io_buflen,
+                                  int64_t i_accessType,
+                                  va_list i_args );
+
 /*
  * @brief On the NV Controller, the page is selected by writing to offset
  *        0x00 with the page you would like to switch too. e.g. to activate