PRD: fixed structure alignment in firdata code

Change-Id: Ie6e8b1d86640fdf5392c59e7d7b76b3f1edc87d5
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/43294
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Caleb N. Palmer <cnpalmer@us.ibm.com>
Reviewed-by: Benjamin J. Weisenbeck <bweisenb@us.ibm.com>
Reviewed-by: Zane C. Shelley <zshelle@us.ibm.com>
Squashed: I3af463929b2c284ffe9c62172e92adf9af102403
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/43358
Reviewed-by: Brian J. Stegmiller <bjs@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>

Author: zane131

src/usr/diag/prdf/occ_firdata/homerData_common.h

@@ -52,9 +52,21 @@
  *  - Register address lists - These lists vary in size depending on the number
  *      of register addresses needed in each list. The list counts are stored in
  *      HOMER_Data_t::regCounts. Order of the lists must match the array indexes
- *      HOMER_Data_t::regCounts, which are specified in TrgtType_t and
+ *      of HOMER_Data_t::regCounts, which are specified in TrgtType_t and
  *      RegType_t.
  *
+ *  - Chip specific address list - This is a list of HOMER_ChipSpecAddr_t
+ *      structs and will vary in size depending on the number of register
+ *      addresses that are specific to a certain chip or DD level. The number of
+ *      entries in this list is stored in HOMER_Data_t::ecDepCounts.
+ *
+ *  IMPORTANT NOTE: All of the structs used here are packed. Therefore, we must
+ *      ensure the variables within the struct are byte aligned. Meaning each
+ *      uint32_t within the struct must be 4-byte aligned and each uint16_t must
+ *      be 2-byte aligned. This also means the structs must always start on a
+ *      4-bye word boundary to maintain alignment. This is required due to the
+ *      limitations of the PPE42/SRAM hardware.
+ *
  *  Note that FIRs and indirect-SCOM FIRs characterize a set of registers to
  *  capture. In addition to capturing the FIR (or ID FIR), the OCC will need to
  *  capture the following addresses for each type:
@@ -79,6 +91,7 @@ typedef enum
 } HOMER_Version_t;
 
 /** PNOR information contained within the HOMER data. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t pnorOffset;     /** Physical offset of FIRDATA in PNOR */
@@ -90,25 +103,26 @@ typedef struct __attribute__((packed))
 
 /** HOMER data header information containing hardware configurations and
  *  register counts. */
+/* NOTE: This structure may, or may not, be 4-byte word aligned. It all depends
+ *       on the size of regCounts, which will change based on the number of
+ *       target types and register types we support. When reading/writing this
+ *       data ensure that proper padding has been added after this structure so
+ *       that subsequent structures are 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t header; /** Magic number to indicate valid data and version */
 
-    uint8_t chipCount; /** Number of configured chips per node */
+    uint32_t chipCount   :  8; /** Number of configured chips per node */
+    uint32_t ecDepCounts :  8; /** Number of regs that are EC dependent */
+    uint32_t iplState    :  1; /** See IplState_t. */
+    uint32_t reserved    : 15;
 
-    uint8_t iplState : 1; /** See IplState_t. */
-    uint8_t reserved : 7;
+    /** Information regarding the PNOR location and size. */
+    HOMER_PnorInfo_t pnorInfo;
 
     /** Contains number of registers per type for each target type. */
     uint8_t regCounts[TRGT_MAX][REG_MAX];
 
-    /** Number of regs that are dependent on EC level      **/
-    /** (these registers follow the normal register list)  **/
-    uint8_t ecDepCounts;
-
-    /** Information regarding the PNOR location and size. */
-    HOMER_PnorInfo_t pnorInfo;
-
 } HOMER_Data_t;
 
 /** @return An initialized HOMER_Data_t struct. */
@@ -133,20 +147,20 @@ typedef enum
 } HOMER_ChipType_t;
 
 /** Information for each configured chip. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t fsiBaseAddr;   /** FSI base address for the chip. */
 
-    uint16_t chipType :  4; /** Chip type (see HOMER_ChipType_t) */
-    uint16_t chipPos  :  6; /** Chip position relative to the node. */
-    uint16_t reserved :  6;
-
-    uint8_t  chipEcLevel;   /** EC level for this chip */
+    uint32_t chipType    :  4; /** Chip type (see HOMER_ChipType_t) */
+    uint32_t chipPos     :  6; /** Chip position relative to the node. */
+    uint32_t chipEcLevel :  8; /** EC level for this chip */
+    uint32_t reserved    : 14;
 
 } HOMER_Chip_t;
 
