coreboot-gerrit February 2019

coreboot-gerrit@coreboot.org

1 participants
1249 discussions

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by build bot (Jenkins) (Code Review) 20 Feb '19

20 Feb '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 62: (14 comments) https://review.coreboot.org/#/c/29547/62/src/lib/cbfs.c File src/lib/cbfs.c: https://review.coreboot.org/#/c/29547/62/src/lib/cbfs.c@102 PS62, Line 102: size_t in_size, void *buffer, size_t buffer_size, uint32_t compression) line over 80 characters https://review.coreboot.org/#/c/29547/62/src/lib/cbfs.c@115 PS62, Line 115: if ((ENV_BOOTBLOCK || ENV_VERSTAGE) && !IS_ENABLED(CONFIG_COMPRESS_PRERAM_STAGES)) line over 80 characters https://review.coreboot.org/#/c/29547/62/src/lib/cbfs.c@257 PS62, Line 257: if (ENV_VERSTAGE && !IS_ENABLED(CONFIG_NO_XIP_EARLY_STAGES) && IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED)) { line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_crtm.h File src/security/vboot/vboot_crtm.h: https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_crtm.h@49 PS62, Line 49: #if (IS_ENABLED(CONFIG_VBOOT_MEASURED_BOOT) && !ENV_BOOTBLOCK && !ENV_DECOMPRESSOR) line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c: https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_crtm.c@38 PS62, Line 38: if (cbfs_boot_locate(&bootblock_data, prog_name(&bootblock), NULL) == 0) { line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_crtm.c@55 PS62, Line 55: if (cbfs_boot_locate(&romstage_data, prog_name(&romstage), NULL) == 0) { line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_crtm.c@73 PS62, Line 73: if (cbfs_boot_locate(&verstage_data, prog_name(&verstage), NULL) == 0) { line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c File src/security/vboot/vboot_logic.c: https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@98 PS62, Line 98: BUG(); /* Should never get called if init() returned an error. */ Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@1… PS62, Line 104: BUG(); /* Should never get called if init() returned an error. */ Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@2… PS62, Line 281: return vboot_extend_pcr(ctx, 0, BOOT_MODE_PCR) || vboot_extend_pcr(ctx, 1, HWID_DIGEST_PCR); line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@3… PS62, Line 308: if (IS_ENABLED(CONFIG_RESUME_PATH_SAME_AS_BOOT) && vboot_platform_is_resuming()) line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@3… PS62, Line 319: if (IS_ENABLED(CONFIG_VBOOT_MEASURED_BOOT) && !(ctx.flags & VB2_CONTEXT_S3_RESUME)) { line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@3… PS62, Line 324: if (IS_ENABLED(CONFIG_VBOOT_PHYSICAL_DEV_SWITCH) && get_developer_mode_switch()) line over 80 characters https://review.coreboot.org/#/c/29547/62/src/security/vboot/vboot_logic.c@3… PS62, Line 333: if (IS_ENABLED(CONFIG_VBOOT_WIPEOUT_SUPPORTED) && get_wipeout_mode_switch()) line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 62 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 20 Feb 2019 12:48:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Philipp Deppenwiese (Code Review) 20 Feb '19

20 Feb '19

