Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31329 )
Change subject: vboot: copy data structures to CBMEM for downstream use ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/31329/1/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/31329/1/src/security/vboot/common.c@43 PS1, Line 43: static struct vb2_working_data *vb2_wd = NULL;
fwiw, static uninitialized variables are 0 by default as they land in the .bss. […]
We could just mark this CAR_GLOBAL, right? Shouldn't hurt?
https://review.coreboot.org/#/c/31329/1/src/security/vboot/common.c@48 PS1, Line 48: #if ENV_POSTCAR || ENV_RAMSTAGE
Yes, you have to handle the ENV_ cases as well when combined w/ VBOOT_STARTS_IN and SEPARATE_VERSTAG […]
Well... I don't see any existing helpers that would really help here (other than cbmem_possibly_online() in the way we're already using it). Whether vboot executed is mostly irrelevant here, the question is whether CBMEM is available. We could add another cbmem_definitely_online() for it, but in the end that's really just the same as __PRE_RAM__. (We could also add an ENV_PRE_RAM so we don't always need to use a raw #ifdef for that?)