Change in ...coreboot[master]: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

Joel Kitching (Code Review)

6 Apr 2019 6 Apr '19

5:03 p.m.

Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32234

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag

The value of "write protect" GPIO shall be read in depthcharge, and the flag shall be set there instead.

BUG=b:124141368, b:124192753, chromium:1556855 TEST=Build locally CQ-DEPEND=CL:1556855 BRANCH=none

Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/vboot_handoff.c 1 file changed, 0 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/32234/1

diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c index 2f239e6..047fcaf 100644 --- a/src/security/vboot/vboot_handoff.c +++ b/src/security/vboot/vboot_handoff.c @@ -58,9 +58,6 @@ vb_sd->data_used = sizeof(VbSharedDataHeader); vb_sd->fw_version_tpm = vb2_sd->fw_version_secdata;

- if (get_write_protect_state()) - vb_sd->flags |= VBSD_BOOT_FIRMWARE_WP_ENABLED; - if (vb2_sd->recovery_reason) { vb_sd->firmware_index = 0xFF; if (vb2_sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY)

Attachments:

attachment.htm (text/html — 2.8 KB)

Show replies by date

Furquan Shaikh (Code Review)

7 Apr 7 Apr

4:43 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32234 )

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

Patch Set 1: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/32234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Gerrit-Change-Number: 32234 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching kitching@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Simon Glass sjg@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 07 Apr 2019 16:43:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Julius Werner (Code Review)

8 Apr 8 Apr

10:59 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32234 )

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

Patch Set 1: Code-Review-1

Did you really double-check for *all* Chrome OS boards that the "write protect" lb_gpio is defined and initialized to represent the same (when combined with polarity) as what get_write_protect_state() returns? I wouldn't be so sure that's always the case, errors can easily creep in. (For example, in cheza/chromeos.c I think the polarity got messed up.)

-- To view, visit https://review.coreboot.org/c/coreboot/+/32234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Gerrit-Change-Number: 32234 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching kitching@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Simon Glass sjg@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 08 Apr 2019 22:59:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Julius Werner (Code Review)

11:02 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32234 )

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

Patch Set 1: -Code-Review

Sorry, I'm reading patches out of order. Looks like you already noticed the issue. Looking at that patch now...

-- To view, visit https://review.coreboot.org/c/coreboot/+/32234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Gerrit-Change-Number: 32234 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching kitching@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Simon Glass sjg@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 08 Apr 2019 23:02:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Julius Werner (Code Review)

11:02 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32234 )

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

Patch Set 1: Code-Review+2

(This LGTM on its own as long as the previous patch does the right thing.)

-- To view, visit https://review.coreboot.org/c/coreboot/+/32234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Gerrit-Change-Number: 32234 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching kitching@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Simon Glass sjg@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 08 Apr 2019 23:02:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Joel Kitching (Code Review)

9 Apr 9 Apr

11:06 a.m.

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32234 )

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

Patch Set 1:

...

Patch Set 1: Code-Review+2

(This LGTM on its own as long as the previous patch does the right thing.)

Right, thanks =)

-- To view, visit https://review.coreboot.org/c/coreboot/+/32234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Gerrit-Change-Number: 32234 Gerrit-PatchSet: 1 Gerrit-Owner: Joel Kitching kitching@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Simon Glass sjg@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 09 Apr 2019 11:06:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Georgi (Code Review)

11 Apr 11 Apr

11:23 a.m.

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32234 )

Change subject: vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag ......................................................................

vboot: do not set VBSD_BOOT_FIRMWARE_WP_ENABLED flag

The value of "write protect" GPIO shall be read in depthcharge, and the flag shall be set there instead.

BUG=b:124141368, b:124192753, chromium:1556855 TEST=Build locally CQ-DEPEND=CL:1556855 BRANCH=none

Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Signed-off-by: Joel Kitching kitching@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32234 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Julius Werner jwerner@chromium.org --- M src/security/vboot/vboot_handoff.c 1 file changed, 0 insertions(+), 3 deletions(-)

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Julius Werner: Looks good to me, approved

- if (get_write_protect_state()) - vb_sd->flags |= VBSD_BOOT_FIRMWARE_WP_ENABLED; - if (vb2_sd->recovery_reason) { vb_sd->firmware_index = 0xFF; if (vb2_sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY)

-- To view, visit https://review.coreboot.org/c/coreboot/+/32234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d24a057b1385244a836a67c565ee6726a894fdc Gerrit-Change-Number: 32234 Gerrit-PatchSet: 2 Gerrit-Owner: Joel Kitching kitching@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Simon Glass sjg@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1998

days inactive

2003

days old

coreboot-gerrit@coreboot.org

6 comments

4 participants

tags (0)

participants (4)

Furquan Shaikh (Code Review)
Joel Kitching (Code Review)
Julius Werner (Code Review)
Patrick Georgi (Code Review)