[coreboot-gerrit] Patch set updated for coreboot: and/nb/mct_ddr3: Pack all structures passed to ramstage and set alignment

Timothy Pearson (tpearson@raptorengineeringinc.com) gerrit at coreboot.org
Sun Apr 10 03:59:21 CEST 2016


Timothy Pearson (tpearson at raptorengineeringinc.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/14303

-gerrit

commit 4c72109f8133949c925c6aee091708a197672b79
Author: Timothy Pearson <tpearson at raptorengineeringinc.com>
Date:   Sat Apr 9 19:19:01 2016 -0500

    and/nb/mct_ddr3: Pack all structures passed to ramstage and set alignment
    
    Two of the MCT data structures passed as substructures to ramstage were
    not packed, and additionally no alignmnent was specified.  On at least
    SP5100-based platforms, specifying packed with no alignment caused boot
    failure dependent on the exact compiled binary layout (LPC hang).
    
    Specifying the alignment and packing the remaining structures appears to
    have resolved the remaining LPC hang issues on the KGPE-D16.  Note that
    packing the remaining structures alone was not sufficient to eliminate
    the hang, however removing the packed attribute entirely (during debugging)
    did resolve the hang at the expense of potential problems in ramstage.
    
    Change-Id: If3a7509ed438870d4d05caaaaa091e1c47bf9b97
    Signed-off-by: Timothy Pearson <tpearson at raptorengineeringinc.com>
---
 src/northbridge/amd/amdmct/mct_ddr3/mct_d.h  | 16 +++++++++-------
 src/northbridge/amd/amdmct/mct_ddr3/mwlc_d.h |  4 ++--
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/northbridge/amd/amdmct/mct_ddr3/mct_d.h b/src/northbridge/amd/amdmct/mct_ddr3/mct_d.h
index bb6c723..7f5de19 100644
--- a/src/northbridge/amd/amdmct/mct_ddr3/mct_d.h
+++ b/src/northbridge/amd/amdmct/mct_ddr3/mct_d.h
@@ -301,7 +301,7 @@ struct MCTStatStruc {
 	uint32_t TSCFreq;
 	uint16_t nvram_checksum;
 	uint8_t try_ecc;
-} __attribute__((packed));
+} __attribute__((packed, aligned(4)));
 
 /*=============================================================================
 	Global MCT Configuration Status Word (GStatus)
@@ -330,7 +330,7 @@ struct amd_spd_node_data {
 	uint64_t nvram_spd_hash[MAX_DIMMS_SUPPORTED];	/* [DIMM] */
 	uint8_t nvram_spd_match;
 	uint8_t nvram_memclk[2];			/* [channel] */
-} __attribute__((packed));
+} __attribute__((packed, aligned(4)));
 
 struct DCTStatStruc {		/* A per Node structure*/
 /* DCTStatStruct_F -  start */
@@ -636,7 +636,9 @@ struct DCTStatStruc {		/* A per Node structure*/
 	uint32_t DimmSerialNumber[MAX_DIMMS_SUPPORTED];
 
 	struct amd_spd_node_data spd_data;
-} __attribute__((packed));
+
+	/* Work around packed strucutre compiler bug */
+} __attribute__((packed, aligned(4)));
 
 struct amd_s3_persistent_mct_channel_data {
 	/* Stage 1 (1 dword) */
@@ -797,19 +799,19 @@ struct amd_s3_persistent_mct_channel_data {
 	uint32_t f2x9cx0d0fe00a;
 
 	/* TOTAL: 343 dwords */
-} __attribute__((packed));
+} __attribute__((packed, aligned(4)));
 
 struct amd_s3_persistent_node_data {
 	uint32_t node_present;
 	uint64_t spd_hash[MAX_DIMMS_SUPPORTED];
 	uint8_t memclk[2];
 	struct amd_s3_persistent_mct_channel_data channel[2];
-} __attribute__((packed));
+} __attribute__((packed, aligned(4)));
 
 struct amd_s3_persistent_data {
 	struct amd_s3_persistent_node_data node[MAX_NODES_SUPPORTED];
 	uint16_t nvram_checksum;
-} __attribute__((packed));
+} __attribute__((packed, aligned(4)));
 
 /*===============================================================================
 	Local Error Status Codes (DCTStatStruc.ErrCode)
@@ -976,7 +978,7 @@ struct amdmct_memory_info {
 	struct DCTStatStruc dct_stat[MAX_NODES_SUPPORTED];
 	uint16_t ecc_enabled;
 	uint16_t ecc_scrub_rate;
-} __attribute__((packed));
+} __attribute__((packed, aligned(4)));
 
 u32 Get_NB32(u32 dev, u32 reg);
 void Set_NB32(u32 dev, u32 reg, u32 val);
diff --git a/src/northbridge/amd/amdmct/mct_ddr3/mwlc_d.h b/src/northbridge/amd/amdmct/mct_ddr3/mwlc_d.h
index 2bd39f9..28359a1 100644
--- a/src/northbridge/amd/amdmct/mct_ddr3/mwlc_d.h
+++ b/src/northbridge/amd/amdmct/mct_ddr3/mwlc_d.h
@@ -106,7 +106,7 @@
 typedef struct _sMCTStruct
 {
 	void (*AgesaDelay)(u32 delayval);	/* IBV defined Delay Function */
-} sMCTStruct;
+} __attribute__((packed, aligned(4))) sMCTStruct;
 
 /* DCT 0 and DCT 1 Data structure */
 typedef struct _sDCTStruct
@@ -161,6 +161,6 @@ typedef struct _sDCTStruct
 	u8 DimmRanks[MAX_TOTAL_DIMMS];	/* Total Number of Ranks(per Dimm) */
 	uint64_t LogicalCPUID;
 	u8 WLPass;
-} sDCTStruct;
+} __attribute__((packed, aligned(4))) sDCTStruct;
 
 #endif



More information about the coreboot-gerrit mailing list