Added new GenericFspMboxMessage_t interface to save NVDIMM keys

* Added struct AttributeSetter_t to be sent as a GenericFspMboxMessage_t
  message via a firmware request.
* Created 'sendAttributes(const vector<Attribute>&)' to pack the
  attribute data into the message to be sent.
* Moved AttributeTank::Attribute from the private area to the public
  area so it can be used outside of class AttributeTank.
* Added more documentation on how the process of adding and creating
  messages is done.

Change-Id: Id592b6460a6f1f12a75c01b5a131bcf492480d8b
RTC:208343
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/77951
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Reviewed-by: Matthew Raybuck <matthew.raybuck@ibm.com>
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>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

4 files changed, 353 insertions, 36 deletions

diff --git a/src/include/runtime/README.md b/src/include/runtime/README.md
new file mode 100755
index 000000000..b26c51901
--- /dev/null
+++ b/src/include/runtime/README.md
@@ -0,0 +1,81 @@
+# interface.h::hostInterfaces::hbrt_fw_msg
+How to create an HBRT to FW request message interface
+    0) If passing an HBRT to FSP via MBOX or receiving a firmware notify message,
+       then use instruction 'generic_hbrt_fsp_message.H::GenericFspMboxMessage_t'
+       below.
+    1) The biggest part will be defining the interface.  Inspect the current
+       interfaces (req_hcode_update, error_log, etc) for inspiration.
+    2) Once an interface has been designed, add it to the anonymous
+       hbrt_fw_msg::union, among the other interfaces.
+    3) Append a new hbrt_fw_msg::io_type, that will be used to
+       identify the interface.
+    4) How to use the new interface to pass a message
+       a) Make sure 'g_hostInterfaces' and 'g_hostInterfaces->firmware_request'
+          are not NULL.
+       b) Create the firmware_request request struct (hostInterfaces::hbrt_fw_msg)
+          to send data.
+       c) Populate the firmware_request request struct with data.
+       b) Create the firmware_request response struct (hostInterfaces::hbrt_fw_msg)
+          to retrieve data. 'Zero' it out.  Currently, this appears to be not
+          used, but is needed for the firmware request call.
+       d) Make the firmware_request call via method 'firmware_request_helper'
+       Examples:
+            ~/src/usr/sbeio/runtime/sbeio_vital_attn.C::vital_attn_inform_opal
+            ~/src/usr/isteps/nvdimm/runtime/nvdimm_rt.C::notifyNvdimmProtectionChange
+            ~/src/usr/isteps/pm/runtime/rt_pm.C::hcode_update
+            ~/src/usr/errl/runtime/rt_errlmanager.C::sendMboxMsg
+    5) Update /hostboot/src/usr/util/runtime/rt_fwreq_helper.C::firmware_request_helper,
+       to capture data from the request if an error occurs.  Capture data in the
+       TWO switch statements 'switch (l_req_fw_msg->io_type)'.  Look at others
+       for examples.
+
+generic_hbrt_fsp_message.H::GenericFspMboxMessage_t
+    Firm Ware request:
+        1) The biggest part will be defining the interface.  Inspect the current
+           interfaces (AttributeSetter_t, SingleScomOpHbrtFspData_t,
+           TargetDeconfigHbrtFspData_t, etc) for inspiration.
+        2) Once an interface has been designed, add the structure to this file
+           with the other interfaces.
+        3) Create an MBOX message queue enum for the interface and add to:
+           /hostboot/src/include/usr/mbox/mbox_queues.H::queue_id_t
+           see current message queues for example
+        4) Add a new message type for the interface to:
+           enum generic_hbrt_fsp_message.H::GENERIC_FSP_MBOX_MESSAGE_MSG_TYPE.
+        5) How to use the new interface to pass a message
+           a) Make sure g_hostInterfaces and g_hostInterfaces->firmware_request
+              are not NULL.
+           b) Determine size of data.  It could be as simple as the size of the
+              structure itself.
+           c) Use createGenericFspMsg to create the messages for you.
+           c) Populate the firmware_request request struct with data.
+           d) Make the firmware_request call via method
+              firmware_request_helper
+           Examples:
+                ~/src/usr/hwas/hwasPlatDeconfigGard.C::platPostDeconfigureTarget
+                ~/src/usr/fsiscom/runtime/rt_fsiscom.C::sendScomOpToFsp
+                ~/src/usr/fsiscom/runtime/rt_fsiscom.C::sendMultiScomReadToFsp
+                ~/src/usr/hwas/hwasPlatDeconfigGard.C::DeconfigGard::platPostDeconfigureTarget
+
+    Firm Ware notify:
+        1) The biggest part will be defining the interface.  Inspect the current
+           interfaces (sbeRetryReqData_t, HbrtAttrSyncData_t, etc) for inspiration.
+        2) Once an interface has been designed, add the structure to this file
+           with the other interfaces.
+        3) Since this is a message sent from the HWSV team, you will need for
+           them to provide the message queue and message type, once you have
+           this info
+           a) Add the message queue to:
+              /hostboot/src/include/usr/mbox/mbox_queues.H::queue_id_t
+           b) Add the message type to:
+              enum generic_hbrt_fsp_message.H::GENERIC_FSP_MBOX_MESSAGE_MSG_TYPE
+        4) Create a method to process the notify call in file:
+           ~/src/usr/util/runtime/rt_fwnotify.C.  This method is where the
+           interface in step 1 will be used.
+           Examples:
+                ~/src/usr/util/runtime/rt_fwnotify.C::sbeAttemptRecovery
+                ~/src/usr/util/runtime/rt_fwnotify.C::occActiveNotification
+                ~/src/usr/util/runtime/rt_fwnotify.C::attrSyncRequest
+        5) Update the case statement 'switch (l_hbrt_fw_msg->io_type)' found in
+           method ~/src/usr/util/runtime/rt_fwnotify.C::firmware_notify to
+           call method created in step 4.
+
diff --git a/src/include/runtime/generic_hbrt_fsp_message.H b/src/include/runtime/generic_hbrt_fsp_message.H
index 3d2f48c0a..f3d252ae2 100644
--- a/src/include/runtime/generic_hbrt_fsp_message.H
+++ b/src/include/runtime/generic_hbrt_fsp_message.H
@@ -30,10 +30,14 @@
 
 
 /** @file generic_hbrt_fsp_message.H
+ *
  *  @brief A generic structure for passing data
  *
  *  This file has a generic struct to be used by the
  *  FSP/HWSV team to send and receive data.
+ *
+ *  @note  See README.md file on how to create an HBRT to FW
+ *         request/notify message interface
  */
 
 // Sentinel value for bad SCOM reads
