Enable rolling counter and verification of req id for Explorer

Rather than using a constant value based on which command is being
sent to the OCMB, we will instead define an attribute that is set
by the platform in some unique way to differentiate this particular
command/response from all others going on.  This will allow the
code to catch bugs where commands erroneously are sent to the wrong
target for some reason.

Change-Id: I3162a8ac040d8487e9311f770a0466ee8500b415
RTC: 210371
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/83851
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: PPE CI <ppe-ci+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Dev-Ready: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: ANDRE A MARIN <aamarin@us.ibm.com>
Reviewed-by: Matt K Light <mklight@us.ibm.com>
Reviewed-by: Jennifer A Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/84633
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: Daniel M Crowell <dcrowell@us.ibm.com>

src/import/chips/ocmb/explorer/procedures/hwp/memory/exp_draminit.C

@@ -55,6 +55,7 @@ extern "C"
         mss::display_git_commit_info("exp_draminit");
 
         uint32_t l_crc = 0;
+        host_fw_command_struct l_cmd;
 
         user_input_msdg l_phy_params;
         FAPI_TRY(mss::exp::setup_phy_params(i_target, l_phy_params),
@@ -64,10 +65,12 @@ extern "C"
         FAPI_TRY( mss::exp::ib::putUserInputMsdg(i_target, l_phy_params, l_crc),
                   "Failed putUserInputMsdg() for %s", mss::c_str(i_target) );
 
-        // Issue full boot mode cmd though EXP-FW REQ buffer
         {
-            host_fw_command_struct l_cmd;
-            mss::exp::setup_cmd_params(l_crc, sizeof(l_phy_params), l_cmd);
+            // Issue full boot mode cmd though EXP-FW REQ buffer
+            FAPI_TRY( mss::exp::setup_cmd_params(i_target,
+                                                 l_crc,
+                                                 sizeof(l_phy_params),
+                                                 l_cmd) );
             FAPI_TRY( mss::exp::ib::putCMD(i_target, l_cmd),
                       "Failed putCMD() for  %s", mss::c_str(i_target) );
         }
@@ -96,7 +99,7 @@ extern "C"
             FAPI_TRY( mss::exp::train::display_info(i_target, l_train_response));
 
             // Check if cmd was successful
-            l_rc = mss::exp::check::response(i_target, l_response);
+            l_rc = mss::exp::check::response(i_target, l_response, l_cmd);
 
             // If not, then we need to process the bad bitmap
             if(l_rc != fapi2::FAPI2_RC_SUCCESS)

src/import/chips/ocmb/explorer/procedures/hwp/memory/exp_fw_update.C

@@ -55,11 +55,13 @@ namespace bupg
 ///
 /// @brief Checks explorer response argument for a successful command
 /// @param[in] i_target OCMB target
-/// @param[in] i_rsp response command
+/// @param[in] i_rsp response from command
+/// @param[in] i_cmd original command
 /// @return FAPI2_RC_SUCCESS iff okay
 ///
 fapi2::ReturnCode check_response(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
-                                 const host_fw_response_struct& i_rsp)
+                                 const host_fw_response_struct& i_rsp,
+                                 const host_fw_command_struct& i_cmd)
 {
     std::vector<uint8_t> resp_arg;
     uint8_t  success_flag = 0;
@@ -76,16 +78,16 @@ fapi2::ReturnCode check_response(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHI
     FAPI_TRY(mss::exp::ib::readCrctEndian(resp_arg, index, err_code));
 
     // Check if cmd was successful
-    FAPI_ASSERT(success_flag == omi::response_arg::SUCCESS,
+    FAPI_ASSERT(success_flag == omi::response_arg::SUCCESS &&
+                i_rsp.request_identifier == i_cmd.request_identifier,
                 fapi2::EXP_UPDATE_CMD_FAILED().
                 set_TARGET(i_target).
                 set_RSP_ID(i_rsp.response_id).
                 set_REQ_ID(i_rsp.request_identifier).
                 set_ERROR_CODE(err_code).
                 set_RSP_DATA(i_rsp),
-                "Recieved failure response for firmware update command on %s,"
-                " with success_flag = 0x%01x and error_code = 0x%02x",
-                mss::c_str(i_target), success_flag, err_code);
+                "Recieved failure response for firmware update command on %s",
+                mss::c_str(i_target));
 
 fapi_try_exit:
     return fapi2::current_err;
@@ -101,7 +103,8 @@ fapi_try_exit:
 /// @param[out] o_cmd the command packet to update
 /// @return FAPI2_RC_SUCCESS iff ok
 ///
-fapi2::ReturnCode setup_flash_write_cmd(const uint32_t i_binary_size,
+fapi2::ReturnCode setup_flash_write_cmd(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
+                                        const uint32_t i_binary_size,
                                         const uint16_t i_seq_number,
                                         const uint32_t i_cmd_data_crc,
                                         host_fw_command_struct& o_cmd)
@@ -118,7 +121,9 @@ fapi2::ReturnCode setup_flash_write_cmd(const uint32_t i_binary_size,
     o_cmd.cmd_flags = mss::exp::omi::ADDITIONAL_DATA;
 
     // Host generated id number (returned in response packet)
-    o_cmd.request_identifier = 0xfed1;
+    uint32_t l_counter = 0;
+    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_OCMB_COUNTER, i_target, l_counter));
+    o_cmd.request_identifier = l_counter;
 
     //always send a block of data
     o_cmd.cmd_length = FLASH_WRITE_BLOCK_SIZE;
@@ -171,20 +176,23 @@ fapi_try_exit:
 /// @brief Sets the command_argument fields for flash_commit sub-command
 ///        in the correct endianness.
 ///
+/// @param[in] i_target OCMB target that will be acted upon with this command
 /// @param[out] o_cmd the command packet to update
 ///
-void setup_flash_commit_cmd(host_fw_command_struct& o_cmd)
+fapi2::ReturnCode setup_flash_commit_cmd(
+    const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
+    host_fw_command_struct& o_cmd)
 {
     memset(&o_cmd, 0, sizeof(host_fw_command_struct));
 
     // Issue EXP_FW_BINARY_UPGRADE cmd though EXP-FW REQ buffer
     o_cmd.cmd_id = mss::exp::omi::EXP_FW_BINARY_UPGRADE;
     o_cmd.cmd_flags = 0;
 
-    // Host generated id number (returned in response packet)
-    // NOTE: This is arbitrarily chosen until it is decided how we want to
-    //       use this field.
-    o_cmd.request_identifier = 0xfcfc;
+    // Retrieve a unique sequence id for this transaction
+    uint32_t l_counter = 0;
+    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_OCMB_COUNTER, i_target, l_counter));
+    o_cmd.request_identifier = l_counter;
     o_cmd.cmd_length = 0;
 
     o_cmd.cmd_crc = 0xffffffff;
@@ -194,6 +202,9 @@ void setup_flash_commit_cmd(host_fw_command_struct& o_cmd)
 
     // Set the sub-command ID in the command argument field to FLASH_COMMIT
     o_cmd.command_argument[0] = bupg::SUB_CMD_COMMIT;
+
+fapi_try_exit:
+    return fapi2::current_err;
 }
 
 }//exp
