Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version for cbmem to display

List overview All Threads
Download

newer

older

Change in coreboot[4.11_branch]:...

Change in coreboot[master]:...

Johnny Lin (Code Review)

28 May 2020 28 May '20

6:24 a.m.

Johnny Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

lib/coreboot_table: Add Intel FSP version for cbmem to display

Add Intel FSP version to coreboot table LB_TAG_EXTRA_VERSION when FSP_ADD_VERSION_TO_CBMEM is selected.

Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/lib/coreboot_table.c 1 file changed, 23 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/1

diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 075bd04..51c1cc3 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -31,6 +31,9 @@ #include <vendorcode/google/chromeos/chromeos.h> #include <vendorcode/google/chromeos/gnvs.h> #endif +#if CONFIG(FSP_ADD_VERSION_TO_CBMEM) +#include <fsp/util.h> +#endif

static struct lb_header *lb_table_init(unsigned long addr) { @@ -408,6 +411,23 @@

}

+#if CONFIG(FSP_ADD_VERSION_TO_CBMEM) +/* Add FSP version to coreboot table LB_TAG_EXTRA_VERSION */ +static void lb_strings_fsp_version(struct lb_header *header) +{ + struct lb_string *rec; + size_t len; + char fsp_version[FSP_VER_LEN] = {0}; + + fsp_get_version(fsp_version); + rec = (struct lb_string *)lb_new_record(header); + rec->tag = LB_TAG_EXTRA_VERSION; + len = strlen(fsp_version); + rec->size = ALIGN_UP(sizeof(*rec) + len + 1, 8); + memcpy(rec->string, fsp_version, len+1); +} +#endif + static void lb_record_version_timestamp(struct lb_header *header) { struct lb_timestamp *rec; @@ -516,6 +536,9 @@

/* Record our various random string information */ lb_strings(head); +#if CONFIG(FSP_ADD_VERSION_TO_CBMEM) + lb_strings_fsp_version(head); +#endif lb_record_version_timestamp(head); /* Record our framebuffer */ lb_framebuffer(head);

Attachments:

attachment.htm (text/html — 4.6 KB)

Show replies by date

Johnny Lin (Code Review)

28 May 28 May

6:30 a.m.

Johnny Lin has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

lib/coreboot_table: Add Intel FSP version for cbmem to display

Add Intel FSP version to coreboot table LB_TAG_EXTRA_VERSION when FSP_ADD_VERSION_TO_CBMEM is selected. It relies on change [1] to copy the FSP version.

[1] https://review.coreboot.org/c/coreboot/+/41760

Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/lib/coreboot_table.c 1 file changed, 23 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/2

Nico Huber (Code Review)

10:24 a.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

Patch Set 2:

(1 comment)

A solution without #if would be much preferred. All you need is an always available fsp_get_version() forward declaration. Then the call to lb_strings_fsp_version() can be guarded by a C `if`:

if (CONFIG(FSP_ADD_VERSION_TO_CBMEM)) lb_strings_fsp_version(head);

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

https://review.coreboot.org/c/coreboot/+/41809/2//COMMIT_MSG@9 PS2, Line 9: LB_TAG_EXTRA_VERSION Please add a new tag. Existing software can be confused if this tag shows up multiple times in the table.

Johnny Lin (Code Review)

29 May 29 May

1:50 a.m.

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

lib/coreboot_table: Add Intel FSP version for cbmem to display

Add a new LB_TAG_FSP_VERSION for FSP version, it would add Intel FSP version to coreboot table LB_TAG_FSP_VERSION when FSP_ADD_VERSION_TO_CBMEM is selected. It relies on change [1] to copy the FSP version.

[1] https://review.coreboot.org/c/coreboot/+/41760

Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/commonlib/include/commonlib/coreboot_tables.h M src/lib/coreboot_table.c 2 files changed, 19 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/3

Johnny Lin (Code Review)

1:54 a.m.

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

Patch Set 3:

(1 comment)

...

Patch Set 2:

(1 comment)

A solution without #if would be much preferred. All you need is an always available fsp_get_version() forward declaration. Then the call to lb_strings_fsp_version() can be guarded by a C `if`:
if (CONFIG(FSP_ADD_VERSION_TO_CBMEM))
        lb_strings_fsp_version(head);

Done, thanks.

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

https://review.coreboot.org/c/coreboot/+/41809/2//COMMIT_MSG@9 PS2, Line 9: LB_TAG_EXTRA_VERSION

...

Please add a new tag. Existing software can be confused if this […]

