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)))