Change in coreboot[master]: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Arthur Heymans (Code Review)

7 Dec 2020 7 Dec '20

1:36 p.m.

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48407 )

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE

In both postcar and ramstage the console is initialized before cbmem. With USE_OPTION_TABLE the console loglevel is fetched from RTC nvram. The layout for the RTC nvram is fetched from cbfs on each read which won't work before CBFS is initialized.

Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/48407/1

diff --git a/src/Kconfig b/src/Kconfig index 77d077f..2111b79 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -123,6 +123,7 @@ config USE_OPTION_TABLE bool "Use CMOS for configuration values" depends on HAVE_OPTION_TABLE + select NO_CBFS_MCACHE help Enable this option if coreboot shall read options from the "CMOS" NVRAM instead of using hard-coded values.

Attachments:

attachment.htm (text/html — 2.5 KB)

Show replies by date

Arthur Heymans (Code Review)

7 Dec 7 Dec

1:38 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48407 )

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Patch Set 1:

I guess a better solution can be worked out (like setting a constant for the loglevel and have it updated with a cbmem_init hook), but let's do this for now?

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Comment-Date: Mon, 07 Dec 2020 13:38:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

1:45 p.m.

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

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/48407/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/48407/1//COMMIT_MSG@11 PS1, Line 11: cbfs nit: CBFS (for consistency)

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Comment-Date: Mon, 07 Dec 2020 13:45:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

3:43 p.m.

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

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48407/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/48407/1//COMMIT_MSG@13 PS1, Line 13: If you could add the Fixes: tag, that’d be great:

...

Fixes:

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 07 Dec 2020 15:43:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Julius Werner (Code Review)

8:45 p.m.

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

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Patch Set 1: Code-Review+2

Thanks for the fix!

I guess there are more CBFS-before-CBMEM issues than I thought. I think the best thing would probably be a general fix where the CBFS code doesn't try to access the mcache (and instead falls back to lookup on flash) when CBMEM is not available yet in these stages?

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 07 Dec 2020 20:45:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Julius Werner (Code Review)

11:05 p.m.

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

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Patch Set 1:

Here's my alternative suggestion for solving this problem: CB:48429

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 07 Dec 2020 23:05:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

8 Dec 8 Dec

6:26 a.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48407 )

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Patch Set 1:

...

Patch Set 1:

Here's my alternative suggestion for solving this problem: CB:48429

Looks good. That probably also fixes the issue with FSP-T and postcar.

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 08 Dec 2020 06:26:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

6:26 a.m.

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48407 )

Change subject: src/Kconfig: Select NO_CBFS_MCACHE if USE_OPTION_TABLE ......................................................................

Abandoned

-- To view, visit https://review.coreboot.org/c/coreboot/+/48407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5a2da8759323239adc758811cdbb4ba6e42fc9c Gerrit-Change-Number: 48407 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: abandon

1301

days inactive

1302

days old

coreboot-gerrit@coreboot.org

7 comments

4 participants

tags (0)

participants (4)

Angel Pons (Code Review)
Arthur Heymans (Code Review)
Julius Werner (Code Review)
Paul Menzel (Code Review)