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

[Nico Huber]

Nico Huber ([e-mail]) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17992

-gerrit

commit b0774dbce285848f37cd87a0bb31be39b5fe5238
Author: Martin Roth 
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 
    Signed-off-by: Nico Huber 
---
 src/northbridge/amd/amdmct/mct/mct_d.c | 24 ++++++++++--------------
 src/northbridge/amd/amdmct/mct/mct_d.h |  5 +++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/src/northbridge/amd/amdmct/mct/mct_d.c b/src/northbridge/amd/amdmct/mct/mct_d.c
index 62fc626..2d4970e 100644
--- a/src/northbridge/amd/amdmct/mct/mct_d.c
+++ b/src/northbridge/amd/amdmct/mct/mct_d.c
@@ -3603,33 +3603,29 @@ static void mct_ResetDataStruct_D(struct MCTStatStruc *pMCTstat,
    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 = (u32)(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*/