Hello Patrick Rudolph, Aaron Durbin, Piotr Król, Julius Werner, Krystian Hebel, Patrick Rudolph, Stefan Reinauer, Paul Menzel, build bot (Jenkins), Patrick Georgi, Werner Zeh, Huang Jin, York Yang, David Hendricks, Martin Roth, Michał Żygowski, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29547 to look at the new patch set (#62). Change subject: security/vboot: Add measured boot mode ...................................................................... security/vboot: Add measured boot mode * Introduce a measured boot mode into vboot. * Add hook for stage measurements in prog_loader and cbfs. * Implement and hook-up CRTM in vboot and check for suspend. Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Signed-off-by: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> --- M Documentation/index.md A Documentation/security.md A Documentation/security/index.md A Documentation/security/vboot/measured_boot.md A Documentation/security/vboot/srtm.png M src/cpu/intel/haswell/Makefile.inc M src/cpu/intel/model_2065x/Makefile.inc M src/cpu/intel/model_206ax/Makefile.inc M src/lib/cbfs.c M src/lib/prog_loaders.c M src/security/tpm/tspi/tspi.c M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc A src/security/vboot/vboot_crtm.c A src/security/vboot/vboot_crtm.h M src/security/vboot/vboot_logic.c M src/soc/amd/stoneyridge/Makefile.inc M src/soc/intel/baytrail/Makefile.inc M src/soc/intel/braswell/Makefile.inc M src/soc/intel/broadwell/Makefile.inc M src/soc/intel/fsp_baytrail/Makefile.inc M src/soc/intel/fsp_broadwell_de/Makefile.inc M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/rockchip/rk3288/include/soc/memlayout.ld M util/abuild/abuild 25 files changed, 347 insertions(+), 52 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/29547/62 -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 62 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Philipp Deppenwiese (Code Review) 20 Feb '19

20 Feb '19

Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 61: @Piotr not yet. -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 61 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 20 Feb 2019 12:47:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by build bot (Jenkins) (Code Review) 20 Feb '19

20 Feb '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 61: (14 comments) https://review.coreboot.org/#/c/29547/61/src/lib/cbfs.c File src/lib/cbfs.c: https://review.coreboot.org/#/c/29547/61/src/lib/cbfs.c@102 PS61, Line 102: size_t in_size, void *buffer, size_t buffer_size, uint32_t compression) line over 80 characters https://review.coreboot.org/#/c/29547/61/src/lib/cbfs.c@115 PS61, Line 115: if ((ENV_BOOTBLOCK || ENV_VERSTAGE) && !IS_ENABLED(CONFIG_COMPRESS_PRERAM_STAGES)) line over 80 characters https://review.coreboot.org/#/c/29547/61/src/lib/cbfs.c@257 PS61, Line 257: if (ENV_VERSTAGE && !IS_ENABLED(CONFIG_NO_XIP_EARLY_STAGES) && IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED)) { line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_crtm.h File src/security/vboot/vboot_crtm.h: https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_crtm.h@49 PS61, Line 49: #if (IS_ENABLED(CONFIG_VBOOT_MEASURED_BOOT) && !ENV_BOOTBLOCK && !ENV_DECOMPRESSOR) line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c: https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_crtm.c@38 PS61, Line 38: if (cbfs_boot_locate(&bootblock_data, prog_name(&bootblock), NULL) == 0) { line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_crtm.c@55 PS61, Line 55: if (cbfs_boot_locate(&romstage_data, prog_name(&romstage), NULL) == 0) { line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_crtm.c@73 PS61, Line 73: if (cbfs_boot_locate(&verstage_data, prog_name(&verstage), NULL) == 0) { line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c File src/security/vboot/vboot_logic.c: https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@98 PS61, Line 98: BUG(); /* Should never get called if init() returned an error. */ Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@1… PS61, Line 104: BUG(); /* Should never get called if init() returned an error. */ Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@2… PS61, Line 281: return vboot_extend_pcr(ctx, 0, BOOT_MODE_PCR) || vboot_extend_pcr(ctx, 1, HWID_DIGEST_PCR); line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@3… PS61, Line 308: if (IS_ENABLED(CONFIG_RESUME_PATH_SAME_AS_BOOT) && vboot_platform_is_resuming()) line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@3… PS61, Line 319: if (IS_ENABLED(CONFIG_VBOOT_MEASURED_BOOT) && !(ctx.flags & VB2_CONTEXT_S3_RESUME)) { line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@3… PS61, Line 324: if (IS_ENABLED(CONFIG_VBOOT_PHYSICAL_DEV_SWITCH) && get_developer_mode_switch()) line over 80 characters https://review.coreboot.org/#/c/29547/61/src/security/vboot/vboot_logic.c@3… PS61, Line 333: if (IS_ENABLED(CONFIG_VBOOT_WIPEOUT_SUPPORTED) && get_wipeout_mode_switch()) line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 61 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 20 Feb 2019 12:41:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Philipp Deppenwiese (Code Review) 20 Feb '19

20 Feb '19

Hello Patrick Rudolph, Aaron Durbin, Piotr Król, Julius Werner, Krystian Hebel, Patrick Rudolph, Stefan Reinauer, Paul Menzel, build bot (Jenkins), Patrick Georgi, Werner Zeh, Huang Jin, York Yang, David Hendricks, Martin Roth, Michał Żygowski, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29547 to look at the new patch set (#61). Change subject: security/vboot: Add measured boot mode ...................................................................... security/vboot: Add measured boot mode * Introduce a measured boot mode into vboot. * Add hook for stage measurements in prog_loader and cbfs. * Implement and hook-up CRTM in vboot and check for suspend. Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Signed-off-by: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> --- M Documentation/index.md A Documentation/security.md A Documentation/security/index.md A Documentation/security/vboot/measured_boot.md A Documentation/security/vboot/srtm.png M src/cpu/intel/haswell/Makefile.inc M src/cpu/intel/model_2065x/Makefile.inc M src/cpu/intel/model_206ax/Makefile.inc M src/lib/cbfs.c M src/lib/prog_loaders.c M src/security/tpm/tspi/tspi.c M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc A src/security/vboot/vboot_crtm.c A src/security/vboot/vboot_crtm.h M src/security/vboot/vboot_logic.c M src/soc/amd/stoneyridge/Makefile.inc M src/soc/intel/baytrail/Makefile.inc M src/soc/intel/braswell/Makefile.inc M src/soc/intel/broadwell/Makefile.inc M src/soc/intel/fsp_baytrail/Makefile.inc M src/soc/intel/fsp_broadwell_de/Makefile.inc M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/rockchip/rk3288/include/soc/memlayout.ld M util/abuild/abuild 25 files changed, 347 insertions(+), 52 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/29547/61 -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 61 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: soc/intel/apollolake: use correct size for xlate_region_device_ro_ini...
by Christian Gmeiner (Code Review) 20 Feb '19

20 Feb '19

Christian Gmeiner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31499 Change subject: soc/intel/apollolake: use correct size for xlate_region_device_ro_init(..) ...................................................................... soc/intel/apollolake: use correct size for xlate_region_device_ro_init(..) fast_spi_get_bios_region(..) returns the size of the BIOS region as defined in the IFD. It is wrong the substract 256k from that size and use the resulting value to call xlate_region_device_ro_init(..). The result is that the region used by the newly created region_device is 256K smaller then the one defined in IFD. Change-Id: Ic950f28990a645556a04303f04953a7bc5e41a39 Signed-off-by: Christian Gmeiner <christian.gmeiner(a)gmail.com> --- M src/soc/intel/apollolake/mmap_boot.c 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/31499/1 diff --git a/src/soc/intel/apollolake/mmap_boot.c b/src/soc/intel/apollolake/mmap_boot.c index 1c3077e..6b05752 100644 --- a/src/soc/intel/apollolake/mmap_boot.c +++ b/src/soc/intel/apollolake/mmap_boot.c @@ -92,8 +92,7 @@ bios_mapped_size); xlate_region_device_ro_init(real_dev_ptr, &shadow_dev_ptr->rdev, - start, bios_mapped_size, - CONFIG_ROM_SIZE); + start, size, CONFIG_ROM_SIZE); car_set_var(bios_size, size); } -- To view, visit https://review.coreboot.org/c/coreboot/+/31499 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic950f28990a645556a04303f04953a7bc5e41a39 Gerrit-Change-Number: 31499 Gerrit-PatchSet: 1 Gerrit-Owner: Christian Gmeiner <christian.gmeiner(a)gmail.com> Gerrit-MessageType: newchange