-
 /** Used for Registers that have EC level dependencies */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t chipType :  4; /** See HOMER_ChipType_t. */
@@ -160,7 +174,6 @@ typedef struct __attribute__((packed))
 
 } HOMER_ChipSpecAddr_t;
 
-
 /** @return An initialized HOMER_Chip_t struct. */
 static inline HOMER_Chip_t HOMER_getChip( HOMER_ChipType_t i_type )
 {
@@ -175,6 +188,7 @@ static inline HOMER_Chip_t HOMER_getChip( HOMER_ChipType_t i_type )
 /*----------------------------------------------------------------------------*/
 
 /** Information specific to a P9 Nimbus processor chip. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t isMaster   :  1; /** 1 if this is the master PROC, 0 otherwise */
@@ -188,10 +202,10 @@ typedef struct __attribute__((packed))
     uint32_t mcsMask    :  4; /** Mask of configured MCS units (0-3) */
     uint32_t mcaMask    :  8; /** Mask of configured MCA units (0-7) */
 
-    uint16_t cappMask   :  2; /** Mask of configured CAPP units (0-1) */
-    uint16_t pecMask    :  3; /** Mask of configured PEC units (0-2) */
-    uint16_t phbMask    :  6; /** Mask of configured PHB units (0-5) */
-    uint16_t reserved   :  5;
+    uint32_t cappMask   :  2; /** Mask of configured CAPP units (0-1) */
+    uint32_t pecMask    :  3; /** Mask of configured PEC units (0-2) */
+    uint32_t phbMask    :  6; /** Mask of configured PHB units (0-5) */
+    uint32_t reserved   : 21;
 
 } HOMER_ChipNimbus_t;
 
@@ -206,6 +220,7 @@ static inline HOMER_ChipNimbus_t HOMER_initChipNimbus()
 /*----------------------------------------------------------------------------*/
 
 /** Information specific to a P9 Cumulus processor chip. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t isMaster :  1; /** 1 if this is the master PROC, 0 otherwise */
@@ -219,10 +234,10 @@ typedef struct __attribute__((packed))
     uint32_t miMask   :  4; /** Mask of configured MI units (0-3) */
     uint32_t dmiMask  :  8; /** Mask of configured DMI units (0-7) */
 
-    uint16_t cappMask :  2; /** Mask of configured CAPP units (0-1) */
-    uint16_t pecMask  :  3; /** Mask of configured PEC units (0-2) */
-    uint16_t phbMask  :  6; /** Mask of configured PHB units (0-5) */
-    uint16_t reserved :  5;
+    uint32_t cappMask :  2; /** Mask of configured CAPP units (0-1) */
+    uint32_t pecMask  :  3; /** Mask of configured PEC units (0-2) */
+    uint32_t phbMask  :  6; /** Mask of configured PHB units (0-5) */
+    uint32_t reserved : 21;
 
 } HOMER_ChipCumulus_t;
 
@@ -237,10 +252,11 @@ static inline HOMER_ChipCumulus_t HOMER_initChipCumulus()
 /*----------------------------------------------------------------------------*/
 
 /** Information specific to a Centaur memory buffer chip. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
-    uint8_t mbaMask  : 2; /** Mask of configured MBA units (0-1) */
-    uint8_t reserved : 6;
+    uint32_t mbaMask  :  2; /** Mask of configured MBA units (0-1) */
+    uint32_t reserved : 30;
 
 } HOMER_ChipCentaur_t;
 
@@ -252,23 +268,4 @@ static inline HOMER_ChipCentaur_t HOMER_initChipCentaur()
     return c;
 }
 
-/** @brief Chip information inserted into HOMER data section after header
- *
- *         There is basically an array of these after the initial HOMER
- *         section (HOMER_Data_t).  The register info then follows.
- */
-typedef struct __attribute__((packed))
-{
-    HOMER_Chip_t  hChipType;  /* Nimbus, Centaur, EC Level, etc...*/
-
-    union
-    {
-        HOMER_ChipNimbus_t   hChipN;
-        HOMER_ChipCumulus_t  hChipC;
-        HOMER_ChipCentaur_t  hChipM;
-    };
-
-} HOMER_ChipInfo_t;
-
-
 #endif /* __homerData_common_h */

src/usr/diag/prdf/occ_firdata/pnorData_common.h

