Change in coreboot[master]: security/vboot: Add Kconfig symbol to set hashing block size
Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
security/vboot: Add Kconfig symbol to set hashing block size
Generally, this size probably doesn't matter very much, but in the case of picasso's psp_verstage, the hash is being calculated by hardware using relatively expensive system calls. By increasing the block size, we can save roughly 140ms of boot and resume time.
TEST=Build & boot see that boot time has decreased. BRANCH=Zork BUG=b:169217270 - Zork: SHA calculation in vboot takes too long
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I68eecbbdfadcbf14288dc6e849397724fb66e0b2 --- M src/security/vboot/Kconfig M src/security/vboot/vboot_logic.c 2 files changed, 8 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/46901/1
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 094cbb9..024a746 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -369,6 +369,13 @@ hex "Keyblock preamble flags" default 0x0
+config VBOOT_HASH_BLOCK_SIZE + hex + default 0x400 + help + Set the default hash size. Generally 1k is reasonable, but in some + cases it may improve hashing speed to increase the size. + endmenu # Keys endif # VBOOT endmenu # Verified Boot (vboot) diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index dbaa883..54c8224 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -19,8 +19,6 @@ /* The max hash size to expect is for SHA512. */ #define VBOOT_MAX_HASH_SIZE VB2_SHA512_DIGEST_SIZE
-#define TODO_BLOCK_SIZE 1024 - /* exports */
vb2_error_t vb2ex_read_resource(struct vb2_context *ctx, @@ -144,7 +142,7 @@ { uint64_t load_ts; uint32_t remaining; - uint8_t block[TODO_BLOCK_SIZE]; + uint8_t block[CONFIG_VBOOT_HASH_BLOCK_SIZE]; uint8_t hash_digest[VBOOT_MAX_HASH_SIZE]; const size_t hash_digest_sz = sizeof(hash_digest); size_t block_size = sizeof(block);
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46901/1/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/46901/1/src/security/vboot/Kconfig@... PS1, Line 377: cases it may improve hashing speed to increase the size. Please point out explicitly that this is stack-allocated and that special care must be taken not to exceed the stack size when changing this.
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46901
to look at the new patch set (#2).
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
security/vboot: Add Kconfig symbol to set hashing block size
Generally, this size probably doesn't matter very much, but in the case of picasso's psp_verstage, the hash is being calculated by hardware using relatively expensive system calls. By increasing the block size, we can save roughly 140ms of boot and resume time.
TEST=Build & boot see that boot time has decreased. BRANCH=Zork BUG=b:169217270 - Zork: SHA calculation in vboot takes too long
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I68eecbbdfadcbf14288dc6e849397724fb66e0b2 --- M src/security/vboot/Kconfig M src/security/vboot/vboot_logic.c 2 files changed, 12 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/46901/2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46901/1/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/46901/1/src/security/vboot/Kconfig@... PS1, Line 377: cases it may improve hashing speed to increase the size.
Please point out explicitly that this is stack-allocated and that special care must be taken not to […]
Done.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
Patch Set 2: Code-Review+2
Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
Patch Set 2: Code-Review+1
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46901 )
Change subject: security/vboot: Add Kconfig symbol to set hashing block size ......................................................................
security/vboot: Add Kconfig symbol to set hashing block size
Generally, this size probably doesn't matter very much, but in the case of picasso's psp_verstage, the hash is being calculated by hardware using relatively expensive system calls. By increasing the block size, we can save roughly 140ms of boot and resume time.
TEST=Build & boot see that boot time has decreased. BRANCH=Zork BUG=b:169217270 - Zork: SHA calculation in vboot takes too long
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I68eecbbdfadcbf14288dc6e849397724fb66e0b2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46901 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Kangheui Won khwon@chromium.org --- M src/security/vboot/Kconfig M src/security/vboot/vboot_logic.c 2 files changed, 12 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Kangheui Won: Looks good to me, but someone else must approve
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 094cbb9..e774478 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -369,6 +369,17 @@ hex "Keyblock preamble flags" default 0x0
+config VBOOT_HASH_BLOCK_SIZE + hex + default 0x400 + help + Set the default hash size. Generally 1k is reasonable, but in some + cases it may improve hashing speed to increase the size. + + Note that this buffer is allocated in the stack. Although the + build should fail if the stack size is exceeded, it's something to + be aware of when changing the size. + endmenu # Keys endif # VBOOT endmenu # Verified Boot (vboot) diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index dbaa883..54c8224 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -19,8 +19,6 @@ /* The max hash size to expect is for SHA512. */ #define VBOOT_MAX_HASH_SIZE VB2_SHA512_DIGEST_SIZE
-#define TODO_BLOCK_SIZE 1024 - /* exports */
vb2_error_t vb2ex_read_resource(struct vb2_context *ctx, @@ -144,7 +142,7 @@ { uint64_t load_ts; uint32_t remaining; - uint8_t block[TODO_BLOCK_SIZE]; + uint8_t block[CONFIG_VBOOT_HASH_BLOCK_SIZE]; uint8_t hash_digest[VBOOT_MAX_HASH_SIZE]; const size_t hash_digest_sz = sizeof(hash_digest); size_t block_size = sizeof(block);
-
Aaron Durbin (Code Review) -
Felix Held (Code Review)
-
Furquan Shaikh (Code Review)
-
Julius Werner (Code Review)
-
Kangheui Won (Code Review)
-
Martin Roth (Code Review)