[coreboot-gerrit] Patch set updated for coreboot: amdmct: Fix offset calculation in mct_ResetDataStruct_D()

[Nico Huber (nico.h@gmx.de)]

Nico Huber (nico.h at gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17992

-gerrit

commit 13e577d4c98fa19dfe66800476b2e4d1bf7f6e84
Author: Martin Roth <martinroth at google.com>
Date:   Thu Dec 29 14:03:50 2016 -0700

    amdmct: Fix offset calculation in mct_ResetDataStruct_D()
    
    The code was intentionally using an offset past the end of the array to
    get the value of the next byte when starting to clear the structure.
    The calculated offset was too far off. So rewrite it using offsetof()
    and sizeof() which is less error-prone. Also make use of memset() and
    put a comment into the struct's declaration about the fragile code.
    
    A correct solution would require more extensive refactoring (e.g.
    moving the skipped fields out of the struct).
    
    Fixes warning for GCC 6.2 toolchain update:
    src/northbridge/amd/amdfam10/../amdmct/mct/mct_d.c:3628:27:
    In function 'mct_ResetDataStruct_D':
    error: index 2 denotes an offset greater than size of 'u8[2][4]
    {aka unsigned char[2][4]}' [-Werror=array-bounds]
    
    Change-Id: Ic81cf5e57992fc0e45f6c96b62a35742a8ef891f
    Signed-off-by: Martin Roth <martinroth at google.com>
    Signed-off-by: Nico Huber <nico.huber at secunet.com>
---
 src/northbridge/amd/amdmct/mct/mct_d.c | 25 ++++++++++---------------
 src/northbridge/amd/amdmct/mct/mct_d.h |  5 +++++
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/northbridge/amd/amdmct/mct/mct_d.c b/src/northbridge/amd/amdmct/mct/mct_d.c
index 62fc626..86cc2e0 100644
--- a/src/northbridge/amd/amdmct/mct/mct_d.c
+++ b/src/northbridge/amd/amdmct/mct/mct_d.c
@@ -3601,35 +3601,30 @@ static void mct_ResetDataStruct_D(struct MCTStatStruc *pMCTstat,
 					struct DCTStatStruc *pDCTstatA)
 {
 	u8 Node;
-	u32 i;
 	struct DCTStatStruc *pDCTstat;
-	u32 start, stop;
+	size_t start, stop;
 	u8 *p;
 	u16 host_serv1, host_serv2;
 
 	/* Initialize Data structures by clearing all entries to 0 */
-	p = (u8 *) pMCTstat;
-	for (i = 0; i < sizeof(struct MCTStatStruc); i++) {
-		p[i] = 0;
-	}
+	memset(pMCTstat, 0x00, sizeof(*pMCTstat));
 
 	for (Node = 0; Node < 8; Node++) {
 		pDCTstat = pDCTstatA + Node;
 		host_serv1 = pDCTstat->HostBiosSrvc1;
 		host_serv2 = pDCTstat->HostBiosSrvc2;
 
-		p = (u8 *) pDCTstat;
+		/* clear everything but CH_D_DIR_B_DQS .. CH_D_BC_RCVRDLY */
+		p = (u8 *)pDCTstat;
 		start = 0;
-		stop = (u32)(&((struct DCTStatStruc *)0)->CH_MaxRdLat[2]);
-		for (i = start; i < stop; i++) {
-			p[i] = 0;
-		}
+		stop = offsetof(struct DCTStatStruc, CH_D_DIR_B_DQS);
+		memset(p + start, 0x00, stop - start);
 
-		start = (u32)(&((struct DCTStatStruc *)0)->CH_D_BC_RCVRDLY[2][4]);
+		start = offsetof(struct DCTStatStruc, CH_D_BC_RCVRDLY)
+			+ sizeof(pDCTstat->CH_D_BC_RCVRDLY);
 		stop = sizeof(struct DCTStatStruc);
-		for (i = start; i < stop; i++) {
-			p[i] = 0;
-		}
+		memset(p + start, 0x00, stop - start);
+
 		pDCTstat->HostBiosSrvc1 = host_serv1;
 		pDCTstat->HostBiosSrvc2 = host_serv2;
 	}
diff --git a/src/northbridge/amd/amdmct/mct/mct_d.h b/src/northbridge/amd/amdmct/mct/mct_d.h
index 4e1a909..fbfbf6f 100644
--- a/src/northbridge/amd/amdmct/mct/mct_d.h
+++ b/src/northbridge/amd/amdmct/mct/mct_d.h
@@ -451,6 +451,9 @@ struct DCTStatStruc {		/* A per Node structure*/
 
 	u16 CH_MaxRdLat[2];	/* Max Read Latency (ns) for DCT 0*/
 		/* Max Read Latency (ns) for DCT 1*/
+
+	/* DON'T MOVE the following declarations. They form a
+	   region that's to be skipped in mct_ResetDataStruct_D(). */
 	u8 CH_D_DIR_B_DQS[2][4][2][9];	/* [A/B] [DIMM1-4] [R/W] [DQS] */
 		/* CHA DIMM0 Byte 0 - 7 and Check Write DQS Delay*/
 		/* CHA DIMM0 Byte 0 - 7 and Check Read DQS Delay*/
@@ -473,6 +476,8 @@ struct DCTStatStruc {		/* A per Node structure*/
 	u8 CH_D_BC_RCVRDLY[2][4];
 		/* CHA DIMM 0 - 4 Check Byte Receiver Enable Delay*/
 		/* CHB DIMM 0 - 4 Check Byte Receiver Enable Delay*/
+	/* END DON'T MOVE (see above) */
+
 	u8 DIMMValidDCT[2];	/* DIMM# in DCT0*/
 				/* DIMM# in DCT1*/
 	u8 MaxDCTs;		/* Max number of DCTs in system*/