@@ -44,6 +44,13 @@
  *      - A list of all regular registers (PNOR_Reg_t).
  *      - A list of all indirect-SCOM registers (PNOR_IdReg_t).
  *
+ *  IMPORTANT NOTE: All of the structs used here are packed. Therefore, we must
+ *      ensure the variables within the struct are byte aligned. Meaning each
+ *      uint32_t within the struct must be 4-byte aligned and each uint16_t must
+ *      be 2-byte aligned. This also means the structs must always start on a
+ *      4-bye word boundary to maintain alignment. This is required due to the
+ *      limitations of the PPE42/SRAM hardware.
+ *
  *  The PNOR has limited data space. So the following rules will apply:
  *      - Any registers with the value of zero will not be captured.
  *      - Registers with SCOM errors will not be captured, however, the number
@@ -70,14 +77,15 @@ typedef enum
 } PNOR_Version_t;
 
 /** PNOR data header information. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t header; /** Magic number to indicate valid data and version */
 
-    uint16_t trgts    : 12; /** Number of targets with register data */
-    uint16_t full     :  1; /** 1 if PNOR data is full and data incomplete */
-    uint16_t iplState :  1; /** See enum IplState_t */
-    uint16_t reserved :  2;
+    uint32_t trgts    : 12; /** Number of targets with register data */
+    uint32_t full     :  1; /** 1 if PNOR data is full and data incomplete */
+    uint32_t iplState :  1; /** See enum IplState_t */
+    uint32_t reserved : 18;
 
 } PNOR_Data_t;
 
@@ -101,16 +109,17 @@ typedef enum
 } PNOR_Trgt_RegLimits_t;
 
 /** Information for each target with SCOM data. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
-    uint32_t chipPos  : 6; /** Parent chip position relative to the node */
-    uint32_t unitPos  : 5; /** Unit position relative to the parent chip */
-    uint32_t regs     : 9; /** Number of normal registers */
-    uint32_t idRegs   : 4; /** Number of indirect-SCOM registers */
-    uint32_t scomErrs : 8; /** Number of SCOM errors detected */
+    uint32_t chipPos  :  6; /** Parent chip position relative to the node */
+    uint32_t unitPos  :  5; /** Unit position relative to the parent chip */
+    uint32_t regs     :  9; /** Number of normal registers */
+    uint32_t idRegs   :  4; /** Number of indirect-SCOM registers */
+    uint32_t scomErrs :  8; /** Number of SCOM errors detected */
 
-    uint8_t trgtType  : 6; /** Target type. See enum TrgtType_t */
-    uint8_t reserved  : 2;
+    uint32_t trgtType :  6; /** Target type. See enum TrgtType_t */
+    uint32_t reserved : 26;
 
 } PNOR_Trgt_t;
 
@@ -132,6 +141,7 @@ static inline PNOR_Trgt_t PNOR_getTrgt( uint32_t i_trgtType, uint32_t i_chipPos,
 };
 
 /** Information for a normal register. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint32_t addr;  /** 32-bit address */
@@ -140,6 +150,7 @@ typedef struct __attribute__((packed))
 } PNOR_Reg_t;
 
 /** Information for an indirect-SCOM register. */
+/* NOTE: This structure is 4-byte word aligned. */
 typedef struct __attribute__((packed))
 {
     uint64_t addr;  /** 64-bit address */

src/usr/diag/prdf/occ_firdata/prdfWriteHomerFirData.C

@@ -695,6 +695,26 @@ errlHndl_t getPnorInfo( HOMER_Data_t & o_data )
 }
 
 //------------------------------------------------------------------------------