@@ -54,12 +58,42 @@ private:
 };
 
 
+// Latest attribute setter (struct AttributeSetter_t) version
+enum ATTRIBUTE_SETTER_VERSION: uint8_t
+{
+    ATTRIBUTE_STRUCT_VERSION_FIRST = 0x01,
+    ATTRIBUTE_STRUCT_VERSION_LATEST = ATTRIBUTE_STRUCT_VERSION_FIRST,
+};
+
+/**
+  * A useful struct to serialize/deserialize the Attributes data
+  * from GenericFspMboxMessage_t::data.
+  */
+struct AttributeSetter_t // a firmware_request call
+{
+    ATTRIBUTE_SETTER_VERSION iv_structVersion;// latest attribute setter version
+    uint8_t  iv_reserved;
+    uint16_t iv_numAttributes; // number of attributes being sent
+    uint8_t  iv_attrData[];    // points to a stream of
+                               // AttributeTank::Attribute structures
+    // A method to set the local vars to a default state
+    void initialize()
+    {
+        iv_structVersion = ATTRIBUTE_STRUCT_VERSION_LATEST;
+        iv_reserved = 0;
+        iv_numAttributes = 0;
+    };
+} PACKED ;
+
 /**
   * A useful struct to pack/access the HUID and HWAS state
   * from the GenericFspMboxMessage_t.data.
   * The HUID will be in the first 4 bytes followed by the HWAS state.
+  *
+  * @note message queue = MBOX::FSP_TARG_DECONFIG_MSGQ;
+  *       message type  = MSG_DECONFIG_TARGET
   */
