Change in coreboot[master]: security/vboot: Make vboot library is accessible to ramstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Make vboot library is accessible to ramstage
......................................................................

security/vboot: Make vboot library is accessible to ramstage

Add changes to Makefile to make vboot helper functions accessible
to ramstage. These functions help CSE firmware update module to
trigger recovery mode for runtime errors.

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
1 file changed, 2 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/1

diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc
index d1cc2da..d7c792a 100644
--- a/src/security/vboot/Makefile.inc
+++ b/src/security/vboot/Makefile.inc
@@ -115,6 +115,7 @@
 else
 verstage-y += secdata_tpm.c
 romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_tpm.c
+ramstage-y += secdata_tpm.c
 endif

ifneq ($(CONFIG_TPM1)$(CONFIG_TPM2),)
@@ -126,6 +127,7 @@

ramstage-y += common.c
 postcar-y += common.c
+ramstage-y += vboot_logic.c

romstage-$(CONFIG_FSP2_0_USES_TPM_MRC_HASH) += mrc_cache_hash_tpm.c

[Julius Werner (Code Review)]

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

Change subject: security/vboot: Make vboot library is accessible to ramstage
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/2/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/2/src/security/vboot/Makefile... 
PS2, Line 130: ramstage-y += vboot_logic.c
vboot_logic.c was supposed to be the file where we only have the code that is really needed at verification time, and all random support routines that might be needed at other times as well were supposed to go elsewhere. I guess we already failed that by adding it to romstage, but would you mind fixing it now? I think you just need to move vboot_save_data() and vboot_save_nvdata_only() from vboot_logic.c into vboot_common.c (and then you should be able to remove the romstage-y += vboot_logic.c as well).

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Make vboot library is accessible to ramstage
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40562/2//COMMIT_MSG@7 
PS2, Line 7: is
drop the "is"

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#3).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage. So, below
support routines are moved from vboot_logic.c to vboot_common.c.
1. vboot_save_data()
2. vboot_save_data()

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 24 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/3

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40562/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40562/2//COMMIT_MSG@7 
PS2, Line 7: is
> drop the "is"

Done