+
+/* Helper struct for chip information inserted into HOMER data section after the
+ * header. There is basically an array of these after the initial HOMER section
+ * (HOMER_Data_t).  The register info then follows.
+ */
+typedef struct __attribute__((packed))
+{
+    HOMER_Chip_t  hChipType;  /* Nimbus, Centaur, EC Level, etc...*/
+
+    union
+    {
+        HOMER_ChipNimbus_t   hChipN;
+        HOMER_ChipCumulus_t  hChipC;
+        HOMER_ChipCentaur_t  hChipM;
+    };
+
+} HOMER_ChipInfo_t;
+
+//------------------------------------------------------------------------------
+
 errlHndl_t getHwConfig( std::vector<HOMER_ChipInfo_t> &io_chipInfVector,
                         const HwInitialized_t i_curHw )
 {
@@ -720,7 +740,6 @@ errlHndl_t getHwConfig( std::vector<HOMER_ChipInfo_t> &io_chipInfVector,
         l_procModelType = (MODEL_CUMULUS == getChipModel(masterProc)) ?
                            HOMER_CHIP_CUMULUS : HOMER_CHIP_NIMBUS;
 
-
         // Iterate the list of functional PROCs.
         TargetHandleList procList = getFunctionalTargetList( TYPE_PROC );
         for ( TargetHandleList::iterator procIt = procList.begin();
@@ -965,6 +984,9 @@ errlHndl_t writeData( uint8_t * i_hBuf, size_t i_hBufSize,
     errlHndl_t errl = NULL;
     TrgtMap_t  l_targMap;
 
+    const size_t u32 = sizeof(uint32_t);
+    const size_t u64 = sizeof(uint64_t);
+
     do
     {
         size_t sz_hBuf = 0;
@@ -973,10 +995,12 @@ errlHndl_t writeData( uint8_t * i_hBuf, size_t i_hBufSize,
         size_t sz_data = sizeof(io_homerData);
         sz_hBuf += sz_data;
 
+        // The HOMER_Data_t struct may have an uneven size. Add some padding to
+        // ensure the subsequent HOMER_Chip_t structs are word aligned. */
+        const size_t padding = (u32 - (sizeof(HOMER_Data_t) % u32)) % u32;
+        sz_hBuf += padding;
 
         // Add register counts to the data.
-        const size_t u32 = sizeof(uint32_t);
-        const size_t u64 = sizeof(uint64_t);
         // initialize SCOM addresses for all targets & regs
         getAddresses(l_targMap);
 
@@ -1025,6 +1049,9 @@ errlHndl_t writeData( uint8_t * i_hBuf, size_t i_hBufSize,
         // Fill the input buffer with chipcount, regcount, pnor info
         memcpy( &i_hBuf[idx], &io_homerData,  sz_data   ); idx += sz_data;
 
+        // Add the padding at the end of the HOMER_Data_t struct.
+        idx += padding;
+
         // We want a list of chips next
         if (0 != io_homerData.chipCount)
         {

src/usr/diag/prdf/test/prdfTest_WriteHomerFir.H

@@ -51,6 +51,24 @@
 
 namespace PRDF
 {
+
+/* Helper struct for chip information inserted into HOMER data section after the
+ * header. There is basically an array of these after the initial HOMER section
+ * (HOMER_Data_t).  The register info then follows.
+ */
+typedef struct __attribute__((packed))
+{
+    HOMER_Chip_t  hChipType;  /* Nimbus, Centaur, EC Level, etc...*/
+
+    union
+    {
+        HOMER_ChipNimbus_t   hChipN;
+        HOMER_ChipCumulus_t  hChipC;
+        HOMER_ChipCentaur_t  hChipM;
+    };
+
+} HOMER_ChipInfo_t;
+
 // these functions from prdfWriteHomerFirData.C are needed for the
 // fakeWriteHomerFirData() function
 errlHndl_t getHwConfig( std::vector<HOMER_ChipInfo_t> &io_chipInfVector,
@@ -129,6 +147,9 @@ private:
     {
         using namespace PRDF;
 
+        const size_t sz_u32 = sizeof(uint32_t);
+        const size_t sz_hd  = sizeof(HOMER_Data_t);
+
         if (MASTER_PROC_CORE == i_curHw || ALL_PROC_MASTER_CORE == i_curHw)
         {
             // Should have at least one processor chip
@@ -138,10 +159,15 @@ private:
             } // end if no chips
             else
             {
-                uint8_t  *l_bytePtr = (reinterpret_cast<uint8_t *>(d)) +
-                                       sizeof(HOMER_Data_t);
+                uint8_t  *l_bytePtr = (reinterpret_cast<uint8_t *>(d)) + sz_hd;
                 HOMER_Chip_t  *l_chipPtr = NULL;
 
+              // The HOMER_Data_t struct may have an uneven size. Add some
+              // padding to ensure the subsequent HOMER_Chip_t structs are
+              // word aligned.
+              const size_t padding = (sz_u32 - (sz_hd % sz_u32)) % sz_u32;
+              l_bytePtr += padding;
+
                 // Have a chip or more to look thru
                 for ( uint32_t  l_chipNum=0;
                       (l_chipNum < d->chipCount); l_chipNum++ )