Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33066 )
Change subject: mb/google/{eve,glados}: Copy channel arrays separately ......................................................................
mb/google/{eve,glados}: Copy channel arrays separately
DqByteMapCh0 and DqByteMapCh1 are declared adjacently in the FSP_M_CONFIG struct, so it is tempting to begin memcpy at the address of the first array and overwrite both of them at once. However, FSP_M_CONFIG is not declared with the packed attribute, so this is not guaranteed to work and is undefined behaviour to boot. It is cleaner and less tricky to copy them independently. The same is true for DqsMapCpu2DramCh0 and DqsMapCpu2DramCh1, so we change those as well.
Change-Id: If394f14c4a39d6787ae31868241229646c26be7a Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1365730, 14013{38,39,40,42,43} Reviewed-on: https://review.coreboot.org/c/coreboot/+/33066 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/google/eve/romstage.c M src/mainboard/google/glados/variants/asuka/variant.c M src/mainboard/google/glados/variants/caroline/variant.c M src/mainboard/google/glados/variants/cave/variant.c M src/mainboard/google/glados/variants/chell/variant.c M src/mainboard/google/glados/variants/glados/variant.c M src/mainboard/google/glados/variants/lars/variant.c M src/mainboard/google/glados/variants/sentry/variant.c 8 files changed, 60 insertions(+), 30 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/eve/romstage.c b/src/mainboard/google/eve/romstage.c index 975eea5..7114715 100644 --- a/src/mainboard/google/eve/romstage.c +++ b/src/mainboard/google/eve/romstage.c @@ -40,8 +40,10 @@ /* Rcomp target */ const u16 rcomp_target[] = { 100, 40, 40, 23, 40 };
- memcpy(&mem_cfg->DqByteMapCh0, dq_map, sizeof(dq_map)); - memcpy(&mem_cfg->DqsMapCpu2DramCh0, dqs_map, sizeof(dqs_map)); + memcpy(&mem_cfg->DqByteMapCh0, dq_map[0], sizeof(dq_map[0])); + memcpy(&mem_cfg->DqByteMapCh1, dq_map[1], sizeof(dq_map[1])); + memcpy(&mem_cfg->DqsMapCpu2DramCh0, dqs_map[0], sizeof(dqs_map[0])); + memcpy(&mem_cfg->DqsMapCpu2DramCh1, dqs_map[1], sizeof(dqs_map[1])); memcpy(&mem_cfg->RcompResistor, rcomp_resistor, sizeof(rcomp_resistor)); memcpy(&mem_cfg->RcompTarget, rcomp_target, sizeof(rcomp_target));
diff --git a/src/mainboard/google/glados/variants/asuka/variant.c b/src/mainboard/google/glados/variants/asuka/variant.c index 4c77897..fdef81c 100644 --- a/src/mainboard/google/glados/variants/asuka/variant.c +++ b/src/mainboard/google/glados/variants/asuka/variant.c @@ -39,10 +39,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(memory_params->DqByteMapCh0, dq_map, - sizeof(memory_params->DqByteMapCh0) * 2); - memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, - sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->DqByteMapCh0, dq_map[0], + sizeof(memory_params->DqByteMapCh0)); + memcpy(memory_params->DqByteMapCh1, dq_map[1], + sizeof(memory_params->DqByteMapCh1)); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(memory_params->DqsMapCpu2DramCh0)); + memcpy(memory_params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(memory_params->DqsMapCpu2DramCh1)); memcpy(memory_params->RcompResistor, RcompResistor, sizeof(memory_params->RcompResistor)); memcpy(memory_params->RcompTarget, RcompTarget, diff --git a/src/mainboard/google/glados/variants/caroline/variant.c b/src/mainboard/google/glados/variants/caroline/variant.c index d61a538..ab6bd2c 100644 --- a/src/mainboard/google/glados/variants/caroline/variant.c +++ b/src/mainboard/google/glados/variants/caroline/variant.c @@ -41,10 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(memory_params->DqByteMapCh0, dq_map, - sizeof(memory_params->DqByteMapCh0) * 2); - memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, - sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->DqByteMapCh0, dq_map[0], + sizeof(memory_params->DqByteMapCh0)); + memcpy(memory_params->DqByteMapCh1, dq_map[1], + sizeof(memory_params->DqByteMapCh1)); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(memory_params->DqsMapCpu2DramCh0)); + memcpy(memory_params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(memory_params->DqsMapCpu2DramCh1)); memcpy(memory_params->RcompResistor, RcompResistor, sizeof(memory_params->RcompResistor)); memcpy(memory_params->RcompTarget, RcompTarget, diff --git a/src/mainboard/google/glados/variants/cave/variant.c b/src/mainboard/google/glados/variants/cave/variant.c index fc27fb4..d63a298 100644 --- a/src/mainboard/google/glados/variants/cave/variant.c +++ b/src/mainboard/google/glados/variants/cave/variant.c @@ -41,10 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(memory_params->DqByteMapCh0, dq_map, - sizeof(memory_params->DqByteMapCh0) * 2); - memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, - sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->DqByteMapCh0, dq_map[0], + sizeof(memory_params->DqByteMapCh0)); + memcpy(memory_params->DqByteMapCh1, dq_map[1], + sizeof(memory_params->DqByteMapCh1)); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(memory_params->DqsMapCpu2DramCh0)); + memcpy(memory_params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(memory_params->DqsMapCpu2DramCh1)); memcpy(memory_params->RcompResistor, RcompResistor, sizeof(memory_params->RcompResistor)); memcpy(memory_params->RcompTarget, RcompTarget, diff --git a/src/mainboard/google/glados/variants/chell/variant.c b/src/mainboard/google/glados/variants/chell/variant.c index 2d1b363..3e8503f 100644 --- a/src/mainboard/google/glados/variants/chell/variant.c +++ b/src/mainboard/google/glados/variants/chell/variant.c @@ -41,10 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(memory_params->DqByteMapCh0, dq_map, - sizeof(memory_params->DqByteMapCh0) * 2); - memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, - sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->DqByteMapCh0, dq_map[0], + sizeof(memory_params->DqByteMapCh0)); + memcpy(memory_params->DqByteMapCh1, dq_map[1], + sizeof(memory_params->DqByteMapCh1)); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(memory_params->DqsMapCpu2DramCh0)); + memcpy(memory_params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(memory_params->DqsMapCpu2DramCh1)); memcpy(memory_params->RcompResistor, RcompResistor, sizeof(memory_params->RcompResistor)); memcpy(memory_params->RcompTarget, RcompTarget, diff --git a/src/mainboard/google/glados/variants/glados/variant.c b/src/mainboard/google/glados/variants/glados/variant.c index fc27fb4..d63a298 100644 --- a/src/mainboard/google/glados/variants/glados/variant.c +++ b/src/mainboard/google/glados/variants/glados/variant.c @@ -41,10 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(memory_params->DqByteMapCh0, dq_map, - sizeof(memory_params->DqByteMapCh0) * 2); - memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, - sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->DqByteMapCh0, dq_map[0], + sizeof(memory_params->DqByteMapCh0)); + memcpy(memory_params->DqByteMapCh1, dq_map[1], + sizeof(memory_params->DqByteMapCh1)); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(memory_params->DqsMapCpu2DramCh0)); + memcpy(memory_params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(memory_params->DqsMapCpu2DramCh1)); memcpy(memory_params->RcompResistor, RcompResistor, sizeof(memory_params->RcompResistor)); memcpy(memory_params->RcompTarget, RcompTarget, diff --git a/src/mainboard/google/glados/variants/lars/variant.c b/src/mainboard/google/glados/variants/lars/variant.c index cff0096..37860c3 100644 --- a/src/mainboard/google/glados/variants/lars/variant.c +++ b/src/mainboard/google/glados/variants/lars/variant.c @@ -58,10 +58,14 @@ if (spd_index == K4E6E304EB_MEM_ID) targeted_rcomp = StrengthendRcompTarget;
- memcpy(params->DqByteMapCh0, dq_map, - sizeof(params->DqByteMapCh0) * 2); - memcpy(params->DqsMapCpu2DramCh0, dqs_map, - sizeof(params->DqsMapCpu2DramCh0) * 2); + memcpy(params->DqByteMapCh0, dq_map[0], + sizeof(params->DqByteMapCh0)); + memcpy(params->DqByteMapCh1, dq_map[1], + sizeof(params->DqByteMapCh1)); + memcpy(params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(params->DqsMapCpu2DramCh0)); + memcpy(params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(params->DqsMapCpu2DramCh1)); memcpy(params->RcompResistor, RcompResistor, sizeof(params->RcompResistor)); memcpy(params->RcompTarget, targeted_rcomp, diff --git a/src/mainboard/google/glados/variants/sentry/variant.c b/src/mainboard/google/glados/variants/sentry/variant.c index 4c7fa23..101be2a 100644 --- a/src/mainboard/google/glados/variants/sentry/variant.c +++ b/src/mainboard/google/glados/variants/sentry/variant.c @@ -51,10 +51,14 @@ if (spd_index == K4E6E304EE_MEM_ID) targeted_rcomp = StrengthendRcompTarget;
- memcpy(memory_params->DqByteMapCh0, dq_map, - sizeof(memory_params->DqByteMapCh0) * 2); - memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, - sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->DqByteMapCh0, dq_map[0], + sizeof(memory_params->DqByteMapCh0)); + memcpy(memory_params->DqByteMapCh1, dq_map[1], + sizeof(memory_params->DqByteMapCh1)); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map[0], + sizeof(memory_params->DqsMapCpu2DramCh0)); + memcpy(memory_params->DqsMapCpu2DramCh1, dqs_map[1], + sizeof(memory_params->DqsMapCpu2DramCh1)); memcpy(memory_params->RcompResistor, RcompResistor, sizeof(memory_params->RcompResistor)); memcpy(memory_params->RcompTarget, targeted_rcomp,