Change in coreboot[master]: arch/x86/cbmem.c: Clean up unused paths

List overview All Threads
Download

newer

older

Change in coreboot[master]: Don't...

Change in coreboot[master]:...

Arthur Heymans (Code Review)

26 Oct 2019 26 Oct '19

12:28 p.m.

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

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

arch/x86/cbmem.c: Clean up unused paths

This code is not used in ramstage.

Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c 1 file changed, 1 insertion(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/36358/1

diff --git a/src/arch/x86/cbmem.c b/src/arch/x86/cbmem.c index 16a73c7..e055c18 100644 --- a/src/arch/x86/cbmem.c +++ b/src/arch/x86/cbmem.c @@ -18,19 +18,8 @@

void *cbmem_top_romstage(void) { - static void *cbmem_top_backup; - void *top_backup; - - if (ENV_RAMSTAGE && cbmem_top_backup != NULL) - return cbmem_top_backup; - /* Top of CBMEM is at highest usable DRAM address below 4GiB. */ - top_backup = (void *)restore_top_of_low_cacheable(); - - if (ENV_RAMSTAGE) - cbmem_top_backup = top_backup; - - return top_backup; + return (void *)restore_top_of_low_cacheable(); }

#endif /* CBMEM_TOP_BACKUP */

Attachments:

attachment.htm (text/html — 3.1 KB)

Show replies by date

Kyösti Mälkki (Code Review)

26 Oct 26 Oct

5:02 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36358 )

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c@22 PS1, Line 22: return (void *)restore_top_of_low_cacheable(); Assume this to be slow. It would be nice if the callee (cbmem_top()) would cache the value in romstage, even though we did not previously do it.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36358 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Gerrit-Change-Number: 36358 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Sat, 26 Oct 2019 15:02:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

5:04 p.m.

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

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c@22 PS1, Line 22: return (void *)restore_top_of_low_cacheable();

...

Assume this to be slow. It would be nice if the callee (cbmem_top()) would cache the value in romstage, even though we did not previously do it.

MAYBE_STATIC_BSS should cover the NO_CAR_GLOBAL_MIGRATION use case?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36358 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Gerrit-Change-Number: 36358 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Sat, 26 Oct 2019 15:04:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Arthur Heymans (Code Review)

5:36 p.m.

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

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c@22 PS1, Line 22: return (void *)restore_top_of_low_cacheable();

...

...
Assume this to be slow. It would be nice if the callee (cbmem_top()) would cache the value in romstage, even though we did not previously do it.

MAYBE_STATIC_BSS should cover the NO_CAR_GLOBAL_MIGRATION use case?

done in CB:36144

-- To view, visit https://review.coreboot.org/c/coreboot/+/36358 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Gerrit-Change-Number: 36358 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Sat, 26 Oct 2019 15:36:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

5:41 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36358 )

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:

https://review.coreboot.org/c/coreboot/+/36358/1/src/arch/x86/cbmem.c@22 PS1, Line 22: return (void *)restore_top_of_low_cacheable();

...

...
...
Assume this to be slow. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/36358 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Gerrit-Change-Number: 36358 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Oct 2019 15:41:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Michael Niewöhner (Code Review)

7:59 p.m.

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36358 )

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

Patch Set 4: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/36358 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Gerrit-Change-Number: 36358 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Oct 2019 17:59:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans (Code Review)

30 Oct 30 Oct

12:02 a.m.

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

Change subject: arch/x86/cbmem.c: Clean up unused paths ......................................................................

Abandoned

squashed in CB:36144

-- To view, visit https://review.coreboot.org/c/coreboot/+/36358 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I023f11cfcefacdcae3c5db22bf98b40a3575cc9c Gerrit-Change-Number: 36358 Gerrit-PatchSet: 9 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: abandon

1842

days inactive

1845

days old

coreboot-gerrit@coreboot.org

6 comments

3 participants

tags (0)

participants (3)

Arthur Heymans (Code Review)
Kyösti Mälkki (Code Review)
Michael Niewöhner (Code Review)