10 comments:
Patch Set #27, Line 25: #include <security/tpm/tspi/crtm.h>
Check your alphabetical order. (Other files too.)
Patch Set #27, Line 73: !CONFIG(VBOOT) && CONFIG(TSPI_MEASURED_BOOT)
Why can't we just run this unconditionally here, and remove the call from verstage?
File src/mainboard/siemens/mc_apl1/variants/mc_apl2/Kconfig:
config VBOOT
select TSPI_MEASURED_BOOT
Shouldn't this live under TPM now?
File src/security/tpm/Kconfig:
Patch Set #27, Line 119: Runtime data whitelist of cbfs filenames. Needs to be a comma separated
This line looks longer than all the others?
File src/security/tpm/tspi/crtm.h:
Try to keep a blank line in between function headers?
File src/security/tpm/tspi/crtm.c:
Patch Set #27, Line 25: This functions
This function or these functions?
If singular, probably don't need a blank line below.
Patch Set #27, Line 162: int crtm_is_set;
Can this be a static variable inside the function?
Patch Set #27, Line 218: boot_platform_is_resuming()
If this is not just used by vboot_ anymore, we should find a better home for it.
if (result != VB2_SUCCESS) {
printk(BIOS_INFO,
"Initializing CRTM failed!");
} else {
crtm_is_set = 1;
}
I think that printk can fit on one line now, with the longer coreboot.org line length limit.
And for one-statement conditional bodies, we don't need the {} braces here.
File src/security/vboot/vboot_logic.c:
if (CONFIG(TSPI_MEASURED_BOOT) &&
!(ctx->flags & VB2_CONTEXT_S3_RESUME)) {
if (tspi_init_crtm() != VB2_SUCCESS)
Can we just always run this in bootblock and remove the code here?
To view, visit change 35077. To unsubscribe, or for help writing mail filters, visit settings.