Done

Johnny Lin (Code Review)

5:45 a.m.

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

lib/coreboot_table: Add Intel FSP version for cbmem to display

[1] https://review.coreboot.org/c/coreboot/+/41760

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/5

Johnny Lin (Code Review)

6:38 a.m.

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

Patch Set 5:

...

Patch Set 3:

(1 comment)

...
Patch Set 2:

(1 comment)

A solution without #if would be much preferred. All you need is an always available fsp_get_version() forward declaration. Then the call to lb_strings_fsp_version() can be guarded by a C `if`:
if (CONFIG(FSP_ADD_VERSION_TO_CBMEM))
        lb_strings_fsp_version(head);
Done, thanks.

I rollback to use #if to avoid re-defining FSP_VER_LEN here again.

Johnny Lin (Code Review)

11 Jun 11 Jun

7:30 a.m.

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

lib/coreboot_table: Add Intel FSP version for cbmem to display

[1] https://review.coreboot.org/c/coreboot/+/41760

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/8

Paul Menzel (Code Review)

15 Jun 15 Jun

8:51 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version for cbmem to display ......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/41809/9/src/lib/coreboot_table.c@53... PS9, Line 538: #if CONFIG(FSP_ADD_VERSION_TO_CBMEM) Please do the check in C and not the preprocessor.

Johnny Lin (Code Review)

16 Jun 16 Jun

5:40 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

lib/coreboot_table: Add Intel FSP version to coreboot table

Add a new LB_TAG_FSP_VERSION for FSP version, it would add Intel FSP version to coreboot table LB_TAG_FSP_VERSION when PLATFORM_USES_FSP2_0 is selected. It relies on change [1] to copy the FSP version.

[1] https://review.coreboot.org/c/coreboot/+/41760

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/10

Johnny Lin (Code Review)

5:48 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/41809/9/src/lib/coreboot_table.c@53... PS9, Line 538: #if CONFIG(FSP_ADD_VERSION_TO_CBMEM)

...

Please do the check in C and not the preprocessor.

Done

Johnny Lin (Code Review)

6:49 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

lib/coreboot_table: Add Intel FSP version to coreboot table

[1] https://review.coreboot.org/c/coreboot/+/41760

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/11

Johnny Lin (Code Review)

8:06 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 12:

This change and CB:41760 depend on each other, need to combine them into one change to avoid the UNSTABLE build error, not sure if it's necessary.

Paul Menzel (Code Review)

9:02 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 12:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/12/src/lib/coreboot_table.c File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/41809/12/src/lib/coreboot_table.c@3... PS12, Line 37: #endif Excuse my ignorance, but why not unconditionally include `fsp/util.h`? It looks like just for consistency with the current style in the file, but I believe we stopped guarding header inclusion.

Johnny Lin (Code Review)

12:59 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

lib/coreboot_table: Add Intel FSP version to coreboot table

[1] https://review.coreboot.org/c/coreboot/+/41760

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/13

Johnny Lin (Code Review)

1 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/12/src/lib/coreboot_table.c File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/41809/12/src/lib/coreboot_table.c@3... PS12, Line 37: #endif

...