3 4

Change in ...coreboot[master]: util/lint: Exclude util/inteltool from checkpatch
by Nico Huber (Code Review) 20 Feb '19

20 Feb '19

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31367 Change subject: util/lint: Exclude util/inteltool from checkpatch ...................................................................... util/lint: Exclude util/inteltool from checkpatch It's causing too much noise during review of register tables. Change-Id: Iae6cd4454c5ed84b5fe0ea5f8a244e2a2fa13407 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M util/lint/lint-007-checkpatch 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31367/1 diff --git a/util/lint/lint-007-checkpatch b/util/lint/lint-007-checkpatch index abc63f1..afa593e 100755 --- a/util/lint/lint-007-checkpatch +++ b/util/lint/lint-007-checkpatch @@ -22,6 +22,7 @@ EXCLUDED_DIRS="^payloads/libpayload/util/kconfig\|\ ^payloads/libpayload/curses/PDCurses\|\ ^util/crossgcc/patches\|\ +^util/inteltool\|\ ^util/kconfig\|\ ^util/superiotool\|\ ^src/vendorcode\|\ -- To view, visit https://review.coreboot.org/c/coreboot/+/31367 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iae6cd4454c5ed84b5fe0ea5f8a244e2a2fa13407 Gerrit-Change-Number: 31367 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

3 4

Change in ...coreboot[master]: soc/intel/common: Implement EFI_MP_SERVICES_PPI structure APIs
by Nico Huber (Code Review) 20 Feb '19