@@ -261,11 +272,11 @@ extern "C"
                                     buffer));
 
             // Issue flash_write sub-command through EXP-FW request buffer
+            host_fw_command_struct flash_write_cmd;
             {
-                host_fw_command_struct flash_write_cmd;
-
                 // Set up the command packet
                 FAPI_TRY(mss::exp::setup_flash_write_cmd(
+                             i_target,
                              i_image_sz,
                              seq_num,
                              block_crc,
@@ -300,7 +311,7 @@ extern "C"
                              mss::c_str(i_target), seq_num);
 
                     // Check status in response packet
-                    FAPI_TRY(mss::exp::bupg::check_response(i_target, response),
+                    FAPI_TRY(mss::exp::bupg::check_response(i_target, response, flash_write_cmd),
                              "exp_fw_update: error response for flash_write "
                              "on %s! seq_num[%u]",
                              mss::c_str(i_target), seq_num);
@@ -322,10 +333,11 @@ extern "C"
             seq_num++;
         }
 
+        host_fw_command_struct flash_commit_cmd;
         // Issue the flash_commit sub-command through EXP-FW request buffer
         {
-            host_fw_command_struct flash_commit_cmd;
-            mss::exp::setup_flash_commit_cmd(flash_commit_cmd);
+            FAPI_TRY(mss::exp::setup_flash_commit_cmd(i_target,
+                     flash_commit_cmd));
             FAPI_TRY(mss::exp::ib::putCMD(i_target, flash_commit_cmd),
                      "exp_fw_update: putCMD() failed for flash_commit on %s!",
                      mss::c_str(i_target));
@@ -353,7 +365,7 @@ extern "C"
                      mss::c_str(i_target) );
 
             // Check if cmd was successful
-            FAPI_TRY(mss::exp::bupg::check_response(i_target, response),
+            FAPI_TRY(mss::exp::bupg::check_response(i_target, response, flash_commit_cmd),
                      "exp_fw_update: error response for flash_commit on %s!",
                      mss::c_str(i_target) );
         }

src/import/chips/ocmb/explorer/procedures/hwp/memory/exp_fw_update.H

@@ -85,22 +85,26 @@ typedef enum sub_cmd_id
 /// @brief Checks explorer response argument for a successful command
 /// @param[in] i_target OCMB target
 /// @param[in] i_rsp response command
+/// @param[in] i_cmd original command
 /// @return FAPI2_RC_SUCCESS iff okay
 ///
 fapi2::ReturnCode check_response(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
-                                 const host_fw_response_struct& i_rsp);
+                                 const host_fw_response_struct& i_rsp,
+                                 const host_fw_command_struct& i_cmd);
 
 }// ns bupg
 
 ///
 /// @brief host_fw_command_struct structure setup for flash_write