Excuse my ignorance, but why not unconditionally include `fsp/util. […]

Saw several build errors, the fsp path is not included for those platforms, maybe there's a better way to fix it. https://qa.coreboot.org/job/coreboot-gerrit/130774/ src/lib/coreboot_table.c:33:10: fatal error: fsp/util.h: No such file or directory #include <fsp/util.h>

Johnny Lin (Code Review)

18 Jun 18 Jun

1:48 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/12/src/lib/coreboot_table.c File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/41809/12/src/lib/coreboot_table.c@3... PS12, Line 37: #endif

...

Saw several build errors, the fsp path is not included for those platforms, maybe there's a better w […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 13 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Thu, 18 Jun 2020 01:48:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Johnny Lin Johnny_Lin@wiwynn.com Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Patrick Georgi (Code Review)

30 Jun 30 Jun

5:53 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 14:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/14//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/41809/14//COMMIT_MSG@12 PS14, Line 12: It relies on change [1] to copy the FSP version. you will have to merge the two commits

Johnny Lin (Code Review)

6:32 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Hello build bot (Jenkins), Jonathan Zhang, Paul Menzel, Jingle Hsu, Morgan Jang,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

lib/coreboot_table: Add Intel FSP version to coreboot table

Add a new LB_TAG_FSP_VERSION for FSP version, it would add Intel FSP version to coreboot table LB_TAG_FSP_VERSION when PLATFORM_USES_FSP2_0 is selected.

Tested=On OCP Delta Lake, with an updated LinuxBoot payload cbmem utility can see "LB_TAG_FSP_VERSION": "2.1-0.0.1.120"

Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/commonlib/include/commonlib/coreboot_tables.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/util.c M src/lib/coreboot_table.c 4 files changed, 48 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/15

Johnny Lin (Code Review)

6:35 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 16:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/14//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/41809/14//COMMIT_MSG@12 PS14, Line 12: It relies on change [1] to copy the FSP version.

...

you will have to merge the two commits

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 16 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Tue, 30 Jun 2020 06:35:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Jonathan Zhang (Code Review)

5:08 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 16: Code-Review+1

LGTM

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 16 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Tue, 30 Jun 2020 17:08:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Subrata Banik (Code Review)

5:14 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 16: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 16 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Tue, 30 Jun 2020 17:14:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

7:06 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 16:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/16/src/commonlib/include/comm... File src/commonlib/include/commonlib/coreboot_tables.h:

https://review.coreboot.org/c/coreboot/+/41809/16/src/commonlib/include/comm... PS16, Line 82: LB_TAG_CMOS_OPTION_TABLE = 0x00c8, : LB_TAG_OPTION = 0x00c9, : LB_TAG_OPTION_ENUM = 0x00ca, : LB_TAG_OPTION_DEFAULTS = 0x00cb, : LB_TAG_OPTION_CHECKSUM = 0x00cc, These are actually a group of tags starting from 200 (somehow this information got lost during refactoring). The new entry should be added above as 0x38.

And I would still prefer to use a vendor-neutral tag name, e.g. `LB_TAG_PLATFORM_BLOB_VERSION`.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 16 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Tue, 30 Jun 2020 19:06:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Johnny Lin (Code Review)

1 Jul 1 Jul

1:57 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Christian Walter, Paul Menzel, Jingle Hsu, Subrata Banik, Morgan Jang, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

lib/coreboot_table: Add Intel FSP version to coreboot table

Add a new LB_TAG_PLATFORM_BLOB_VERSION for FSP version, it would add Intel FSP version to coreboot table LB_TAG_PLATFORM_BLOB_VERSION when PLATFORM_USES_FSP2_0 is selected.

Tested=On OCP Delta Lake, with an updated LinuxBoot payload cbmem utility can see "LB_TAG_PLATFORM_BLOB_VERSION": "2.1-0.0.1.120"

Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/commonlib/include/commonlib/coreboot_tables.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/util.c M src/lib/coreboot_table.c 4 files changed, 48 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/41809/17

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 17 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

Johnny Lin (Code Review)

3:35 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 17:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41809/16/src/commonlib/include/comm... File src/commonlib/include/commonlib/coreboot_tables.h:

...

These are actually a group of tags starting from 200 (somehow this […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 17 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Wed, 01 Jul 2020 03:35:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Nico Huber (Code Review)

11:54 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 17: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 17 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Wed, 01 Jul 2020 11:54:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Christian Walter (Code Review)

12:48 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 17: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 17 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Wed, 01 Jul 2020 12:48:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Philipp Deppenwiese (Code Review)

2 Jul 2 Jul

8:53 p.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

Patch Set 17: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 17 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Thu, 02 Jul 2020 20:53:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Philipp Deppenwiese (Code Review)

4 Jul 4 Jul

11:20 a.m.

New subject: Change in coreboot[master]: lib/coreboot_table: Add Intel FSP version to coreboot table

Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41809 )

Change subject: lib/coreboot_table: Add Intel FSP version to coreboot table ......................................................................

lib/coreboot_table: Add Intel FSP version to coreboot table

Add a new LB_TAG_PLATFORM_BLOB_VERSION for FSP version, it would add Intel FSP version to coreboot table LB_TAG_PLATFORM_BLOB_VERSION when PLATFORM_USES_FSP2_0 is selected.

Tested=On OCP Delta Lake, with an updated LinuxBoot payload cbmem utility can see "LB_TAG_PLATFORM_BLOB_VERSION": "2.1-0.0.1.120"

Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41809 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Christian Walter christian.walter@9elements.com Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com --- M src/commonlib/include/commonlib/coreboot_tables.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/util.c M src/lib/coreboot_table.c 4 files changed, 48 insertions(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Philipp Deppenwiese: Looks good to me, approved Christian Walter: Looks good to me, but someone else must approve

diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h index 3627996..b7e59ad 100644 --- a/src/commonlib/include/commonlib/coreboot_tables.h +++ b/src/commonlib/include/commonlib/coreboot_tables.h @@ -79,6 +79,7 @@ LB_TAG_MMC_INFO = 0x0035, LB_TAG_TCPA_LOG = 0x0036, LB_TAG_FMAP = 0x0037, + LB_TAG_PLATFORM_BLOB_VERSION = 0x0038, LB_TAG_CMOS_OPTION_TABLE = 0x00c8, LB_TAG_OPTION = 0x00c9, LB_TAG_OPTION_ENUM = 0x00ca, diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h index ad6a4b5..f154a34 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/util.h +++ b/src/drivers/intel/fsp2_0/include/fsp/util.h @@ -12,6 +12,8 @@ #include <program_loading.h> #include <types.h>

+#define FSP_VER_LEN 30 + struct hob_header { uint16_t type; uint16_t length; @@ -85,7 +87,8 @@ const struct hob_header *fsp_next_hob(const struct hob_header *parent); bool fsp_guid_compare(const uint8_t guid1[16], const uint8_t guid2[16]); void fsp_find_bootloader_tolum(struct range_entry *re); - +void fsp_get_version(char *buf); +void lb_string_platform_blob_version(struct lb_header *header);

/* Fill in header and validate sanity of component within region device. */ enum cb_err fsp_validate_component(struct fsp_header *hdr, diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index e8c5876..cfa83d7 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */

+#include <boot/coreboot_tables.h> #include <device/mmio.h> #include <cbfs.h> #include <cf9_reset.h> @@ -208,3 +209,38 @@

return CB_SUCCESS; } + +/* Only call this function when FSP header has been read and validated */ +void fsp_get_version(char *buf) +{ + struct fsp_header *hdr = &fsps_hdr; + union { + uint32_t val; + struct { + uint8_t bld_num; + uint8_t revision; + uint8_t minor; + uint8_t major; + } rev; + } revision; + + revision.val = hdr->fsp_revision; + snprintf(buf, FSP_VER_LEN, "%u.%u-%u.%u.%u.%u", (hdr->spec_version >> 4), + hdr->spec_version & 0xf, revision.rev.major, + revision.rev.minor, revision.rev.revision, revision.rev.bld_num); +} + +/* Add FSP version to coreboot table LB_TAG_PLATFORM_BLOB_VERSION */ +void lb_string_platform_blob_version(struct lb_header *header) +{ + struct lb_string *rec; + size_t len; + char fsp_version[FSP_VER_LEN] = {0}; + + fsp_get_version(fsp_version); + rec = (struct lb_string *)lb_new_record(header); + rec->tag = LB_TAG_PLATFORM_BLOB_VERSION; + len = strlen(fsp_version); + rec->size = ALIGN_UP(sizeof(*rec) + len + 1, 8); + memcpy(rec->string, fsp_version, len+1); +} diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 12282fc..9148405 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -30,6 +30,11 @@ #include <vendorcode/google/chromeos/chromeos.h> #include <vendorcode/google/chromeos/gnvs.h> #endif +#if CONFIG(PLATFORM_USES_FSP2_0) +#include <fsp/util.h> +#else +void lb_string_platform_blob_version(struct lb_header *header); +#endif

static struct lb_header *lb_table_init(unsigned long addr) { @@ -515,6 +520,8 @@

/* Record our various random string information */ lb_strings(head); + if (CONFIG(PLATFORM_USES_FSP2_0)) + lb_string_platform_blob_version(head); lb_record_version_timestamp(head); /* Record our framebuffer */ lb_framebuffer(head);

-- To view, visit https://review.coreboot.org/c/coreboot/+/41809 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92a13ca91b9f66a7517cfd6784f3f692ff34e765 Gerrit-Change-Number: 41809 Gerrit-PatchSet: 18 Gerrit-Owner: Johnny Lin Johnny_Lin@wiwynn.com Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Jingle Hsu jingle_hsu@wiwynn.com Gerrit-Reviewer: Jonathan Zhang jonzhang@fb.com Gerrit-Reviewer: Morgan Jang Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: merged

1550

days inactive

1587

days old

coreboot-gerrit@coreboot.org

28 comments

8 participants

tags (0)

participants (8)

Christian Walter (Code Review)
Johnny Lin (Code Review)
Jonathan Zhang (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)
Philipp Deppenwiese (Code Review)
Subrata Banik (Code Review)