EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. Thsi can avoid system hang up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/1
diff --git a/src/soc/intel/tigerlake/meminit.c b/src/soc/intel/tigerlake/meminit.c index 8664547..1210bcd 100644 --- a/src/soc/intel/tigerlake/meminit.c +++ b/src/soc/intel/tigerlake/meminit.c @@ -308,7 +308,7 @@ get_spd_smbus(blk);
for (i = 0; i < ARRAY_SIZE(blk->addr_map); i++) { - if (blk->addr_map[i]) + if (blk->addr_map[i] && blk->spd_array[i] != NULL) print_spd_info((unsigned char *)blk->spd_array[i]); } }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/1/src/soc/intel/tigerlake/mem... File src/soc/intel/tigerlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/40560/1/src/soc/intel/tigerlake/mem... PS1, Line 311: blk->addr_map[i] && blk->spd_array[i] != NULL Can you please add a comment here that the SPD gets printed only if: a) mainboard provides a non-zero SMBus address and b) SPD is successfully read using the SMBus address
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/1/src/soc/intel/tigerlake/mem... File src/soc/intel/tigerlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/40560/1/src/soc/intel/tigerlake/mem... PS1, Line 311: blk->addr_map[i] && blk->spd_array[i] != NULL
Can you please add a comment here that the SPD gets printed only if: […]
Of course!!
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Varun Joshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40560
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. Thsi can avoid system hang up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/2
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Varun Joshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40560
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. Thsi can avoid system hang up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/1/src/soc/intel/tigerlake/mem... File src/soc/intel/tigerlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/40560/1/src/soc/intel/tigerlake/mem... PS1, Line 311: blk->addr_map[i] && blk->spd_array[i] != NULL
Of course!!
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40560/3//COMMIT_MSG@9 PS3, Line 9: Thsi "This can prevent the system from hanging up" ?
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Varun Joshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40560
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. This can prevent the system from hanging up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/4
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40560/3//COMMIT_MSG@9 PS3, Line 9: Thsi
"This can prevent the system from hanging up" ?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/4/src/soc/intel/tigerlake/mem... File src/soc/intel/tigerlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/40560/4/src/soc/intel/tigerlake/mem... PS4, Line 316: blk->addr_map[i] && This check is redundant now because get_spd_smbus() returns NULL if addr_map[i] is 0.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40560/4/src/soc/intel/tigerlake/mem... File src/soc/intel/tigerlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/40560/4/src/soc/intel/tigerlake/mem... PS4, Line 316: blk->addr_map[i] &&
This check is redundant now because get_spd_smbus() returns NULL if addr_map[i] is 0.
agree :)
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Varun Joshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40560
to look at the new patch set (#5).
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. This can prevent the system from hanging up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/5
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 5: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Varun Joshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40560
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. This can prevent the system from hanging up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/7
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Varun Joshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40560
to look at the new patch set (#8).
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. This can prevent the system from hanging up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/40560/8
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 8: Code-Review+2
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
soc/intel/tigerlake: Check SPD is not NULL before print
Check SPD is not NULL before print. This can prevent the system from hanging up.
BUG=b:154445630 TEST=Check NULL SPD is not print.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iccd9fce99eda7ae2b8fb1b4f3c2e635c2a428f04 Reviewed-on: https://review.coreboot.org/c/coreboot/+/40560 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/meminit.c 1 file changed, 6 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/meminit.c b/src/soc/intel/tigerlake/meminit.c index ebadcff..231e261 100644 --- a/src/soc/intel/tigerlake/meminit.c +++ b/src/soc/intel/tigerlake/meminit.c @@ -315,8 +315,13 @@
get_spd_smbus(blk);
+ /* + * SPD gets printed only if: + * a) mainboard provides a non-zero SMBus address and + * b) SPD is successfully read using the SMBus address + */ for (i = 0; i < ARRAY_SIZE(blk->addr_map); i++) { - if (blk->addr_map[i]) + if (blk->spd_array[i] != NULL) print_spd_info((uint8_t *)blk->spd_array[i]); } }
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40560 )
Change subject: soc/intel/tigerlake: Check SPD is not NULL before print ......................................................................
Patch Set 9:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2902 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2901 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2900 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/2899
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
EricR Lai (Code Review)
-
Furquan Shaikh (Code Review)
-
Tim Wawrzynczak (Code Review)