-struct TargetDeconfigHbrtFspData_t
+struct TargetDeconfigHbrtFspData_t  // a firmware_request call
 {
    uint32_t huid;
    TARGETING::HwasState hwasState;
@@ -70,8 +104,11 @@ struct TargetDeconfigHbrtFspData_t
   * This struct contains the message-specific data for
   * MSG_SINGLE_SCOM_OP calls to the FSP.
   * It shows the format of GenericFspMboxMessage_t.data.
+  *
+  * @note message queue = MBOX::FSP_SCOM_OPS_MSGQ
+  *       message type  = MSG_SINGLE_SCOM_OP
   */
-struct SingleScomOpHbrtFspData_t
+struct SingleScomOpHbrtFspData_t // a firmware_request call
 {
     uint8_t     scom_op;    // DeviceFW::READ, DeviceFW::WRITE
     uint32_t    huid;       // hardware target
@@ -85,8 +122,11 @@ struct SingleScomOpHbrtFspData_t
   * This struct contains the message-specific data for
   * MSG_MULTI_SCOM_OP calls to the FSP.
   * It shows the format of GenericFspMboxMessage_t.data.
+  *
+  * @note message queue = MBOX::FSP_SCOM_OPS_MSGQ
+  *       message type  = MSG_MULTI_SCOM_OP
   */
-struct MultiScomReadHbrtFspData_t
+struct MultiScomReadHbrtFspData_t  // a firmware_request call
 {
     uint32_t    huid;       // hardware target
     uint8_t     scom_num;   // number of SCOMs to read
@@ -107,8 +147,11 @@ struct MultiScomReadHbrtFspData_t
 /**
   * A useful struct to access the PLID from GenericFspMboxMessage_t.data
   * in the case where the FSP is requesting Hostboot to restart a SBE
+  *
+  * @note message queue = MBOX::FSP_SCOM_OPS_MSGQ
+  *       message type  = MSG_SBE_ERROR
   */
-struct SbeRetryReqData_t
+struct SbeRetryReqData_t  // a firmware_notify call
 {
    uint32_t huid;
    uint32_t plid;
@@ -122,8 +165,10 @@ struct SbeRetryReqData_t
   * The HUID will be the first 4 bytes followed by a 4-byte attribute ID,
   * then a 4-byte size of attribute data, and finally the attribute's data
   *
+  * @note message queue = MBOX::HB_ATTR_SYNC_MSGQ
+  *       message type  = MSG_ATTR_SYNC_REQUEST
   */
-struct HbrtAttrSyncData_t
+struct HbrtAttrSyncData_t  // a firmware_notify call
 {
     // HUID of target on which we want to update the attribute
     uint32_t huid;
@@ -187,14 +232,15 @@ struct GenericFspMboxMessage_t
         MSG_TOD_BACKUP_RESET             = 0x0001,
         MSG_TOD_BACKUP_RESET_INFORM_PHYP = 0x0002,
         MSG_TOD_TOPOLOGY_DATA            = 0x0003,
-        MSG_DECONFIG_TARGET              = 0x0004,
-        MSG_SINGLE_SCOM_OP               = 0x0005,
-        MSG_MULTI_SCOM_OP                = 0x0006,
-        MSG_ATTR_SYNC_REQUEST            = 0x000000A5,
+        MSG_DECONFIG_TARGET              = 0x0004,  // for TargetDeconfigHbrtFspData_t
+        MSG_SINGLE_SCOM_OP               = 0x0005,  // for SingleScomOpHbrtFspData_t
+        MSG_MULTI_SCOM_OP                = 0x0006,  // for MultiScomReadHbrtFspData_t
+        MSG_ATTR_WRITE_OP                = 0x0007,  // for AttributeSetter_t
+        MSG_ATTR_SYNC_REQUEST            = 0x000000A5, // for HbrtAttrSyncData_t
         MSG_OCC_ACTIVE                   = 0x000000A6,
-        MSG_SBE_ERROR                    = 0x000000D4,
-        MSG_SBE_RECOVERY_SUCCESS         = 0x000000D5,
-        MSG_SBE_RECOVERY_FAILED          = 0x000000D6,
+        MSG_SBE_ERROR                    = 0x000000D4, // for SbeRetryReqData_t
+        MSG_SBE_RECOVERY_SUCCESS         = 0x000000D5, // associated with SbeRetryReqData_t
+        MSG_SBE_RECOVERY_FAILED          = 0x000000D6, // associated with SbeRetryReqData_t
     };
 
     /**
diff --git a/src/include/runtime/hbrt_utilities.H b/src/include/runtime/hbrt_utilities.H
index 649677422..87a9cc708 100644
--- a/src/include/runtime/hbrt_utilities.H
+++ b/src/include/runtime/hbrt_utilities.H
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2017,2018                        */
+/* Contributors Listed Below - COPYRIGHT 2017,2019                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -29,7 +29,16 @@
 
 #define HBRT_TRACE_NAME "HBRT"
 
-#include "interface.h"
+extern trace_desc_t *g_trac_runtime;
+
+#include <vector>
+
+#include "interface.h"                      // hostInterfaces
+#include "generic_hbrt_fsp_message.H"       // GenericFspMboxMessage_t, AttributeSetter_t
+#include "targeting/common/attributeTank.H" // TARGETING::AttributeTank::Attribute
+#include <initservice/initserviceif.H>      // INITSERVICE
+#include "util/runtime/rt_fwreq_helper.H"   // firmware_request_helper
+#include "runtime/runtime_reasoncodes.H"    // MOD_XXX and RC_XXX
 
 /** @file hbrt_utilities.H
  *  @brief A file to put HBRT Interface utilities
@@ -54,9 +63,9 @@
  *          to newly created data.)
  *
  *  @post   The request/response messages point to a valid struct,
- *          the request/response message size are equal to each other
- *          and contain the size of the request/response messages
- *          respectively upon a successful call else all the output
+ *          the request/response messages' size are equal to each other, request
+ *          message is initalized (data size is set) and response message is
+ *          zeroed out, upon a successful call else all the output
  *          parameters will either be NULL or 0 based on type.
  *
  *  @note   Use this function iff hbrt_fw_msg::io_type is of type
@@ -65,7 +74,7 @@
  *  @note   Caller is responsible for deleting (use delete []) the
  *          allocated memory
  *
- *  @param[in] i_fspReqPayloadSize  The size of the payload that will
+ *  @param[in] i_fspReqPayloadSize The size of the payload that will
  *                                 populate GenericFspMboxMessage_t::data
  *  @param[out] o_fspMsgSize       Return the size of the
  *                                 GenericFspMboxMessage_t, adjusted to
@@ -76,14 +85,19 @@
  *                                 will be equal to the request msg size
  *  @param[out] o_responseMsg      The allocated response message (not
  *                                 NULL), zeroed out
+ *  @return true if NO issue allocating memory for request/response messages,
+ *          else false
  */
-void createGenericFspMsg(uint32_t i_fspReqPayloadSize,
-                       uint32_t &o_fspMsgSize,
-                       uint64_t &o_requestMsgSize,
-                       hostInterfaces::hbrt_fw_msg* &o_requestMsg,
-                       uint64_t &o_responseMsgSize,
-                       hostInterfaces::hbrt_fw_msg* &o_responseMsg)
+bool createGenericFspMsg(uint32_t i_fspReqPayloadSize,
+                         uint32_t &o_fspMsgSize,
+                         uint64_t &o_requestMsgSize,
+                         hostInterfaces::hbrt_fw_msg* &o_requestMsg,
+                         uint64_t &o_responseMsgSize,
+                         hostInterfaces::hbrt_fw_msg* &o_responseMsg)
 {
+   // Default the return value to true, assume things will go right
+   bool l_retVal(true);
+
    // Do some quick initialization of the output data
    o_fspMsgSize = o_requestMsgSize = o_responseMsgSize = 0;
    o_requestMsg = o_responseMsg = nullptr;
@@ -111,7 +125,7 @@ void createGenericFspMsg(uint32_t i_fspReqPayloadSize,
    o_requestMsg = reinterpret_cast<hostInterfaces::hbrt_fw_msg *>
                      (new uint8_t[o_requestMsgSize]);
 
-   // If anyone of these two message's memory can't be allocated, then
+   // If any one of these two message's memory can't be allocated, then
    // delete both messages (in case one did allocate memory), set both
    // messages to NULL pointers and set their respective sizes to zero.
    if (!o_responseMsg || !o_requestMsg)
@@ -123,6 +137,10 @@ void createGenericFspMsg(uint32_t i_fspReqPayloadSize,
       // Return output data zeroed out
       o_responseMsg = o_requestMsg = nullptr;
       o_fspMsgSize = o_requestMsgSize = o_responseMsgSize = 0;
+
+      // Return false, indicating that this function had an issue creating
+      // the request and/or response message
+      l_retVal = false;
    }
    else
    {
@@ -135,7 +153,175 @@ void createGenericFspMsg(uint32_t i_fspReqPayloadSize,
       o_requestMsg->generic_msg.dataSize = o_fspMsgSize;
       o_requestMsg->generic_msg.__req = GenericFspMboxMessage_t::REQUEST;
    }
+
+   return l_retVal;
 }  // end createGenericFspMsg
 
+
+/**
+ *  @brief  Serializes a list of Attributes to be sent to FSP
+ *
+ *  @param[in] i_attributeList  List of attributes to send to FSP
+ *
+ *  @return - error, NULL if no error
+ */
+errlHndl_t sendAttributes(const std::vector<TARGETING::AttributeTank::Attribute>&
+                          i_attributeList)
+{
+    TRACFCOMP(g_trac_runtime,
+              ENTER_MRK"sendAttributes - number of attributes to send %d",
+              i_attributeList.size());
+
+    // Handle to error log
+    errlHndl_t l_err{nullptr};
+
+    // Handles to the firmware messages
+    hostInterfaces::hbrt_fw_msg *l_fwRequestMsg{nullptr};   // request message
+    hostInterfaces::hbrt_fw_msg *l_fwResponseMsg{nullptr};  // response message
+
+    do
+    {
+        // If caller passes in an empty list, then nothing to do
+        if (!i_attributeList.size())
+        {
+            TRACFCOMP(g_trac_runtime, "sendAttributes: attribute list is empty,"
+                      "nothing to do ...");
+            break;
+        }
+
+        // Make sure mbox is available
+        assert(INITSERVICE::spBaseServicesEnabled(),
+               "ERROR: Cannot perform request 'sendAttributes', mbox is not available");
+
+        // Make sure we have all of our function pointers setup right
+        if ((nullptr == g_hostInterfaces) ||
+            (nullptr == g_hostInterfaces->firmware_request))
+        {
+            TRACFCOMP(g_trac_runtime, ERR_MRK"sendAttributes: "
+                      "Hypervisor firmware_request interface not linked");
+
+            /*@
+             * @errortype
+             * @severity     ERRL_SEV_UNRECOVERABLE
+             * @moduleid     MOD_RT_FIRMWARE_REQUEST
+             * @reasoncode   RC_FW_REQUEST_RT_NULL_PTR
+             * @userdata1    Number of Attributes to serialize and send
+             * @devdesc      Hypervisor firmware request interface not linked
+             * @custdesc     Internal firmware error
+             */
+            l_err = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE,
+                                            RUNTIME::MOD_RT_FIRMWARE_REQUEST,
+                                            RUNTIME::RC_FW_REQUEST_RT_NULL_PTR,
+                                            i_attributeList.size(),
+                                            0,
+                                            ERRORLOG::ErrlEntry::ADD_SW_CALLOUT);
+
+            break;
+        }
+
+        /// Calculate the size requirements needed to serialize the Attribute info
+        // Start with the size of 'struct AttributeSetter_t' itself
+        uint16_t l_dataSize(sizeof(AttributeSetter_t));
+
+        // Then add in the individual attributes themselves
+        for (auto l_attribute: i_attributeList)
+        {
+            // Add in the size of `struct AttributeHeader`
+            l_dataSize += sizeof(l_attribute.iv_hdr);
+            // Finally add in the size of the attribute value
+            l_dataSize += l_attribute.iv_hdr.iv_valSize;
+        }
+
+        // Create and initialize to zero a few needed variables
+        uint32_t l_fullFspDataSize(0);
+        uint64_t l_fwRequestMsgSize(0), l_fwResponseMsgSize(0);
+
+        // Create the dynamic firmware messages
+        if (!createGenericFspMsg(l_dataSize,
+                                 l_fullFspDataSize,
+                                 l_fwRequestMsgSize,
+                                 l_fwRequestMsg,
+                                 l_fwResponseMsgSize,
+                                 l_fwResponseMsg) )
+        {
+            TRACFCOMP(g_trac_runtime, ERR_MRK"sendAttributes: "
+                      "Unable to allocate firmware request messages");
+
+            /*@
+             * @errortype
+             * @severity     ERRL_SEV_UNRECOVERABLE
+             * @moduleid     MOD_SEND_ATTRIBUTES_TO_FSP
+             * @reasoncode   RC_NULL_FIRMWARE_MSG_PTR
+             * @userdata1    Number of Attributes to serialize and send
+             * @devdesc      Unable to allocate firmware request messages
+             * @custdesc     Internal firmware error
+             */
+            l_err = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE,
+                                            RUNTIME::MOD_SEND_ATTRIBUTES_TO_FSP,
+                                            RUNTIME::RC_NULL_FIRMWARE_MSG_PTR,
+                                            i_attributeList.size(),
+                                            0,
+                                            ERRORLOG::ErrlEntry::ADD_SW_CALLOUT);
+
+            break;
+        }
+
+        // Populate the 'message queue' and 'message type' for this message
+        l_fwRequestMsg->generic_msg.msgq = MBOX::FSP_NVDIMM_KEYS_MSGQ_ID;
+        l_fwRequestMsg->generic_msg.msgType =
+                                   GenericFspMboxMessage_t::MSG_DECONFIG_TARGET;
+
+        // Create a useful struct to populate the generic_msg::data field
+        AttributeSetter_t* l_attributeSetter =
+                                reinterpret_cast<AttributeSetter_t*>
+                                        (&(l_fwRequestMsg->generic_msg.data));
+
+        // Initialize the AttributeSetter to default values
+        l_attributeSetter->initialize();
+
+        // The number of attributes being copied can be obtained from
+        // size of the attrbute input list
+        l_attributeSetter->iv_numAttributes = i_attributeList.size();
+
+        // Retrieve the individual attributes (header and value)
+        // Create a useful struct to poulate attribute data
+        uint8_t* l_attributeData = l_attributeSetter->iv_attrData;
+        for (const auto & l_attribute: i_attributeList)
+        {
+            // Copy the attribute header
+            memcpy(l_attributeData,
+                   &(l_attribute.iv_hdr),
+                   sizeof(l_attribute.iv_hdr));
+            // Advance pointer, one beyond the attribute header
+            l_attributeData += sizeof(l_attribute.iv_hdr);
+
+            // Copy the attribute value
+            memcpy(l_attributeData,
+                   l_attribute.iv_pVal,
+                   l_attribute.iv_hdr.iv_valSize);
+            // Advance pointer, one beyond the attribute value
+            l_attributeData += l_attribute.iv_hdr.iv_valSize;
+        }
+
+        // Make the firmware_request call
+        l_err = firmware_request_helper(l_fwRequestMsgSize,
+                                        l_fwRequestMsg,
+                                        &l_fwResponseMsgSize,
+                                        l_fwResponseMsg);
+    } while (0);
+
+    // Release the firmware messages and set to NULL
+    delete []l_fwRequestMsg;
+    delete []l_fwResponseMsg;
+    l_fwRequestMsg = l_fwResponseMsg = nullptr;
+
+    TRACFCOMP(g_trac_runtime, EXIT_MRK"sendAttributes - exit with %s",
+              (nullptr == l_err ? "no error" : "error"));
+
+
+    return l_err;
+}
+
+
 #endif //__HOSTBOOT_RUNTIME_INTERFACE_VERSION_ONLY
 #endif // __RUNTIME__UTILITIES_H
diff --git a/src/include/runtime/interface.h b/src/include/runtime/interface.h
index cd6d0e23a..261e7e42f 100644
--- a/src/include/runtime/interface.h
+++ b/src/include/runtime/interface.h
@@ -541,19 +541,23 @@ typedef struct hostInterfaces
     /**
      *  @brief Structure to be sent and received in the
      *         firmware_request call
+     *
+     *  @note  see README.md file on how to create an HBRT to FW
+     *         request message interface
      */
-    enum
+
+    enum  // hbrt_fw_msg::io_type             the struct associated with io_type
     {
        HBRT_FW_MSG_TYPE_REQ_NOP = 0,
-       HBRT_FW_MSG_TYPE_RESP_NOP = 1,
-       HBRT_FW_MSG_TYPE_RESP_GENERIC = 2,
-       HBRT_FW_MSG_TYPE_REQ_HCODE_UPDATE = 3,
-       HBRT_FW_MSG_HBRT_FSP_REQ = 4,
-       HBRT_FW_MSG_TYPE_ERROR_LOG = 5,
-       HBRT_FW_MSG_HBRT_FSP_RESP = 6,
-       HBRT_FW_MSG_TYPE_I2C_LOCK = 7,
-       HBRT_FW_MSG_TYPE_SBE_STATE = 8,
-       HBRT_FW_MSG_TYPE_NVDIMM_PROTECTION = 9,
+       HBRT_FW_MSG_TYPE_RESP_NOP = 1,          // struct resp_generic
+       HBRT_FW_MSG_TYPE_RESP_GENERIC = 2,      // struct resp_generic
+       HBRT_FW_MSG_TYPE_REQ_HCODE_UPDATE = 3,  // struct req_hcode_update
+       HBRT_FW_MSG_HBRT_FSP_REQ = 4,           // struct GenericFspMboxMessage_t
+       HBRT_FW_MSG_TYPE_ERROR_LOG = 5,         // struct error_log
+       HBRT_FW_MSG_HBRT_FSP_RESP = 6,          // struct GenericFspMboxMessage_t
+       HBRT_FW_MSG_TYPE_I2C_LOCK = 7,          // struct req_i2c_lock
+       HBRT_FW_MSG_TYPE_SBE_STATE = 8,         // struct sbe_state
+       HBRT_FW_MSG_TYPE_NVDIMM_PROTECTION = 9, // struct nvdimm_protection_state
     };
 
     // NVDIMM protection state enum
@@ -593,7 +597,7 @@ typedef struct hostInterfaces
           } req_hcode_update;
 
           // This struct is sent from HBRT with
-          // io_type set to HBRT_FW_MSG_TYPE_ERR_LOG
+          // io_type set to HBRT_FW_MSG_TYPE_ERROR_LOG
           // Send an error log to FSP
           struct
           {