+/// @param[in] i_target OCMB target that will be acted upon with this command
 /// @param[in] i_binary_size the total size of the binary image
 /// @param[in] i_seq_number the sequence number of this command
 /// @param[in] i_cmd_data_crc the command data CRC
 /// @param[out] o_cmd the command packet to update
 /// @return FAPI2_RC_SUCCESS iff ok
 ///
-fapi2::ReturnCode setup_flash_write_cmd(const uint32_t i_binary_size,
+fapi2::ReturnCode setup_flash_write_cmd(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
+                                        const uint32_t i_binary_size,
                                         const uint16_t i_seq_number,
                                         const uint32_t i_cmd_data_crc,
                                         host_fw_command_struct& o_cmd);
@@ -109,9 +113,12 @@ fapi2::ReturnCode setup_flash_write_cmd(const uint32_t i_binary_size,
 /// @brief Sets the command_argument fields for flash_commit sub-command
 ///        in the correct endianness.
 ///
+/// @param[in] i_target OCMB target that will be acted upon with this command
 /// @param[out] o_cmd the command packet to update
+/// @return FAPI2_RC_SUCCESS iff ok
 ///
-void setup_flash_commit_cmd(host_fw_command_struct& o_cmd);
+fapi2::ReturnCode setup_flash_commit_cmd(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
+        host_fw_command_struct& o_cmd);
 
 }// ns exp
 }// ns mss

src/import/chips/ocmb/explorer/procedures/hwp/memory/lib/exp_draminit_utils.C

@@ -42,11 +42,13 @@ namespace exp
 
 ///
 /// @brief host_fw_command_struct structure setup
+/// @param[in] i_target the OCMB being acted upon
 /// @param[in] i_cmd_data_crc the command data CRC
 /// @param[in] i_cmd_length the length of the command present in the data buffer (if any)
 /// @param[out] o_cmd the command parameters to set
 ///
-void setup_cmd_params(
+fapi2::ReturnCode setup_cmd_params(
+    const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
     const uint32_t i_cmd_data_crc,
     const uint8_t i_cmd_length,
     host_fw_command_struct& o_cmd)
@@ -56,12 +58,20 @@ void setup_cmd_params(
     // Explicit with all of these (including 0 values) to avoid ambiguity
     o_cmd.cmd_id = mss::exp::omi::EXP_FW_DDR_PHY_INIT;
     o_cmd.cmd_flags = 0;
-    // TK - Fabricated value need to figure out if we'll be creating req_id tables
-    o_cmd.request_identifier = 0xBB;
+
+    // Retrieve a unique sequence id for this transaction
+    uint32_t l_counter = 0;
+    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_OCMB_COUNTER, i_target, l_counter));
+
+    o_cmd.request_identifier = l_counter;
     o_cmd.cmd_length = i_cmd_length;
     o_cmd.cmd_crc = i_cmd_data_crc;
     o_cmd.host_work_area = 0;
     o_cmd.cmd_work_area = 0;
+    memset(o_cmd.padding, 0, sizeof(o_cmd.padding));
+
+fapi_try_exit:
+    return fapi2::current_err;
 }
 
 ///