https://review.coreboot.org/c/coreboot/+/40562/2/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/2/src/security/vboot/Makefile... 
PS2, Line 130: ramstage-y += vboot_logic.c
> vboot_logic. […]

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40562/3//COMMIT_MSG@12 
PS3, Line 12: 2. vboot_save_data()
You list the same function twice. One of these should be vboot_save_nvdata_only() ?

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#4).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage. So, below
support routines are moved from vboot_logic.c to vboot_common.c.
1. vboot_save_data()
2. vboot_save_data_only()

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 24 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/4

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40562/3//COMMIT_MSG@12 
PS3, Line 12: 2. vboot_save_data()
> You list the same function twice. […]

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#5).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage. So, below
support routines are moved from vboot_logic.c to vboot_common.c.
1. vboot_save_nvdata()
2. vboot_save_nvdata_only()

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 24 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/5

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#6).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage. So, below
support routines are moved from vboot_logic.c to vboot_common.c.
1. vboot_save_data()
2. vboot_save_nvdata_only()

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 24 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/6

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Julius Werner (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 6:

Your base commit is really old... this needs a rebase onto CB:40389. Otherwise LGTM.

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Joel Kitching (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 6:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40562/6/src/security/vboot/vboot_co... 
File src/security/vboot/vboot_common.c:

https://review.coreboot.org/c/coreboot/+/40562/6/src/security/vboot/vboot_co... 
PS6, Line 14: #
Blank line in between <> and "" includes.

https://review.coreboot.org/c/coreboot/+/40562/6/src/security/vboot/vboot_co... 
PS6, Line 30: 				
Seems like the indentation got changed here?

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#7).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 24 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/7

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#8).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 26 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/8

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 8:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40562/6/src/security/vboot/vboot_co... 
File src/security/vboot/vboot_common.c:

https://review.coreboot.org/c/coreboot/+/40562/6/src/security/vboot/vboot_co... 
PS6, Line 14: #
> Blank line in between <> and "" includes.

Done

https://review.coreboot.org/c/coreboot/+/40562/6/src/security/vboot/vboot_co... 
PS6, Line 30:
> Seems like the indentation got changed here?

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Julius Werner (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
PS8, Line 118: $(CONFIG_VBOOT_SEPARATE_VERSTAGE)
This should be 'y' (actually, for 'romstage' it should also be 'y').

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#9).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 26 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/9

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
PS8, Line 118: $(CONFIG_VBOOT_SEPARATE_VERSTAGE)
> This should be 'y' (actually, for 'romstage' it should also be 'y').

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Julius Werner (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/9/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/9/src/security/vboot/Makefile... 
PS9, Line 114: romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_mock.c
What about secdata_mock.c here?

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#10).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 27 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/10

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/9/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/9/src/security/vboot/Makefile... 
PS9, Line 114: romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_mock.c
> What about secdata_mock. […]

Valid point. I did see the issue on RVP, but forgot to update here. Thanks.

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/9/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/9/src/security/vboot/Makefile... 
PS9, Line 114: romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_mock.c
> Valid point. I did see the issue on RVP, but forgot to update here. Thanks.

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Julius Werner (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 10: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
PS8, Line 118: $(CONFIG_VBOOT_SEPARATE_VERSTAGE)
> Done

Note I just fixed this for romstage (CB:40760) so you probably need to rebase.

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Julius Werner, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#11).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 27 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/11

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
File src/security/vboot/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/40562/8/src/security/vboot/Makefile... 
PS8, Line 118: $(CONFIG_VBOOT_SEPARATE_VERSTAGE)
> Note I just fixed this for romstage (CB:40760) so you probably need to rebase.

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Furquan Shaikh (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 11: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/11//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40562/11//COMMIT_MSG@15 
PS11, Line 15: 
BUG=?

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth, Sridhar Siricilla, Rizwan Qureshi, Julius Werner, Angel Pons, Subrata Banik, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40562

to look at the new patch set (#12).

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

BUG=b:155544643
TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 27 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/40562/12

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 12:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40562/11//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40562/11//COMMIT_MSG@15 
PS11, Line 15:
> BUG=?

Done

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Furquan Shaikh (Code Review)]

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

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 12: Code-Review+2

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Rizwan Qureshi (Code Review)]

Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

Patch Set 12: Code-Review+2

Change in coreboot[master]: security/vboot: Limit vboot verification code access to only verstage

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40562 )

Change subject: security/vboot: Limit vboot verification code access to only verstage
......................................................................

security/vboot: Limit vboot verification code access to only verstage

Make vboot verification code accessible in only verstage.
Vboot verification code in vboot_logic.c is being used
in verstage. Due to support function vboot_save_data(),
so core functionality in vboot_logic.c is made available in romstage.
The patch decouples the support function frm vboot_logic.c to
limit itself to verstage.

BUG=b:155544643
TEST=Verified on hatch

Signed-off-by: Sridhar Siricilla 
Change-Id: Id1ede45c4dffe90afcef210eabaa657cf92a9335
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40562
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Rizwan Qureshi 
---
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_common.c
M src/security/vboot/vboot_logic.c
3 files changed, 27 insertions(+), 23 deletions(-)

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

diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc
index 67ee0f5..b452e93 100644
--- a/src/security/vboot/Makefile.inc
+++ b/src/security/vboot/Makefile.inc
@@ -112,16 +112,17 @@
 ifeq (${CONFIG_VBOOT_MOCK_SECDATA},y)
 verstage-y += secdata_mock.c
 romstage-y += secdata_mock.c
+ramstage-y += secdata_mock.c
 else
 verstage-y += secdata_tpm.c
 romstage-y += secdata_tpm.c
+ramstage-y += secdata_tpm.c
 endif

ifneq ($(CONFIG_TPM1)$(CONFIG_TPM2),)
 verstage-y += tpm_common.c
 endif

-romstage-y += vboot_logic.c
 romstage-y += common.c

ramstage-y += common.c
diff --git a/src/security/vboot/vboot_common.c b/src/security/vboot/vboot_common.c
index 36cd1ad..049b4a9 100644
--- a/src/security/vboot/vboot_common.c
+++ b/src/security/vboot/vboot_common.c
@@ -12,6 +12,31 @@
 #include <security/vboot/vbnv.h>
 #include <vb2_api.h>

+#include "antirollback.h"
+
+void vboot_save_data(struct vb2_context *ctx)
+{
+	if (ctx->flags & VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED &&
+			(CONFIG(VBOOT_MOCK_SECDATA) || tlcl_lib_init() == VB2_SUCCESS)) {
+		printk(BIOS_INFO, "Saving secdata firmware\n");
+		antirollback_write_space_firmware(ctx);
+		ctx->flags &= ~VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED;
+	}
+
+	if (ctx->flags & VB2_CONTEXT_SECDATA_KERNEL_CHANGED &&
+			(CONFIG(VBOOT_MOCK_SECDATA) || tlcl_lib_init() == VB2_SUCCESS)) {
+		printk(BIOS_INFO, "Saving secdata kernel\n");
+		antirollback_write_space_kernel(ctx);
+		ctx->flags &= ~VB2_CONTEXT_SECDATA_KERNEL_CHANGED;
+	}
+
+	if (ctx->flags & VB2_CONTEXT_NVDATA_CHANGED) {
+		printk(BIOS_INFO, "Saving nvdata\n");
+		save_vbnv(ctx->nvdata);
+		ctx->flags &= ~VB2_CONTEXT_NVDATA_CHANGED;
+	}
+}
+
 /* Check if it is okay to enable USB Device Controller (UDC). */
 int vboot_can_enable_udc(void)
 {
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index e1c77b6..a8a7be5 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -209,28 +209,6 @@
    return VB2_SUCCESS;
 }

-void vboot_save_data(struct vb2_context *ctx)
-{
-	if (ctx->flags & VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED &&
-	    (CONFIG(VBOOT_MOCK_SECDATA) || tlcl_lib_init() == VB2_SUCCESS)) {
-		printk(BIOS_INFO, "Saving secdata firmware\n");
-		antirollback_write_space_firmware(ctx);
-		ctx->flags &= ~VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED;
-	}
-
-	if (ctx->flags & VB2_CONTEXT_SECDATA_KERNEL_CHANGED &&
-	    (CONFIG(VBOOT_MOCK_SECDATA) || tlcl_lib_init() == VB2_SUCCESS)) {
-		printk(BIOS_INFO, "Saving secdata kernel\n");
-		antirollback_write_space_kernel(ctx);
-		ctx->flags &= ~VB2_CONTEXT_SECDATA_KERNEL_CHANGED;
-	}
-
-	if (ctx->flags & VB2_CONTEXT_NVDATA_CHANGED) {
-		printk(BIOS_INFO, "Saving nvdata\n");
-		save_vbnv(ctx->nvdata);
-		ctx->flags &= ~VB2_CONTEXT_NVDATA_CHANGED;
-	}
-}

static uint32_t extend_pcrs(struct vb2_context *ctx)
 {