Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33135 )
Change subject: mb/{asrock,intel,purism}: Copy channel arrays separately ......................................................................
mb/{asrock,intel,purism}: 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: Ic6bb2bd5773af24329575926dbc70e0211f29051 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 136538{8,9}, 140134{1,4} Reviewed-on: https://review.coreboot.org/c/coreboot/+/33135 Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/asrock/h110m/romstage.c M src/mainboard/intel/kblrvp/romstage.c M src/mainboard/intel/kblrvp/spd/spd.h M src/mainboard/intel/kblrvp/spd/spd_util.c M src/mainboard/intel/kunimitsu/romstage.c M src/mainboard/intel/kunimitsu/romstage_fsp20.c M src/mainboard/intel/kunimitsu/spd/spd.h M src/mainboard/intel/kunimitsu/spd/spd_util.c M src/mainboard/intel/saddlebrook/romstage.c M src/mainboard/intel/saddlebrook/spd/spd.h M src/mainboard/intel/saddlebrook/spd/spd_util.c M src/mainboard/purism/librem_skl/romstage.c 12 files changed, 60 insertions(+), 38 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/asrock/h110m/romstage.c b/src/mainboard/asrock/h110m/romstage.c index 4961a79..efbc650 100644 --- a/src/mainboard/asrock/h110m/romstage.c +++ b/src/mainboard/asrock/h110m/romstage.c @@ -20,7 +20,7 @@
#define RCOMP_TARGET_PARAMS 0x5
-static void mainboard_fill_dq_map_data(void *dq_map_ptr) +static void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -28,16 +28,18 @@ 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00 }, { 0x33, 0xCC, 0x00, 0xCC, 0x33, 0xCC, 0x33, 0x00, 0xFF, 0x00, 0xFF, 0x00 } }; - memcpy(dq_map_ptr, dq_map, sizeof(dq_map)); + memcpy(dq_map_ch0, dq_map[0], sizeof(dq_map[0])); + memcpy(dq_map_ch1, dq_map[1], sizeof(dq_map[1])); }
-static void mainboard_fill_dqs_map_data(void *dqs_map_ptr) +static void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1) { /* DQS CPU<>DRAM map */ const u8 dqs_map[2][8] = { { 0, 1, 3, 2, 4, 5, 6, 7 }, { 1, 0, 4, 5, 2, 3, 6, 7 } }; - memcpy(dqs_map_ptr, dqs_map, sizeof(dqs_map)); + memcpy(dqs_map_ch0, dqs_map[0], sizeof(dqs_map[0])); + memcpy(dqs_map_ch1, dqs_map[1], sizeof(dqs_map[1])); }
static void mainboard_fill_rcomp_res_data(void *rcomp_ptr) @@ -64,8 +66,10 @@ };
mem_cfg = &mupd->FspmConfig; - mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0); - mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0); + mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0, + &mem_cfg->DqByteMapCh1); + mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0, + &mem_cfg->DqsMapCpu2DramCh1); mainboard_fill_rcomp_res_data(&mem_cfg->RcompResistor); mainboard_fill_rcomp_strength_data(&mem_cfg->RcompTarget);
diff --git a/src/mainboard/intel/kblrvp/romstage.c b/src/mainboard/intel/kblrvp/romstage.c index 0385e29..1ae7d6e 100644 --- a/src/mainboard/intel/kblrvp/romstage.c +++ b/src/mainboard/intel/kblrvp/romstage.c @@ -35,8 +35,10 @@
printk(BIOS_INFO, "SPD index %d\n", spd_index);
- mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0); - mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0); + mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0, + &mem_cfg->DqByteMapCh1); + mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0, + &mem_cfg->DqsMapCpu2DramCh1); mainboard_fill_rcomp_res_data(&mem_cfg->RcompResistor); mainboard_fill_rcomp_strength_data(&mem_cfg->RcompTarget);
diff --git a/src/mainboard/intel/kblrvp/spd/spd.h b/src/mainboard/intel/kblrvp/spd/spd.h index c24baa0..316ff5e 100644 --- a/src/mainboard/intel/kblrvp/spd/spd.h +++ b/src/mainboard/intel/kblrvp/spd/spd.h @@ -22,8 +22,8 @@
#define RCOMP_TARGET_PARAMS 0x5
-void mainboard_fill_dq_map_data(void *dq_map_ptr); -void mainboard_fill_dqs_map_data(void *dqs_map_ptr); +void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1); +void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1); void mainboard_fill_rcomp_res_data(void *rcomp_ptr); void mainboard_fill_rcomp_strength_data(void *rcomp_strength_ptr); #endif diff --git a/src/mainboard/intel/kblrvp/spd/spd_util.c b/src/mainboard/intel/kblrvp/spd/spd_util.c index 9318c39..4795435 100644 --- a/src/mainboard/intel/kblrvp/spd/spd_util.c +++ b/src/mainboard/intel/kblrvp/spd/spd_util.c @@ -19,7 +19,7 @@ #include "../board_id.h" #include "spd.h"
-void mainboard_fill_dq_map_data(void *dq_map_ptr) +void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -27,16 +27,18 @@ 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00 }, { 0x33, 0xCC, 0x00, 0xCC, 0x33, 0xCC, 0x33, 0x00, 0xFF, 0x00, 0xFF, 0x00 } }; - memcpy(dq_map_ptr, dq_map, sizeof(dq_map)); + memcpy(dq_map_ch0, dq_map[0], sizeof(dq_map[0])); + memcpy(dq_map_ch1, dq_map[1], sizeof(dq_map[1])); }
-void mainboard_fill_dqs_map_data(void *dqs_map_ptr) +void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1) { /* DQS CPU<>DRAM map */ const u8 dqs_map[2][8] = { { 0, 1, 3, 2, 4, 5, 6, 7 }, { 1, 0, 4, 5, 2, 3, 6, 7 } }; - memcpy(dqs_map_ptr, dqs_map, sizeof(dqs_map)); + memcpy(dqs_map_ch0, dqs_map[0], sizeof(dqs_map[0])); + memcpy(dqs_map_ch1, dqs_map[1], sizeof(dqs_map[1])); }
void mainboard_fill_rcomp_res_data(void *rcomp_ptr) diff --git a/src/mainboard/intel/kunimitsu/romstage.c b/src/mainboard/intel/kunimitsu/romstage.c index f900ca3..0312ad1 100644 --- a/src/mainboard/intel/kunimitsu/romstage.c +++ b/src/mainboard/intel/kunimitsu/romstage.c @@ -24,8 +24,10 @@ MEMORY_INIT_UPD *memory_params) { spd_memory_init_params(memory_params); - mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0); - mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0); + mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0, + &memory_params->DqByteMapCh1); + mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0, + &memory_params->DqsMapCpu2DramCh1); mainboard_fill_rcomp_res_data(&memory_params->RcompResistor); mainboard_fill_rcomp_strength_data(&memory_params->RcompTarget); memory_params->MemorySpdDataLen = SPD_LEN; diff --git a/src/mainboard/intel/kunimitsu/romstage_fsp20.c b/src/mainboard/intel/kunimitsu/romstage_fsp20.c index 5364693..2a4474e 100644 --- a/src/mainboard/intel/kunimitsu/romstage_fsp20.c +++ b/src/mainboard/intel/kunimitsu/romstage_fsp20.c @@ -25,8 +25,10 @@ FSP_M_CONFIG *mem_cfg; mem_cfg = &mupd->FspmConfig;
- mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0); - mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0); + mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0, + &mem_cfg->DqByteMapCh1); + mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0, + &mem_cfg->DqsMapCpu2DramCh1); mainboard_fill_rcomp_res_data(&mem_cfg->RcompResistor); mainboard_fill_rcomp_strength_data(&mem_cfg->RcompTarget);
diff --git a/src/mainboard/intel/kunimitsu/spd/spd.h b/src/mainboard/intel/kunimitsu/spd/spd.h index 22d371f..8bc7336 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd.h +++ b/src/mainboard/intel/kunimitsu/spd/spd.h @@ -55,8 +55,8 @@ return (gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios))); } void spd_memory_init_params(MEMORY_INIT_UPD *memory_params); -void mainboard_fill_dq_map_data(void *dq_map_ptr); -void mainboard_fill_dqs_map_data(void *dqs_map_ptr); +void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1); +void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1); void mainboard_fill_rcomp_res_data(void *rcomp_ptr); void mainboard_fill_rcomp_strength_data(void *rcomp_strength_ptr); uintptr_t mainboard_get_spd_data(void); diff --git a/src/mainboard/intel/kunimitsu/spd/spd_util.c b/src/mainboard/intel/kunimitsu/spd/spd_util.c index 288ee1e..05a0a86 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd_util.c +++ b/src/mainboard/intel/kunimitsu/spd/spd_util.c @@ -19,7 +19,7 @@ #include "boardid.h" #include "spd.h"
-void mainboard_fill_dq_map_data(void *dq_map_ptr) +void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -27,16 +27,18 @@ 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00 }, { 0x0F, 0xF0, 0x00, 0xF0, 0x0F, 0xF0, 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00 } }; - memcpy(dq_map_ptr, dq_map, sizeof(dq_map)); + memcpy(dq_map_ch0, dq_map[0], sizeof(dq_map[0])); + memcpy(dq_map_ch1, dq_map[1], sizeof(dq_map[1])); }
-void mainboard_fill_dqs_map_data(void *dqs_map_ptr) +void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1) { /* DQS CPU<>DRAM map */ const u8 dqs_map[2][8] = { { 0, 1, 3, 2, 6, 5, 4, 7 }, { 2, 3, 0, 1, 6, 7, 4, 5 } }; - memcpy(dqs_map_ptr, dqs_map, sizeof(dqs_map)); + memcpy(dqs_map_ch0, dqs_map[0], sizeof(dqs_map[0])); + memcpy(dqs_map_ch1, dqs_map[1], sizeof(dqs_map[1])); }
void mainboard_fill_rcomp_res_data(void *rcomp_ptr) diff --git a/src/mainboard/intel/saddlebrook/romstage.c b/src/mainboard/intel/saddlebrook/romstage.c index 45f39d4..46c2cdd 100644 --- a/src/mainboard/intel/saddlebrook/romstage.c +++ b/src/mainboard/intel/saddlebrook/romstage.c @@ -49,8 +49,10 @@ * should be set in the FSP flash image and should not need to be * changed. */ - mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0); - mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0); + mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0, + &memory_params->DqByteMapCh1); + mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0, + &memory_params->DqsMapCpu2DramCh1); mainboard_fill_rcomp_res_data(&memory_params->RcompResistor); mainboard_fill_rcomp_strength_data(&memory_params->RcompTarget);
diff --git a/src/mainboard/intel/saddlebrook/spd/spd.h b/src/mainboard/intel/saddlebrook/spd/spd.h index a5f1af3..70a1f68 100644 --- a/src/mainboard/intel/saddlebrook/spd/spd.h +++ b/src/mainboard/intel/saddlebrook/spd/spd.h @@ -19,8 +19,8 @@
#define RCOMP_TARGET_PARAMS 0x5
-void mainboard_fill_dq_map_data(void *dq_map_ptr); -void mainboard_fill_dqs_map_data(void *dqs_map_ptr); +void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1); +void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1); void mainboard_fill_rcomp_res_data(void *rcomp_ptr); void mainboard_fill_rcomp_strength_data(void *rcomp_strength_ptr);
diff --git a/src/mainboard/intel/saddlebrook/spd/spd_util.c b/src/mainboard/intel/saddlebrook/spd/spd_util.c index 5055d9a..a09cebc 100644 --- a/src/mainboard/intel/saddlebrook/spd/spd_util.c +++ b/src/mainboard/intel/saddlebrook/spd/spd_util.c @@ -17,7 +17,7 @@ #include <string.h> #include "spd.h"
-void mainboard_fill_dq_map_data(void *dq_map_ptr) +void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -25,16 +25,18 @@ 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00 }, { 0x33, 0xCC, 0x00, 0xCC, 0x33, 0xCC, 0x33, 0x00, 0xFF, 0x00, 0xFF, 0x00 } }; - memcpy(dq_map_ptr, dq_map, sizeof(dq_map)); + memcpy(dq_map_ch0, dq_map[0], sizeof(dq_map[0])); + memcpy(dq_map_ch1, dq_map[1], sizeof(dq_map[1])); }
-void mainboard_fill_dqs_map_data(void *dqs_map_ptr) +void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1) { /* DQS CPU<>DRAM map */ const u8 dqs_map[2][8] = { { 0, 1, 3, 2, 4, 5, 6, 7 }, { 1, 0, 4, 5, 2, 3, 6, 7 } }; - memcpy(dqs_map_ptr, dqs_map, sizeof(dqs_map)); + memcpy(dqs_map_ch0, dqs_map[0], sizeof(dqs_map[0])); + memcpy(dqs_map_ch1, dqs_map[1], sizeof(dqs_map[1])); }
void mainboard_fill_rcomp_res_data(void *rcomp_ptr) diff --git a/src/mainboard/purism/librem_skl/romstage.c b/src/mainboard/purism/librem_skl/romstage.c index faf4090..4200373 100644 --- a/src/mainboard/purism/librem_skl/romstage.c +++ b/src/mainboard/purism/librem_skl/romstage.c @@ -21,7 +21,7 @@ #include <spd_bin.h> #include <string.h>
-static void mainboard_fill_dq_map_data(void *dq_map_ptr) +static void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -29,16 +29,18 @@ 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00 }, { 0x33, 0xCC, 0x00, 0xCC, 0x33, 0xCC, 0x33, 0x00, 0xFF, 0x00, 0xFF, 0x00 } }; - memcpy(dq_map_ptr, dq_map, sizeof(dq_map)); + memcpy(dq_map_ch0, dq_map[0], sizeof(dq_map[0])); + memcpy(dq_map_ch1, dq_map[1], sizeof(dq_map[1])); }
-static void mainboard_fill_dqs_map_data(void *dqs_map_ptr) +static void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1) { /* DQS CPU<>DRAM map */ const u8 dqs_map[2][8] = { { 0, 1, 3, 2, 4, 5, 6, 7 }, { 1, 0, 4, 5, 2, 3, 6, 7 } }; - memcpy(dqs_map_ptr, dqs_map, sizeof(dqs_map)); + memcpy(dqs_map_ch0, dqs_map[0], sizeof(dqs_map[0])); + memcpy(dqs_map_ch1, dqs_map[1], sizeof(dqs_map[1])); }
static void mainboard_fill_rcomp_res_data(void *rcomp_ptr) @@ -68,8 +70,10 @@ dump_spd_info(&blk); assert(blk.spd_array[0][0] != 0);
- mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0); - mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0); + mainboard_fill_dq_map_data(&mem_cfg->DqByteMapCh0, + &mem_cfg->DqByteMapCh1); + mainboard_fill_dqs_map_data(&mem_cfg->DqsMapCpu2DramCh0, + &mem_cfg->DqsMapCpu2DramCh1); mainboard_fill_rcomp_res_data(&mem_cfg->RcompResistor); mainboard_fill_rcomp_strength_data(&mem_cfg->RcompTarget);