@@ -162,15 +172,20 @@ namespace check
 /// @return FAPI2_RC_SUCCESS iff okay
 ///
 fapi2::ReturnCode response(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
-                           const host_fw_response_struct& i_rsp)
+                           const host_fw_response_struct& i_rsp,
+                           const host_fw_command_struct& i_cmd)
 {
     // Check if cmd was successful
-    FAPI_ASSERT(i_rsp.response_argument[0] == omi::response_arg::SUCCESS,
+    FAPI_ASSERT(i_rsp.response_argument[0] == omi::response_arg::SUCCESS &&
+                i_rsp.request_identifier == i_cmd.request_identifier,
                 fapi2::MSS_EXP_RSP_ARG_FAILED().
                 set_TARGET(i_target).
                 set_RSP_ID(i_rsp.response_id).
-                set_ERROR_CODE(i_rsp.response_argument[1]),
-                "Failed to initialize the PHY for %s", mss::c_str(i_target));
+                set_ERROR_CODE(i_rsp.response_argument[1]).
+                set_EXPECTED_REQID(i_cmd.request_identifier).
+                set_ACTUAL_REQID(i_rsp.request_identifier),
+                "Failed to initialize the PHY for %s, response=0x%X",
+                mss::c_str(i_target), i_rsp.response_argument[0]);
 
     return fapi2::FAPI2_RC_SUCCESS;
 

src/import/chips/ocmb/explorer/procedures/hwp/memory/lib/exp_draminit_utils.H

@@ -150,11 +150,14 @@ enum odt_fields
 
 ///
 /// @brief host_fw_command_struct structure setup
+/// @param[in] i_target the OCMB being acted upon
 /// @param[in] i_cmd_data_crc the command data CRC
 /// @param[in] i_cmd_length the length of the command present in the data buffer (if any)
 /// @param[out] o_cmd the command parameters to set
+/// @return FAPI2_RC_SUCCESS iff okay
 ///
-void setup_cmd_params(
+fapi2::ReturnCode setup_cmd_params(
+    const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
     const uint32_t i_cmd_data_crc,
     const uint8_t i_cmd_length,
     host_fw_command_struct& o_cmd);
@@ -1147,11 +1150,13 @@ namespace check
 ///
 /// @brief Checks explorer response argument for a successful command
 /// @param[in] i_target OCMB target
-/// @param[in] i_rsp response command
+/// @param[in] i_rsp response from command
+/// @param[in] i_cmd original command
 /// @return FAPI2_RC_SUCCESS iff okay
 ///
 fapi2::ReturnCode response(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_target,
-                           const host_fw_response_struct& i_rsp);
+                           const host_fw_response_struct& i_rsp,
+                           const host_fw_command_struct& i_cmd);
 
 }//check
 

src/import/chips/ocmb/explorer/procedures/xml/error_info/mss_exp_errors.xml

@@ -460,6 +460,8 @@
     </description>
     <ffdc>RSP_ID</ffdc>
     <ffdc>ERROR_CODE</ffdc>
+    <ffdc>EXPECTED_REQID</ffdc>
+    <ffdc>ACTUAL_REQID</ffdc>
     <callout>
       <procedure>CODE</procedure>
       <priority>MEDIUM</priority>

src/import/chips/p9/procedures/xml/attribute_info/p9a_io_attributes.xml

@@ -5,7 +5,7 @@
 <!--                                                                        -->
 <!-- OpenPOWER HostBoot Project                                             -->
 <!--                                                                        -->
-<!-- Contributors Listed Below - COPYRIGHT 2018                             -->
+<!-- Contributors Listed Below - COPYRIGHT 2018,2019                        -->
 <!-- [+] International Business Machines Corp.                              -->
 <!--                                                                        -->
 <!--                                                                        -->
@@ -31,7 +31,7 @@
     </targetType>
     <description>
        An OMI target's relative logical postion to its OMIC parent target.
-      
+
        pos | DL_GROUP_POS
       -----+--------------
          4 | 0
@@ -48,7 +48,7 @@
         15 | 0
         10 | 1
         11 | 2
-         8 | 0 
+         8 | 0
          9 | 1
 
     </description>
@@ -63,7 +63,7 @@
       TARGET_TYPE_OMI
     </targetType>
     <description>
-      An OMI target's logical DL number 
+      An OMI target's logical DL number
 
        pos | DL_NUM
       -----+--------------
@@ -90,4 +90,17 @@
     <default>0xFF</default><!-- Ensures platform explicitly puts a valid number in here -->
   </attribute>
   <!-- ******************************************************************************** -->
+  <attribute>
+    <id>ATTR_OCMB_COUNTER</id>
+    <targetType>
+      TARGET_TYPE_OCMB_CHIP
+    </targetType>
+    <description>
+        Tracks the sequence id for OCMB command transactions. The platform is
+        expected to guarantee a unique value on each read.
+    </description>
+    <valueType>uint32</valueType>
+    <platInit/>
+    <default>0</default>
+  </attribute>
 </attributes>