awokd@danwin1210.me has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38176 )
Change subject: vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings ......................................................................
vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings
The memNTrainFlowControl array is generating Coverity warnings in multiple places in code where it attempts to write to index 1. The array is defined as either 2 elements or 1 of NULL depending on #if (AGESA_ENTRY_INIT_POST == TRUE). This is likely a false alarm from Coverity (memory should not be training outside of a POST), but adding a second NULL element for the AGESA_ENTRY_INIT_POST == FALSE case.
Change-Id: Iaebe0830471e1854d6191c69cdaa552f900ba7a6 Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1357451, 1357452, 1357453 --- M src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h M src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h M src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h 3 files changed, 6 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/38176/1
diff --git a/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h b/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h index 57dc0c8..231b263 100644 --- a/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h +++ b/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h @@ -680,7 +680,8 @@ *--------------------------------------------------------------------------------------------------- */ OPTION_MEM_FEATURE_NB* memNTrainFlowControl[] = { // Training flow control - NULL + NULL, + NULL, }; /*--------------------------------------------------------------------------------------------------- * DEFAULT TECHNOLOGY BLOCK diff --git a/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h b/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h index 457c51e..f662db2 100644 --- a/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h +++ b/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h @@ -4662,7 +4662,8 @@ *--------------------------------------------------------------------------------------------------- */ OPTION_MEM_FEATURE_NB* memNTrainFlowControl[] = { // Training flow control - NULL + NULL, + NULL, }; /*--------------------------------------------------------------------------------------------------- * DEFAULT TECHNOLOGY BLOCK diff --git a/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h b/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h index e2d4e03..c5484f1 100644 --- a/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h +++ b/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h @@ -1531,7 +1531,8 @@ *--------------------------------------------------------------------------------------------------- */ OPTION_MEM_FEATURE_NB* memNTrainFlowControl[] = { // Training flow control - NULL + NULL, + NULL, }; /*--------------------------------------------------------------------------------------------------- * DEFAULT TECHNOLOGY BLOCK
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38176
to look at the new patch set (#2).
Change subject: vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings ......................................................................
vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings
The memNTrainFlowControl array is generating Coverity warnings in multiple places in code where it attempts to write to index 1. The array is defined as either 2 elements or 1 of NULL depending on #if (AGESA_ENTRY_INIT_POST == TRUE). This is likely a false alarm from Coverity (memory should not be training outside of a POST), but adding a second NULL element for the AGESA_ENTRY_INIT_POST == FALSE case. Tested on Lenovo G505s.
Change-Id: Iaebe0830471e1854d6191c69cdaa552f900ba7a6 Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1357451, 1357452, 1357453 --- M src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h M src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h M src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h 3 files changed, 6 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/38176/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38176 )
Change subject: vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38176 )
Change subject: vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings ......................................................................
vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings
The memNTrainFlowControl array is generating Coverity warnings in multiple places in code where it attempts to write to index 1. The array is defined as either 2 elements or 1 of NULL depending on #if (AGESA_ENTRY_INIT_POST == TRUE). This is likely a false alarm from Coverity (memory should not be training outside of a POST), but adding a second NULL element for the AGESA_ENTRY_INIT_POST == FALSE case. Tested on Lenovo G505s.
Change-Id: Iaebe0830471e1854d6191c69cdaa552f900ba7a6 Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1357451, 1357452, 1357453 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38176 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h M src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h M src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h 3 files changed, 6 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h b/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h index 57dc0c8..231b263 100644 --- a/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h +++ b/src/vendorcode/amd/agesa/f14/Config/OptionMemoryInstall.h @@ -680,7 +680,8 @@ *--------------------------------------------------------------------------------------------------- */ OPTION_MEM_FEATURE_NB* memNTrainFlowControl[] = { // Training flow control - NULL + NULL, + NULL, }; /*--------------------------------------------------------------------------------------------------- * DEFAULT TECHNOLOGY BLOCK diff --git a/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h b/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h index 457c51e..f662db2 100644 --- a/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h +++ b/src/vendorcode/amd/agesa/f15tn/Config/OptionMemoryInstall.h @@ -4662,7 +4662,8 @@ *--------------------------------------------------------------------------------------------------- */ OPTION_MEM_FEATURE_NB* memNTrainFlowControl[] = { // Training flow control - NULL + NULL, + NULL, }; /*--------------------------------------------------------------------------------------------------- * DEFAULT TECHNOLOGY BLOCK diff --git a/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h b/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h index e2d4e03..c5484f1 100644 --- a/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h +++ b/src/vendorcode/amd/agesa/f16kb/Config/OptionMemoryInstall.h @@ -1531,7 +1531,8 @@ *--------------------------------------------------------------------------------------------------- */ OPTION_MEM_FEATURE_NB* memNTrainFlowControl[] = { // Training flow control - NULL + NULL, + NULL, }; /*--------------------------------------------------------------------------------------------------- * DEFAULT TECHNOLOGY BLOCK
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38176 )
Change subject: vc/amd/agesa/[...]/Config: Avoid out-of-bounds warnings ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1111 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1110 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1109
Please note: This test is under development and might not be accurate at all!