20 Feb '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25634 ) Change subject: soc/intel/common: Implement EFI_MP_SERVICES_PPI structure APIs ...................................................................... Patch Set 47: (1 comment) Beside the defined to be resolved part, my next concern is that the current implementation doesn't follow the named EFI interface accurately. IMHO, we should either never mention the EFI standard or follow it exactly. No short- cuts, please. > @Ron/Philipp/Nico: > > Do you have only concern with making use of EFI datatypes ? if so, we can think about something like below https://github.com/u-boot/u-boot/blob/master/include/efi_api.h I don't think we should use the exact U-Boot definitions (see inline comment). The names they chose, however, look much better (one could bikeshed that _t is reserved for standard C types, but let's not). We have to use the exact EFI types. If I understood correctly, Ron's concern was only about their upper-case naming. Please move the discussion about the types somewhere else. Looking at this change here is not good for me. https://review.coreboot.org/#/c/25634/47/src/soc/intel/common/block/cpu/mp_… File src/soc/intel/common/block/cpu/mp_service_ppi.c: https://review.coreboot.org/#/c/25634/47/src/soc/intel/common/block/cpu/mp_… PS47, Line 27: static EFI_STATUS mp_get_number_of_processors(CONST > #define efi_uintn_t size_t We know that this would work with our current toolchain. But I don't see a reason to make it depend on a vaguely defined type such as `size_t` (AFAIK, it's definition is `return type of the sizeof operator`). Please decide first, what problem you want to solve: a) getting rid of the upper-case names only or b) getting rid of the UDK header files too. For a) it would suffice to redefine the existing types, e.g.: typedef UINTN efi_uintn_t; For b) please *don't* depend on vague types like `size_t`. Mimic the respective part of the UDK headers instead, e.g. somewhere in the x86 include path have: typedef uint32_t efi_uintn_t; And other architectures (if need be) can define it differently. If we ever want to interface the existing 32-bit FSP binaries with x86_64 code, we'll have to use something like that (for all archi- tectures): typedef uint32_t efi32_uintn_t; -- To view, visit https://review.coreboot.org/c/coreboot/+/25634 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie844e3f15f759ea09a8f3fd24825ee740151c956 Gerrit-Change-Number: 25634 Gerrit-PatchSet: 47 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Vincent Zimmer <vincent.zimmer(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma(a)intel.com> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com> Gerrit-Comment-Date: Wed, 20 Feb 2019 12:15:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com> Comment-In-Reply-To: ron minnich <rminnich(a)gmail.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Werner Zeh (Code Review) 20 Feb '19

20 Feb '19

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 60: (11 comments) https://review.coreboot.org/#/c/29547/60/Documentation/security/vboot/measu… File Documentation/security/vboot/measured_boot.md: https://review.coreboot.org/#/c/29547/60/Documentation/security/vboot/measu… PS60, Line 2: extensions extension https://review.coreboot.org/#/c/29547/60/Documentation/security/vboot/measu… PS60, Line 3: be https://review.coreboot.org/#/c/29547/60/src/security/vboot/Kconfig File src/security/vboot/Kconfig: https://review.coreboot.org/#/c/29547/60/src/security/vboot/Kconfig@38 PS60, Line 38: default "hwinfo.hex" Does it make sense to have this default here? Only Siemens mainboards have this CBFS entry. https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c: https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@34 PS60, Line 34: == 0) { Weird code formatting. Maybe change to: if (!fmap_locate_area_as_rdev("BOOTBLOCK",&bootblock_fmap)... Should fit on one line then with no need of breaking it. https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@44 PS60, Line 44: == 0) same here https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@65 PS60, Line 65: == 0) { same here https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@68 PS60, Line 68: if (tpm_measure_region(prog_rdev(&romstage), : TPM_CRTM_PCR, : CONFIG_CBFS_PREFIX : "/romstage") : != TPM_SUCCESS) { TPM_SUCCESS is defined as 0x0. Why not just: if (tpm_measure_region(prog_rdev(&romstage), TPM_CRTM_PCR, CONFIG_CBFS_PREFIX"/romstage")) return return VB2_ERROR_UNKNOWN; https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@89 PS60, Line 89: == 0) { see above https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@92 PS60, Line 92: if (tpm_measure_region(prog_rdev(&verstage), : TPM_CRTM_PCR, : CONFIG_CBFS_PREFIX : "/verstage") : != TPM_SUCCESS) { : return VB2_ERROR_UNKNOWN; here again. https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@110 PS60, Line 110: find_runtime_data_string Maybe rename this function to something like "is_runtime_data()" or "is_whitelist_data()" as it just tells you if the given file is part of the runtime/whitelist data list or not. https://review.coreboot.org/#/c/29547/60/src/security/vboot/vboot_crtm.c@118 PS60, Line 118: + spaces around '+' -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 60 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 20 Feb 2019 11:56:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/common: Implement EFI_MP_SERVICES_PPI structure APIs
by Subrata Banik (Code Review) 20 Feb '19

20 Feb '19

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25634 ) Change subject: soc/intel/common: Implement EFI_MP_SERVICES_PPI structure APIs ...................................................................... Patch Set 47: > Subrata you also pass camel case function parameters. Just ditch > camel case completely and also try to use lowercase for data types. > Even if you define them yourself. got it. we will try to modify those and follow u-boot typedef example as suggested above. -- To view, visit https://review.coreboot.org/c/coreboot/+/25634 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie844e3f15f759ea09a8f3fd24825ee740151c956 Gerrit-Change-Number: 25634 Gerrit-PatchSet: 47 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Vincent Zimmer <vincent.zimmer(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma(a)intel.com> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com> Gerrit-Comment-Date: Wed, 20 Feb 2019 10:45:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2019