Hello ashk@codeaurora.org,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to review the following change.
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c M src/soc/qualcomm/sc7180/Makefile.inc 2 files changed, 15 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/1
diff --git a/src/soc/qualcomm/common/qclib.c b/src/soc/qualcomm/common/qclib.c index ac80a76..7f3dfa8 100644 --- a/src/soc/qualcomm/common/qclib.c +++ b/src/soc/qualcomm/common/qclib.c @@ -26,6 +26,7 @@ #include <soc/mmu_common.h> #include <soc/qclib_common.h> #include <soc/symbols_common.h> +#include <security/vboot/gbb.h>
struct qclib_cb_if_table qclib_cb_if_table = { .magic = QCLIB_MAGIC_NUMBER, @@ -158,19 +159,22 @@ QCLIB_GA_ENABLE_UART_LOGGING;
if (CONFIG(QC_SDI_ENABLE)) { - struct prog qcsdi = - PROG_INIT(PROG_REFCODE, CONFIG_CBFS_PREFIX "/qcsdi"); + if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { + struct prog qcsdi = + PROG_INIT(PROG_REFCODE, + CONFIG_CBFS_PREFIX "/qcsdi");
- /* Attempt to load QCSDI elf */ - if (prog_locate(&qcsdi)) - goto fail; + /* Attempt to load QCSDI elf */ + if (prog_locate(&qcsdi)) + goto fail;
- if (cbfs_prog_stage_load(&qcsdi)) - goto fail; + if (cbfs_prog_stage_load(&qcsdi)) + goto fail;
- qclib_add_if_table_entry(QCLIB_TE_QCSDI, prog_entry(&qcsdi), - prog_size(&qcsdi), 0); - printk(BIOS_INFO, "qcsdi.entry[%p]\n", qcsdi.entry); + qclib_add_if_table_entry(QCLIB_TE_QCSDI, + prog_entry(&qcsdi), prog_size(&qcsdi), 0); + printk(BIOS_INFO, "qcsdi.entry[%p]\n", qcsdi.entry); + } }
dump_te_table(); diff --git a/src/soc/qualcomm/sc7180/Makefile.inc b/src/soc/qualcomm/sc7180/Makefile.inc index 66cb35c..695ab88 100644 --- a/src/soc/qualcomm/sc7180/Makefile.inc +++ b/src/soc/qualcomm/sc7180/Makefile.inc @@ -40,6 +40,7 @@ romstage-y += qcom_qup_se.c romstage-$(CONFIG_DRIVERS_UART) += uart.c romstage-$(CONFIG_SC7180_QSPI) += qspi.c +romstage-$(CONFIG_QC_SDI_ENABLE) += ../../../security/vboot/gbb.c
################################################################################ ramstage-y += soc.c
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36518/1/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/1/src/soc/qualcomm/common/qcl... PS1, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
https://review.coreboot.org/c/coreboot/+/36518/1/src/soc/qualcomm/common/qcl... PS1, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { trailing whitespace
https://review.coreboot.org/c/coreboot/+/36518/1/src/soc/qualcomm/common/qcl... PS1, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/36518/1/src/soc/qualcomm/common/qcl... PS1, Line 163: struct prog qcsdi = trailing whitespace
https://review.coreboot.org/c/coreboot/+/36518/1/src/soc/qualcomm/common/qcl... PS1, Line 164: PROG_INIT(PROG_REFCODE, trailing whitespace
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36518/2/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/2/src/soc/qualcomm/common/qcl... PS2, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
https://review.coreboot.org/c/coreboot/+/36518/2/src/soc/qualcomm/common/qcl... PS2, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { trailing whitespace
https://review.coreboot.org/c/coreboot/+/36518/2/src/soc/qualcomm/common/qcl... PS2, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/36518/2/src/soc/qualcomm/common/qcl... PS2, Line 163: struct prog qcsdi = trailing whitespace
https://review.coreboot.org/c/coreboot/+/36518/2/src/soc/qualcomm/common/qcl... PS2, Line 164: PROG_INIT(PROG_REFCODE, trailing whitespace
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36518/3/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/3/src/soc/qualcomm/common/qcl... PS3, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
https://review.coreboot.org/c/coreboot/+/36518/3/src/soc/qualcomm/common/qcl... PS3, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { trailing whitespace
https://review.coreboot.org/c/coreboot/+/36518/3/src/soc/qualcomm/common/qcl... PS3, Line 162: if(!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/36518/3/src/soc/qualcomm/common/qcl... PS3, Line 163: struct prog qcsdi = trailing whitespace
https://review.coreboot.org/c/coreboot/+/36518/3/src/soc/qualcomm/common/qcl... PS3, Line 164: PROG_INIT(PROG_REFCODE, trailing whitespace
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#4).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c M src/soc/qualcomm/sc7180/Makefile.inc 2 files changed, 15 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/4/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/4/src/soc/qualcomm/common/qcl... PS4, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/5/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/5/src/soc/qualcomm/common/qcl... PS5, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/6/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/6/src/soc/qualcomm/common/qcl... PS6, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/common/qcl... PS7, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/8/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/8/src/soc/qualcomm/common/qcl... PS8, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/common/qcl... PS7, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) {
'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
This is how flag is spelled in vboot source code.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/common/qcl... PS7, Line 162: if (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE))) {
This is how flag is spelled in vboot source code.
It's fine, just ignore the warning.
Also, can we just write this with && (instead of nested if)?
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/sc7180/Mak... File src/soc/qualcomm/sc7180/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/sc7180/Mak... PS7, Line 43: romstage-$(CONFIG_QC_SDI_ENABLE) += ../../../security/vboot/gbb.c Please just change security/vboot/Makefile.inc to always do this (in a separate patch).
...actually, there are more issues with that... hmm... let me just quickly do this and then this becomes moot: CB:37261
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#9).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c M src/soc/qualcomm/sc7180/Makefile.inc 2 files changed, 15 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/9/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/9/src/soc/qualcomm/common/qcl... PS9, Line 161: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/9/src/soc/qualcomm/common/qcl... PS9, Line 162: (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/10/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/10/src/soc/qualcomm/common/qc... PS10, Line 161: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/10/src/soc/qualcomm/common/qc... PS10, Line 162: (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/11/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/11/src/soc/qualcomm/common/qc... PS11, Line 161: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/11/src/soc/qualcomm/common/qc... PS11, Line 162: (!(gbb_is_flag_set(VB2_GBB_FLAG_FAFT_KEY_OVERIDE)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#12).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 21 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/12
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/12/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/12/src/soc/qualcomm/common/qc... PS12, Line 165: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/12/src/soc/qualcomm/common/qc... PS12, Line 166: (!(VB2_GBB_FLAG_FAFT_KEY_OVERIDE & vb2api_gbb_get_flags(ctx)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/13/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/13/src/soc/qualcomm/common/qc... PS13, Line 165: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/13/src/soc/qualcomm/common/qc... PS13, Line 166: (!(VB2_GBB_FLAG_FAFT_KEY_OVERIDE & vb2api_gbb_get_flags(ctx)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#14).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 21 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/14
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/14/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/14/src/soc/qualcomm/common/qc... PS14, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/14/src/soc/qualcomm/common/qc... PS14, Line 167: (!(VB2_GBB_FLAG_FAFT_KEY_OVERIDE & vb2api_gbb_get_flags(ctx)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/15/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/15/src/soc/qualcomm/common/qc... PS15, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/15/src/soc/qualcomm/common/qc... PS15, Line 167: (!(VB2_GBB_FLAG_FAFT_KEY_OVERIDE & vb2api_gbb_get_flags(ctx)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/16/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/16/src/soc/qualcomm/common/qc... PS16, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
https://review.coreboot.org/c/coreboot/+/36518/16/src/soc/qualcomm/common/qc... PS16, Line 167: (!(VB2_GBB_FLAG_FAFT_KEY_OVERIDE & vb2api_gbb_get_flags(ctx)))) { 'OVERIDE' may be misspelled - perhaps 'OVERRIDE'?
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#17).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 21 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/17
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/17/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/17/src/soc/qualcomm/common/qc... PS17, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/18/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/18/src/soc/qualcomm/common/qc... PS18, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/19/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/19/src/soc/qualcomm/common/qc... PS19, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/20/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/20/src/soc/qualcomm/common/qc... PS20, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... PS21, Line 166: if (CONFIG(QC_SDI_ENABLE) && suspect code indent for conditional statements (8, 24)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 21:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... PS21, Line 168: This should only have a single indent. You can align the above continuation line with the opening parenthesis of the if() instead to make the distinction clear.
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... PS21, Line 227: qup_spi_init(QUPV3_1_SE0, 1010 * KHz); /* H1 SPI */ I know we still need this HACK until this can get fixed in QcLib (btw is someone tracking that?), but it doesn't belong in this patch.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 21:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG@7 PS21, Line 7: VB2_GBB_FLAG_FAFT_KEY_OVERIDE OVERRIDE?
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG@7 PS21, Line 7: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting Please make that a statement about the change, and use imperative mood.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG@7 PS21, Line 7: VB2_GBB_FLAG_FAFT_KEY_OVERIDE
OVERRIDE?
No, the flag was actually spelled like that. We recently renamed it to VB2_GBB_FLAG_RUNNING_FAFT, so this is out of date anyway.
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#22).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 14 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/22
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... PS21, Line 227: qup_spi_init(QUPV3_1_SE0, 1010 * KHz); /* H1 SPI */
I know we still need this HACK until this can get fixed in QcLib (btw is someone tracking that?), bu […]
One of the QC engineer working on this HACK patch
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 22:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG@7 PS21, Line 7: VB2_GBB_FLAG_FAFT_KEY_OVERIDE
No, the flag was actually spelled like that. […]
Done
https://review.coreboot.org/c/coreboot/+/36518/21//COMMIT_MSG@7 PS21, Line 7: trogdor: QCSDI loading depends on VB2_GBB_FLAG_FAFT_KEY_OVERIDE setting
Please make that a statement about the change, and use imperative mood.
Done
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... PS21, Line 168:
This should only have a single indent. […]
Done
https://review.coreboot.org/c/coreboot/+/36518/21/src/soc/qualcomm/common/qc... PS21, Line 227: qup_spi_init(QUPV3_1_SE0, 1010 * KHz); /* H1 SPI */
One of the QC engineer working on this HACK patch
Done
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#24).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/24
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 26:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/26/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/26/src/soc/qualcomm/common/qc... PS26, Line 164: (!(VB2_GBB_FLAG_RUNNING_FAFT & vb2api_gbb_get_flags(ctx)))) { You need a
CONFIG(VBOOT) &&
check before this so it compiles when vboot is disabled.
Hello ashk@codeaurora.org, Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#27).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/27
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/26/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/26/src/soc/qualcomm/common/qc... PS26, Line 164: (!(VB2_GBB_FLAG_RUNNING_FAFT & vb2api_gbb_get_flags(ctx)))) {
You need a […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context(); This still fails when CONFIG(VBOOT) is disabled. Remove the variable and actually write
CONFIG(VBOOT) && (!(vb2api_gbb_get_flags(vboot_get_context()) & VB2_GBB_FLAG_RUNNING_FAFT)
below
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
This still fails when CONFIG(VBOOT) is disabled. Remove the variable and actually write […]
Actually, even better:
CONFIG(VBOOT) && vboot_is_gbb_flag_set(VB2_GBB_FLAG_RUNNING_FAFT)
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 31:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
Actually, even better: […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 32:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
Done
Where? There are no actual changes in the latest uploads.
Ravi kumar has uploaded a new patch set (#33) to the change originally created by mturney mturney. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/33
Hello Ravi kumar, build bot (Jenkins), Patrick Georgi, Martin Roth, ashk@codeaurora.org,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36518
to look at the new patch set (#34).
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/34
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 35:
Julius, we have tried your suggestion as is and it introduced issues that were picked up by BVT testing related to SDI and IDP. Until we resolve these issues we cannot adopt your suggestion. We will keep you updated on our progress.
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 35:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
Where? There are no actual changes in the latest uploads.
Julius, we have tried your suggestion as is and it introduced issues that were picked up by BVT testing related to SDI and IDP. Until we resolve these issues we cannot adopt your suggestion. We will keep you updated on our progress.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 36:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
Julius, we have tried your suggestion as is and it introduced issues that were picked up by BVT test […]
I mean... I'm not suggesting you change anything functionally, I'm just asking you to rewrite it so that it will build correctly when CONFIG_VBOOT is disabled. If you saw a functional change with that you must have made some mistake. If you upload what you tried, maybe I can help you find it.
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 36:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
I mean... […]
Hi Julius, Do you mean to remove this member "struct vb2_context *ctx = vboot_get_context();"
And replace like below condition: if (CONFIG(QC_SDI_ENABLE) && CONFIG(VBOOT) && (!(vb2api_gbb_get_flags(vboot_get_context()) & VB2_GBB_FLAG_RUNNING_FAFT))))
after doing this changes, not able to work SDI_ENABLE feature. Could you please suggest/correct me,if i understood wrong.
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 36:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
Hi Julius, […]
SDI_ENABLE/DISABLE feature we are using in bubs
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 36:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
SDI_ENABLE/DISABLE feature we are using in bubs
Yes, like that. Why would that not work? You're literally just replacing 'ctx' with 'vboot_get_context()' which is exactly what that variable was initialized to. This shouldn't change anything. If you observe any difference you must be making a mistake somewhere else (maybe you manually set the GBB flag 0x100 or something? Sometimes it can get stuck on if FAFT was aborted by some error).
However, using the vboot_is_gbb_flag_set() helper (from <security/vboot/misc.h>) would do the same thing a bit cleaner. Also, technically, this patch (and that suggestion) still has a mistake for the case where CONFIG(VBOOT) is disabled. So the condition should really be
if (CONFIG(QC_SDI_ENABLE) && (!CONFIG(VBOOT) || !vboot_is_gbb_flag_set(VB2_GBB_FLAG_RUNNING_FAFT)))
Ravi kumar has uploaded a new patch set (#37) to the change originally created by mturney mturney. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 8 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36518/37
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 37:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/36518/28/src/soc/qualcomm/common/qc... PS28, Line 130: struct vb2_context *ctx = vboot_get_context();
Yes, like that. […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 38: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
Patch Set 43:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/sc7180/Mak... File src/soc/qualcomm/sc7180/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36518/7/src/soc/qualcomm/sc7180/Mak... PS7, Line 43: romstage-$(CONFIG_QC_SDI_ENABLE) += ../../../security/vboot/gbb.c
Please just change security/vboot/Makefile.inc to always do this (in a separate patch). […]
Done
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36518 )
Change subject: trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag ......................................................................
trogdor: QCSDI loading depends on VB2_GBB_FLAG_RUNNING_FAFT setting flag
Change-Id: I63f35c94bc6c60934ace5fe0fd9176443059b354 Signed-off-by: Ashwin Kumar ashk@codeaurora.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/36518 Reviewed-by: Julius Werner jwerner@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/qualcomm/common/qclib.c 1 file changed, 8 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/soc/qualcomm/common/qclib.c b/src/soc/qualcomm/common/qclib.c index 6bf2f0e..d06cb42 100644 --- a/src/soc/qualcomm/common/qclib.c +++ b/src/soc/qualcomm/common/qclib.c @@ -13,6 +13,8 @@ #include <soc/mmu_common.h> #include <soc/qclib_common.h> #include <soc/symbols_common.h> +#include <security/vboot/misc.h> +#include <vb2_api.h>
struct qclib_cb_if_table qclib_cb_if_table = { .magic = QCLIB_MAGIC_NUMBER, @@ -144,9 +146,11 @@ qclib_cb_if_table.global_attributes = QCLIB_GA_ENABLE_UART_LOGGING;
- if (CONFIG(QC_SDI_ENABLE)) { + if (CONFIG(QC_SDI_ENABLE) && (!CONFIG(VBOOT) || + !vboot_is_gbb_flag_set(VB2_GBB_FLAG_RUNNING_FAFT))) { struct prog qcsdi = - PROG_INIT(PROG_REFCODE, CONFIG_CBFS_PREFIX "/qcsdi"); + PROG_INIT(PROG_REFCODE, + CONFIG_CBFS_PREFIX "/qcsdi");
/* Attempt to load QCSDI elf */ if (prog_locate(&qcsdi)) @@ -155,8 +159,8 @@ if (cbfs_prog_stage_load(&qcsdi)) goto fail;
- qclib_add_if_table_entry(QCLIB_TE_QCSDI, prog_entry(&qcsdi), - prog_size(&qcsdi), 0); + qclib_add_if_table_entry(QCLIB_TE_QCSDI, + prog_entry(&qcsdi), prog_size(&qcsdi), 0); printk(BIOS_INFO, "qcsdi.entry[%p]\n", qcsdi.entry); }