Nico Huber (nico.h@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@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@google.com Signed-off-by: Nico Huber nico.huber@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*/