Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
vboot: update secdata naming scheme
secdata -> secdata_firmware secdatak -> secdata_firmware
BUG=b:124141368, chromium:972956 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ie2051de51c8f483a8921831385557fad816eb9fb Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/secdata_tpm.c 1 file changed, 21 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37655/1
diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index ef24555..b719edb 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -67,10 +67,11 @@ int attempts = 3;
while (attempts--) { - RETURN_ON_FAILURE(tlcl_read(FIRMWARE_NV_INDEX, ctx->secdata, - VB2_SECDATA_SIZE)); + RETURN_ON_FAILURE(tlcl_read(FIRMWARE_NV_INDEX, + ctx->secdata_firmware, + VB2_SECDATA_FIRMWARE_SIZE));
- if (vb2api_secdata_check(ctx) == VB2_SUCCESS) + if (vb2api_secdata_firmware_check(ctx) == VB2_SUCCESS) return TPM_SUCCESS;
VBDEBUG("TPM: %s() - bad CRC\n", __func__); @@ -194,14 +195,14 @@ static uint32_t set_firmware_space(const void *firmware_blob) { return set_space("firmware", FIRMWARE_NV_INDEX, firmware_blob, - VB2_SECDATA_SIZE, ro_space_attributes, + VB2_SECDATA_FIRMWARE_SIZE, ro_space_attributes, pcr0_unchanged_policy, sizeof(pcr0_unchanged_policy)); }
static uint32_t set_kernel_space(const void *kernel_blob) { return set_space("kernel", KERNEL_NV_INDEX, kernel_blob, - VB2_SECDATAK_SIZE, rw_space_attributes, NULL, 0); + VB2_SECDATA_KERNEL_SIZE, rw_space_attributes, NULL, 0); }
static uint32_t set_rec_hash_space(const uint8_t *data) @@ -222,12 +223,12 @@ * indication that TPM factory initialization was successfully * completed. */ - RETURN_ON_FAILURE(set_kernel_space(ctx->secdatak)); + RETURN_ON_FAILURE(set_kernel_space(ctx->secdata_kernel));
if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) RETURN_ON_FAILURE(set_rec_hash_space(rec_hash_data));
- RETURN_ON_FAILURE(set_firmware_space(ctx->secdata)); + RETURN_ON_FAILURE(set_firmware_space(ctx->secdata_firmware));
return TPM_SUCCESS; } @@ -326,22 +327,22 @@ VBDEBUG("TPM: Clearing owner\n"); RETURN_ON_FAILURE(tpm_clear_and_reenable());
- /* Define and write secdatak kernel space. */ + /* Define and write secdata_kernel space. */ RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX, TPM_NV_PER_PPWRITE, - VB2_SECDATAK_SIZE)); + VB2_SECDATA_KERNEL_SIZE)); RETURN_ON_FAILURE(write_secdata(KERNEL_NV_INDEX, - ctx->secdatak, - VB2_SECDATAK_SIZE)); + ctx->secdata_kernel, + VB2_SECDATA_KERNEL_SIZE));
- /* Define and write secdata firmware space. */ + /* Define and write secdata_firmware space. */ RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX, TPM_NV_PER_GLOBALLOCK | TPM_NV_PER_PPWRITE, - VB2_SECDATA_SIZE)); + VB2_SECDATA_FIRMWARE_SIZE)); RETURN_ON_FAILURE(write_secdata(FIRMWARE_NV_INDEX, - ctx->secdata, - VB2_SECDATA_SIZE)); + ctx->secdata_firmware, + VB2_SECDATA_FIRMWARE_SIZE));
/* Define and set rec hash space, if available. */ if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) @@ -376,9 +377,9 @@ { uint32_t result;
- /* Set initial values of secdata and secdatak spaces. */ - vb2api_secdata_create(ctx); - vb2api_secdatak_create(ctx); + /* Set initial values of secdata_firmware and secdata_kernel spaces. */ + vb2api_secdata_firmware_create(ctx); + vb2api_secdata_kernel_create(ctx);
VBDEBUG("TPM: factory initialization\n");
@@ -430,7 +431,8 @@ { if (CONFIG(CR50_IMMEDIATELY_COMMIT_FW_SECDATA)) tlcl_cr50_enable_nvcommits(); - return write_secdata(FIRMWARE_NV_INDEX, ctx->secdata, VB2_SECDATA_SIZE); + return write_secdata(FIRMWARE_NV_INDEX, ctx->secdata_firmware, + VB2_SECDATA_FIRMWARE_SIZE); }
uint32_t antirollback_read_space_rec_hash(uint8_t *data, uint32_t size)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37655/1/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/37655/1/src/security/vboot/secdata_... PS1, Line 72: VB2_SECDATA_FIRMWARE_SIZE)); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/37655/1/src/security/vboot/secdata_... PS1, Line 342: VB2_SECDATA_FIRMWARE_SIZE)); code indent should use tabs where possible
Hello Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37655
to look at the new patch set (#2).
Change subject: vboot: update secdata naming scheme ......................................................................
vboot: update secdata naming scheme
secdata -> secdata_firmware secdatak -> secdata_firmware
BUG=b:124141368, chromium:972956 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ie2051de51c8f483a8921831385557fad816eb9fb Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/secdata_tpm.c 1 file changed, 21 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37655/2
Hello Aaron Durbin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37655
to look at the new patch set (#3).
Change subject: vboot: update secdata naming scheme ......................................................................
vboot: update secdata naming scheme
secdata -> secdata_firmware secdatak -> secdata_firmware
BUG=b:124141368, chromium:972956 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ie2051de51c8f483a8921831385557fad816eb9fb Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/secdata_tpm.c 1 file changed, 23 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37655/3
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37655/1/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/37655/1/src/security/vboot/secdata_... PS1, Line 72: VB2_SECDATA_FIRMWARE_SIZE));
code indent should use tabs where possible
Thanks.
https://review.coreboot.org/c/coreboot/+/37655/1/src/security/vboot/secdata_... PS1, Line 342: VB2_SECDATA_FIRMWARE_SIZE));
code indent should use tabs where possible
I wholeheartedly agree.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37655/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37655/3//COMMIT_MSG@10 PS3, Line 10: secdata_firmware secdata_kernel?
Hello Aaron Durbin, Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37655
to look at the new patch set (#4).
Change subject: vboot: update secdata naming scheme ......................................................................
vboot: update secdata naming scheme
secdata -> secdata_firmware secdatak -> secdata_kernel
BUG=b:124141368, chromium:972956 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ie2051de51c8f483a8921831385557fad816eb9fb Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/secdata_tpm.c 1 file changed, 23 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37655/4
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37655/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37655/3//COMMIT_MSG@10 PS3, Line 10: secdata_firmware
secdata_kernel?
Indeed!
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37655/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37655/3//COMMIT_MSG@10 PS3, Line 10: secdata_firmware
Indeed!
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37655 )
Change subject: vboot: update secdata naming scheme ......................................................................
vboot: update secdata naming scheme
secdata -> secdata_firmware secdatak -> secdata_kernel
BUG=b:124141368, chromium:972956 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ie2051de51c8f483a8921831385557fad816eb9fb Signed-off-by: Joel Kitching kitching@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37655 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/security/vboot/secdata_tpm.c 1 file changed, 23 insertions(+), 21 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index ef24555..0ce2136 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -67,10 +67,11 @@ int attempts = 3;
while (attempts--) { - RETURN_ON_FAILURE(tlcl_read(FIRMWARE_NV_INDEX, ctx->secdata, - VB2_SECDATA_SIZE)); + RETURN_ON_FAILURE(tlcl_read(FIRMWARE_NV_INDEX, + ctx->secdata_firmware, + VB2_SECDATA_FIRMWARE_SIZE));
- if (vb2api_secdata_check(ctx) == VB2_SUCCESS) + if (vb2api_secdata_firmware_check(ctx) == VB2_SUCCESS) return TPM_SUCCESS;
VBDEBUG("TPM: %s() - bad CRC\n", __func__); @@ -194,14 +195,14 @@ static uint32_t set_firmware_space(const void *firmware_blob) { return set_space("firmware", FIRMWARE_NV_INDEX, firmware_blob, - VB2_SECDATA_SIZE, ro_space_attributes, + VB2_SECDATA_FIRMWARE_SIZE, ro_space_attributes, pcr0_unchanged_policy, sizeof(pcr0_unchanged_policy)); }
static uint32_t set_kernel_space(const void *kernel_blob) { return set_space("kernel", KERNEL_NV_INDEX, kernel_blob, - VB2_SECDATAK_SIZE, rw_space_attributes, NULL, 0); + VB2_SECDATA_KERNEL_SIZE, rw_space_attributes, NULL, 0); }
static uint32_t set_rec_hash_space(const uint8_t *data) @@ -222,12 +223,12 @@ * indication that TPM factory initialization was successfully * completed. */ - RETURN_ON_FAILURE(set_kernel_space(ctx->secdatak)); + RETURN_ON_FAILURE(set_kernel_space(ctx->secdata_kernel));
if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) RETURN_ON_FAILURE(set_rec_hash_space(rec_hash_data));
- RETURN_ON_FAILURE(set_firmware_space(ctx->secdata)); + RETURN_ON_FAILURE(set_firmware_space(ctx->secdata_firmware));
return TPM_SUCCESS; } @@ -326,22 +327,22 @@ VBDEBUG("TPM: Clearing owner\n"); RETURN_ON_FAILURE(tpm_clear_and_reenable());
- /* Define and write secdatak kernel space. */ + /* Define and write secdata_kernel space. */ RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX, TPM_NV_PER_PPWRITE, - VB2_SECDATAK_SIZE)); + VB2_SECDATA_KERNEL_SIZE)); RETURN_ON_FAILURE(write_secdata(KERNEL_NV_INDEX, - ctx->secdatak, - VB2_SECDATAK_SIZE)); + ctx->secdata_kernel, + VB2_SECDATA_KERNEL_SIZE));
- /* Define and write secdata firmware space. */ + /* Define and write secdata_firmware space. */ RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX, - TPM_NV_PER_GLOBALLOCK | - TPM_NV_PER_PPWRITE, - VB2_SECDATA_SIZE)); + TPM_NV_PER_GLOBALLOCK | + TPM_NV_PER_PPWRITE, + VB2_SECDATA_FIRMWARE_SIZE)); RETURN_ON_FAILURE(write_secdata(FIRMWARE_NV_INDEX, - ctx->secdata, - VB2_SECDATA_SIZE)); + ctx->secdata_firmware, + VB2_SECDATA_FIRMWARE_SIZE));
/* Define and set rec hash space, if available. */ if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) @@ -376,9 +377,9 @@ { uint32_t result;
- /* Set initial values of secdata and secdatak spaces. */ - vb2api_secdata_create(ctx); - vb2api_secdatak_create(ctx); + /* Set initial values of secdata_firmware and secdata_kernel spaces. */ + vb2api_secdata_firmware_create(ctx); + vb2api_secdata_kernel_create(ctx);
VBDEBUG("TPM: factory initialization\n");
@@ -430,7 +431,8 @@ { if (CONFIG(CR50_IMMEDIATELY_COMMIT_FW_SECDATA)) tlcl_cr50_enable_nvcommits(); - return write_secdata(FIRMWARE_NV_INDEX, ctx->secdata, VB2_SECDATA_SIZE); + return write_secdata(FIRMWARE_NV_INDEX, ctx->secdata_firmware, + VB2_SECDATA_FIRMWARE_SIZE); }
uint32_t antirollback_read_space_rec_hash(uint8_t *data, uint32_t size)
-
build bot (Jenkins) (Code Review) -
Joel Kitching (Code Review)
-
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)