Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/1
diff --git a/src/include/cbmem.h b/src/include/cbmem.h index f972ba6..a6cd277 100644 --- a/src/include/cbmem.h +++ b/src/include/cbmem.h @@ -74,7 +74,9 @@ * upper limit. * x86 boards or chipsets must return NULL before the cbmem backing store has * been initialized. */ +extern uintptr_t _cbmem_top_ptr; void *cbmem_top(void); +void *cbmem_top_romstage(void);
/* Add a cbmem entry of a given size and id. These return NULL on failure. The * add function performs a find first and do not check against the original diff --git a/src/lib/Kconfig b/src/lib/Kconfig index cb1e4a5..aede4b3 100644 --- a/src/lib/Kconfig +++ b/src/lib/Kconfig @@ -24,6 +24,12 @@ help Selected by features that require `libhwbase` in ramstage.
+config RAMSTAGE_CBMEM_TOP_ARG + bool + help + Select this if stages run after romstage get the cbmem_top + pointer the function arguments when called from romstage. + config FLATTENED_DEVICE_TREE bool help diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index e5678ff..0c569c5 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -149,6 +149,7 @@ ramstage-$(CONFIG_PAYLOAD_FIT_SUPPORT) += fit.c ramstage-$(CONFIG_PAYLOAD_FIT_SUPPORT) += fit_payload.c
+romstage-$(CONFIG_RAMSTAGE_CBMEM_TOP_ARG) += cbmem_top.c romstage-y += cbmem_common.c romstage-y += imd_cbmem.c romstage-y += imd.c diff --git a/src/lib/cbmem_top.c b/src/lib/cbmem_top.c new file mode 100644 index 0000000..6eee0d5 --- /dev/null +++ b/src/lib/cbmem_top.c @@ -0,0 +1,29 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <cbmem.h> + +#if !ENV_ROMSTAGE +void *cbmem_top(void) +{ + return (void *)_cbmem_top_ptr; +} +#else +void *cbmem_top(void) +{ + /* TODO use a static variable once NO_CAR_GLOBAL_MIGRATION + is implemented */ + return cbmem_top_romstage(); +} + +#endif
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h@77 PS1, Line 77: uintptr_t uintptr_t may not match between stages unless we're asserting that's always the case.
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h@79 PS1, Line 79: void *cbmem_top_romstage(void); You should add some commentary on these new functions/objects and their semantics.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h@77 PS1, Line 77: uintptr_t
uintptr_t may not match between stages unless we're asserting that's always the case.
Ok I'll add checks. I think it is a reasonable thing to expect?
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h@79 PS1, Line 79: void *cbmem_top_romstage(void);
You should add some commentary on these new functions/objects and their semantics.
ok
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#2).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 42 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h@77 PS1, Line 77: uintptr_t
uintptr_t may not match between stages unless we're asserting that's always the case. […]
Or let's just capture it in a comment somewhere so the assumptions aren't implicit. I personally think it's hard to assert that outside of qemu where RAM is consistently available vs a real implementation where we actually need different ISAs targeted across stages because one couldn't enter x86-64 mode, e.g., until ramstage.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/1/src/include/cbmem.h@77 PS1, Line 77: uintptr_t
Or let's just capture it in a comment somewhere so the assumptions aren't implicit. I personally think it's hard to assert that outside of qemu where RAM is consistently available vs a real implementation where we actually need different ISAs targeted across stages because one couldn't enter x86-64 mode, e.g., until ramstage.
ok.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/2/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/2/src/lib/cbmem_top.c@25 PS2, Line 25: is implemented */ Using MAYBE_STATIC_BSS you should be able to write this without preprocessor use already today.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/2/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/2/src/lib/cbmem_top.c@25 PS2, Line 25: is implemented */
Using MAYBE_STATIC_BSS you should be able to write this without preprocessor use already today.
Thx!
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#3).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 41 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/3
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/3/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/3/src/lib/cbmem_top.c@20 PS3, Line 20: return (void *)_cbmem_top_ptr; This symbol does not exist until CB:36274, technically this commit would fail to build.
I agree with the way different arch/ are split to followups commits, I just found this a bit confusing to review. A comment might be in order here to say program loader fills it in for us.
https://review.coreboot.org/c/coreboot/+/36273/3/src/lib/cbmem_top.c@27 PS3, Line 27: odd whitespace?
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#4).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/4
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#5).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 42 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/5
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#7).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 44 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/7
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 7: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 9: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig@27 PS9, Line 27: config RAMSTAGE_CBMEM_TOP_ARG nit: Does this need a new Kconfig or can we just make the default cbmem_top() implementation __weak and add it unconditionally? (I like to avoid adding boilerplate Kconfigs that every platform from now on will want to select.)
Alternatively, maybe we could turn the logic around and make a CUSTOM_CBMEM_TOP option that all current platforms need to select?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig@27 PS9, Line 27: config RAMSTAGE_CBMEM_TOP_ARG
nit: Does this need a new Kconfig or can we just make the default cbmem_top() implementation __weak and add it unconditionally? (I like to avoid adding boilerplate Kconfigs that every platform from now on will want to select.)
Alternatively, maybe we could turn the logic around and make a CUSTOM_CBMEM_TOP option that all current platforms need to select?
The intend of the Kconfig is more of transitional nature. The idea was to drop it as soon as all platforms used it. Almost all platforms are covered except nvidia SOC which have a custom entry point and MIPS. It's not really clear to me why they have a custom entry point. Any idea if doing the same as on arm and arm64 would work?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig@27 PS9, Line 27: config RAMSTAGE_CBMEM_TOP_ARG
nit: Does this need a new Kconfig or can we just make the default cbmem_top() implementation __wea […]
On the Nvidia platforms the earlier stages run on a different co-processor using a different architecture (arm32 instead of arm64). That's why they need a custom entry point. You can't directly pass a parameter because one processor can't write the registers of the other. You could instead store the value somewhere in memory and have the custom entry point pick it up to pass through to ramstage main()... however, since (I assume?) you don't have one of these boards to test, it's probably better to just not touch it.
I think switching most platforms over to this is a good idea, but I'd still leave an escape hatch for platforms that have a special issue or are just too old and difficult to untangle right now.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig@27 PS9, Line 27: config RAMSTAGE_CBMEM_TOP_ARG
On the Nvidia platforms the earlier stages run on a different co-processor using a different architecture (arm32 instead of arm64). That's why they need a custom entry point. You can't directly pass a parameter because one processor can't write the registers of the other. You could instead store the value somewhere in memory and have the custom entry point pick it up to pass through to ramstage main()... however, since (I assume?) you don't have one of these boards to test, it's probably better to just not touch it.
I think switching most platforms over to this is a good idea, but I'd still leave an escape hatch for platforms that have a special issue or are just too old and difficult to untangle right now.
That does seem a little more tricky. RISCV also seems more involving. It already passes more than one arguments. I'll flip the Kconfig option at the end of this patch series and have RISCV and the nvidia platforms select it.
Hello Kyösti Mälkki, Julius Werner, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#10).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 44 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/10
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 10: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/10/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/10/src/include/cbmem.h@78 PS10, Line 78: * and later stages. */ Which I still hold is an incorrect assumption. But the code thus far handles it thanks to little endian on x86.
https://review.coreboot.org/c/coreboot/+/36273/10/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/10/src/lib/Kconfig@31 PS10, Line 31: the as the?
Hello Kyösti Mälkki, Aaron Durbin, Julius Werner, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#11).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 44 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/11
Hello Kyösti Mälkki, Aaron Durbin, Julius Werner, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#12).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to be used by all coreboot target. To ease the review process it is currently guarded by a Kconfig symbol to be able to make transition platforms/arch step by step.
To avoid a lot of preprocessor and/or changing file inclusion all current cbmem_top implementations will be renamed cbmem_top_romstage and called by the common cbmem_top function. The additional advantage is that if possible a static variable is used to cache the cbmem_top_romstage result.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c 4 files changed, 44 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/12
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/10/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/10/src/include/cbmem.h@78 PS10, Line 78: * and later stages. */
Which I still hold is an incorrect assumption. But the code thus far handles it thanks to little endian on x86.
Updated it to say that the result of cbem_top_romstage has to fit in the size of uintptr_t in ramstage.
https://review.coreboot.org/c/coreboot/+/36273/10/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/10/src/lib/Kconfig@31 PS10, Line 31: the
as the?
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/12/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/12/src/lib/cbmem_top.c@22 PS12, Line 22: _cbmem_top_ptr Just make this a global within this compilation unit?
Hello Kyösti Mälkki, Aaron Durbin, Julius Werner, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#13).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to all coreboot target.
In romstage a static variable will be used to cache the result of cbmem_top_romstage.
In ramstage if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is set a global variable needs to be populated by the stage entry with the value passed via the calling arguments. if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the same implementation as will be used as in romstage.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/ti/am335x/cbmem.c M src/include/cbmem.h M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/cbmem_top.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 50 files changed, 98 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/13
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
I moved things around a little. This cbmem_top implementation will always be used and either call the romstage version or get it's global pointer updated from the stage entry.
https://review.coreboot.org/c/coreboot/+/36273/12/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/12/src/lib/cbmem_top.c@22 PS12, Line 22: _cbmem_top_ptr
Just make this a global within this compilation unit?
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/10/src/include/cbmem.h File src/include/cbmem.h:
https://review.coreboot.org/c/coreboot/+/36273/10/src/include/cbmem.h@78 PS10, Line 78: * and later stages. */
Which I still hold is an incorrect assumption. […]
Done
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/36273/9/src/lib/Kconfig@27 PS9, Line 27: config RAMSTAGE_CBMEM_TOP_ARG
On the Nvidia platforms the earlier stages run on a different co-processor using a different archi […]
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/13/src/lib/cbmem_top.c@22 PS13, Line 22: { Sorry for nitpicking, but I think it's clearer if we do the following:
if (ENV_ROMSTAGE) { utilize cbmem_top_romstage() } else if (ENV_RAMSTAGE || ENV_POSTCAR) { if (CONFIG(RAMSTAGE_CBMEM_TOP_ARG) return _cbmem_top_ptr; }
return NULL;
The combinations of the logic you have below means we'd be calling out to cbmem_top_romstage() in ramstage under certain conditions which I think is wrong. Being explicit means we aren't implicitly creating such situations. Thoughts? That or cbmem_top_romstage() is not the correct name for this function.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. : I think the naming of your source of cbmem top is the issue. We shouldn't be using functions named _romstage() in stages other than romstage. It just causes confusion.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/13/src/lib/cbmem_top.c@22 PS13, Line 22: {
Sorry for nitpicking, but I think it's clearer if we do the following:
if (ENV_ROMSTAGE) { utilize cbmem_top_romstage() } else if (ENV_RAMSTAGE || ENV_POSTCAR) { if (CONFIG(RAMSTAGE_CBMEM_TOP_ARG) return _cbmem_top_ptr; }
return NULL;
The combinations of the logic you have below means we'd be calling out to cbmem_top_romstage() in ramstage under certain conditions which I think is wrong. Being explicit means we aren't implicitly creating such situations. Thoughts? That or cbmem_top_romstage() is not the correct name for this function.
Ok about making things more explicit. In a later patch series the logic is flipped with CONFIG_RAMSTAGE_CBMEM_TOP_ARG becoming CONFIG_RAMSTAGE_USE_CBMEM_TOP_ROMSTAGE. Is the name bad? My train of thought was that the non-default is to use the same implementation as in romstage. Although that is not necessarily the case if one decides to link other code... Should it be made explicit with a cbmem_top_ramstage callback?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. :
I think the naming of your source of cbmem top is the issue. We shouldn't be using functions named _romstage() in stages other than romstage. It just causes confusion.
I see. What about a 'new' optional callback cbmem_top_ramstage that will in this case return cbmem_top_romstage? At the moment only RISCV (which does not even initialize cbmem in romstage on some targets) and NVIDIA tegra210 need it.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. :
I think the naming of your source of cbmem top is the issue. We shouldn't be using functions named _romstage() in stages other than romstage. It just causes confusion.
I see. What about a 'new' optional callback cbmem_top_ramstage that will in this case return cbmem_top_romstage? At the moment only RISCV (which does not even initialize cbmem in romstage on some targets) and NVIDIA tegra210 need it.
risc-v is broken there then (LATE_CBMEM_INIT is supposed to be gone). Need to look into tegra210 as well.
I'm more concerned about the naming having _romstage() in it. Call it cbmem_top_chipset() instead? Or something else? The suffix may not be the best, but it's not tied to people's thinking around a stage.
Last thing: add your implementation of cbmem_top() to src/lib/imd_cbmem.c as that's where our cbmem API is implemented. I don't think we need a new file as this point. Or add it to cbmem_common.c (I'm not sure why we have that separated tbh).
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. :
I think the naming of your source of cbmem top is the issue. We shouldn't be using functions named _romstage() in stages other than romstage. It just causes confusion.
I see. What about a 'new' optional callback cbmem_top_ramstage that will in this case return cbmem_top_romstage? At the moment only RISCV (which does not even initialize cbmem in romstage on some targets) and NVIDIA tegra210 need it.
risc-v is broken there then (LATE_CBMEM_INIT is supposed to be gone). Need to look into tegra210 as well.
See CB:36446 for riscv. Tegra210 has a different core running the romstage than the ramstage. As they don't share registers passing via calling arguments is not possible.
I'm more concerned about the naming having _romstage() in it. Call it cbmem_top_chipset() instead? Or something else? The suffix may not be the best, but it's not tied to people's thinking around a stage.
Last thing: add your implementation of cbmem_top() to src/lib/imd_cbmem.c as that's where our cbmem API is implemented. I don't think we need a new file as this point. Or add it to cbmem_common.c (I'm not sure why we have that separated tbh).
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. :
I think the naming of your source of cbmem top is the issue. […]
Which risc-v targets are busted? the qemu one? Seems like it. grumble. The emulation targets are helpful, but they allow people to make sweeping assumptions and/or take shortcuts which gets us in this situation. I just pushed https://review.coreboot.org/c/coreboot/+/36448 to correct that.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. :
Which risc-v targets are busted? the qemu one? Seems like it. grumble. […]
Hah. I pushed the same change as CB:36446. That can just land on its own if you pull the dependency from it. I'll abandon mine.
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, Patrick Georgi, ron minnich, Vanny E, Huang Jin, Philipp Deppenwiese, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#14).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to all coreboot target.
In romstage a static variable will be used to cache the result of cbmem_top_romstage.
In ramstage if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is set a global variable needs to be populated by the stage entry with the value passed via the calling arguments. if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the same implementation as will be used as in romstage.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/ti/am335x/cbmem.c M src/include/cbmem.h M src/lib/Kconfig M src/lib/imd_cbmem.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 49 files changed, 85 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/14
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/14/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/36273/14/src/lib/imd_cbmem.c@54 PS14, Line 54: RAMSTAGE_USE_CBMEM_TOP_ROMSTAGE RAMSTAGE_CBMEM_TOP_ARG
https://review.coreboot.org/c/coreboot/+/36273/14/src/lib/imd_cbmem.c@61 PS14, Line 61: RAMSTAGE_USE_CBMEM_TOP_ROMSTAGE RAMSTAGE_CBMEM_TOP_ARG
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, Patrick Georgi, ron minnich, Vanny E, Huang Jin, Philipp Deppenwiese, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#15).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to all coreboot target.
In romstage a static variable will be used to cache the result of cbmem_top_romstage.
In ramstage if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is set a global variable needs to be populated by the stage entry with the value passed via the calling arguments. if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the same implementation as will be used as in romstage.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/ti/am335x/cbmem.c M src/include/cbmem.h M src/lib/Kconfig M src/lib/imd_cbmem.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 49 files changed, 85 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/15
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/15/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/36273/15/src/lib/imd_cbmem.c@64 PS15, Line 64: return NULL; Since this should be unreachable (right?) you can just write
dead_code();
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, Patrick Georgi, ron minnich, Vanny E, Huang Jin, Philipp Deppenwiese, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#16).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to all coreboot target.
In romstage a static variable will be used to cache the result of cbmem_top_romstage.
In ramstage if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is set a global variable needs to be populated by the stage entry with the value passed via the calling arguments. if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the same implementation as will be used as in romstage.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/ti/am335x/cbmem.c M src/include/cbmem.h M src/lib/Kconfig M src/lib/imd_cbmem.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 49 files changed, 86 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/16
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36273/15/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/36273/15/src/lib/imd_cbmem.c@64 PS15, Line 64: return NULL;
Since this should be unreachable (right?) you can just write […]
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 16: Code-Review+2
Let's land and see how things go.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36273/13//COMMIT_MSG@16 PS13, Line 16: if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the : same implementation as will be used as in romstage. :
Hah. I pushed the same change as CB:36446. […]
Done
https://review.coreboot.org/c/coreboot/+/36273/13/src/lib/cbmem_top.c File src/lib/cbmem_top.c:
https://review.coreboot.org/c/coreboot/+/36273/13/src/lib/cbmem_top.c@22 PS13, Line 22: {
Sorry for nitpicking, but I think it's clearer if we do the following: […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 16:
Gerrit says there are problems with integrating this change so you probably need to rebase.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 16: Code-Review+2
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, Patrick Georgi, ron minnich, Vanny E, Huang Jin, Marshall Dawson, Philipp Deppenwiese, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36273
to look at the new patch set (#17).
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to all coreboot target.
In romstage a static variable will be used to cache the result of cbmem_top_romstage.
In ramstage if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is set a global variable needs to be populated by the stage entry with the value passed via the calling arguments. if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the same implementation as will be used as in romstage.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/ti/am335x/cbmem.c M src/include/cbmem.h M src/lib/Kconfig M src/lib/imd_cbmem.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 49 files changed, 86 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/36273/17
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
Patch Set 17: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36273 )
Change subject: lib/cbmem_top: Add a common cbmem_top implementation ......................................................................
lib/cbmem_top: Add a common cbmem_top implementation
This adds a common cbmem_top implementation to all coreboot target.
In romstage a static variable will be used to cache the result of cbmem_top_romstage.
In ramstage if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is set a global variable needs to be populated by the stage entry with the value passed via the calling arguments. if CONFIG_RAMSTAGE_CBMEM_TOP_ARG is not set the same implementation as will be used as in romstage.
Change-Id: Ie767542ee25483acc9a56785ce20a885e9a63098 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/36273 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/ti/am335x/cbmem.c M src/include/cbmem.h M src/lib/Kconfig M src/lib/imd_cbmem.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 49 files changed, 86 insertions(+), 46 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/arch/x86/cbmem.c b/src/arch/x86/cbmem.c index 16c35b5..f7c58a4 100644 --- a/src/arch/x86/cbmem.c +++ b/src/arch/x86/cbmem.c @@ -16,7 +16,7 @@
#if CONFIG(CBMEM_TOP_BACKUP)
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { static void *cbmem_top_backup; void *top_backup; diff --git a/src/cpu/amd/family_10h-family_15h/ram_calc.c b/src/cpu/amd/family_10h-family_15h/ram_calc.c index 3946b67..a1dc1f4 100644 --- a/src/cpu/amd/family_10h-family_15h/ram_calc.c +++ b/src/cpu/amd/family_10h-family_15h/ram_calc.c @@ -86,7 +86,7 @@ return cc6_size; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { uint32_t topmem = rdmsr(TOP_MEM).lo;
diff --git a/src/cpu/ti/am335x/cbmem.c b/src/cpu/ti/am335x/cbmem.c index a626ec6..2ecca65 100644 --- a/src/cpu/ti/am335x/cbmem.c +++ b/src/cpu/ti/am335x/cbmem.c @@ -15,7 +15,7 @@ #include <cbmem.h> #include <symbols.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return _dram + (CONFIG_DRAM_SIZE_MB << 20); } diff --git a/src/include/cbmem.h b/src/include/cbmem.h index 4005fa2..a22c420 100644 --- a/src/include/cbmem.h +++ b/src/include/cbmem.h @@ -73,7 +73,18 @@ * below 4GiB for 32bit coreboot builds. On 64bit coreboot builds there's no * upper limit. This should not be called before memory is initialized. */ +/* The assumption is made that the result of cbmem_top_romstage fits in the size + of uintptr_t in the ramstage. */ +extern uintptr_t _cbmem_top_ptr; void *cbmem_top(void); +/* With CONFIG_RAMSTAGE_CBMEM_TOP_ARG set, the result of cbmem_top is passed via + * calling arguments to the next stage and saved in the global _cbmem_top_ptr + * global variable. Only a romstage callback needs to be implemented by the + * platform. It is up to the stages after romstage to save the calling argument + * in the _cbmem_top_ptr symbol. Without CONFIG_RAMSTAGE_CBMEM_TOP_ARG the same + * implementation as used in romstage will be used. + */ +void *cbmem_top_chipset(void);
/* Add a cbmem entry of a given size and id. These return NULL on failure. The * add function performs a find first and do not check against the original diff --git a/src/lib/Kconfig b/src/lib/Kconfig index cb1e4a5..b94ac49 100644 --- a/src/lib/Kconfig +++ b/src/lib/Kconfig @@ -24,6 +24,12 @@ help Selected by features that require `libhwbase` in ramstage.
+config RAMSTAGE_CBMEM_TOP_ARG + bool + help + Select this if stages run after romstage get the cbmem_top + pointer as the function arguments when called from romstage. + config FLATTENED_DEVICE_TREE bool help diff --git a/src/lib/imd_cbmem.c b/src/lib/imd_cbmem.c index be135c2..cbd4b8f 100644 --- a/src/lib/imd_cbmem.c +++ b/src/lib/imd_cbmem.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <assert.h> #include <boot/coreboot_tables.h> #include <bootstate.h> #include <bootmem.h> @@ -44,6 +45,28 @@ (!CONFIG(ARCH_X86) || ENV_RAMSTAGE || ENV_POSTCAR || \ !CONFIG(CAR_GLOBAL_MIGRATION))
+/* The program loader passes on cbmem_top and the program entry point + has to fill in the _cbmem_top_ptr symbol based on the calling arguments. */ +uintptr_t _cbmem_top_ptr; + +void *cbmem_top(void) +{ + if (ENV_ROMSTAGE + || ((ENV_POSTCAR || ENV_RAMSTAGE) + && !CONFIG(RAMSTAGE_CBMEM_TOP_ARG))) { + MAYBE_STATIC_BSS void *top = NULL; + if (top) + return top; + top = cbmem_top_chipset(); + return top; + } + if ((ENV_POSTCAR || ENV_RAMSTAGE) && CONFIG(RAMSTAGE_CBMEM_TOP_ARG)) + return (void *)_cbmem_top_ptr; + + dead_code(); +} + + static inline struct imd *cbmem_get_imd(void) { if (CAN_USE_GLOBALS) { diff --git a/src/mainboard/emulation/qemu-aarch64/cbmem.c b/src/mainboard/emulation/qemu-aarch64/cbmem.c index c50254d..4389433 100644 --- a/src/mainboard/emulation/qemu-aarch64/cbmem.c +++ b/src/mainboard/emulation/qemu-aarch64/cbmem.c @@ -10,7 +10,7 @@ #include <ramdetect.h> #include <symbols.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return _dram + (probe_ramsize((uintptr_t)_dram, CONFIG_DRAM_SIZE_MB) * MiB); } diff --git a/src/mainboard/emulation/qemu-armv7/cbmem.c b/src/mainboard/emulation/qemu-armv7/cbmem.c index 542e08d..143e11b 100644 --- a/src/mainboard/emulation/qemu-armv7/cbmem.c +++ b/src/mainboard/emulation/qemu-armv7/cbmem.c @@ -15,7 +15,7 @@ #include <symbols.h> #include <ramdetect.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return _dram + (probe_ramsize((uintptr_t)_dram, CONFIG_DRAM_SIZE_MB) * MiB); } diff --git a/src/mainboard/emulation/qemu-i440fx/memmap.c b/src/mainboard/emulation/qemu-i440fx/memmap.c index 8209379..098b3c2 100644 --- a/src/mainboard/emulation/qemu-i440fx/memmap.c +++ b/src/mainboard/emulation/qemu-i440fx/memmap.c @@ -52,7 +52,7 @@ return tomk; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t top = 0;
diff --git a/src/mainboard/emulation/qemu-power8/cbmem.c b/src/mainboard/emulation/qemu-power8/cbmem.c index 3df6b80..7d6d4a8 100644 --- a/src/mainboard/emulation/qemu-power8/cbmem.c +++ b/src/mainboard/emulation/qemu-power8/cbmem.c @@ -15,7 +15,7 @@
#include <cbmem.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { /* Top of cbmem is at lowest usable DRAM address below 4GiB. */ /* For now, last 1M of 4G */ diff --git a/src/northbridge/intel/e7505/memmap.c b/src/northbridge/intel/e7505/memmap.c index c6a20fa..009db80 100644 --- a/src/northbridge/intel/e7505/memmap.c +++ b/src/northbridge/intel/e7505/memmap.c @@ -21,7 +21,7 @@ #include <program_loading.h> #include "e7505.h"
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { pci_devfn_t mch = PCI_DEV(0, 0, 0); uintptr_t tolm; diff --git a/src/northbridge/intel/fsp_rangeley/memmap.c b/src/northbridge/intel/fsp_rangeley/memmap.c index da9ed71..275ddd3 100644 --- a/src/northbridge/intel/fsp_rangeley/memmap.c +++ b/src/northbridge/intel/fsp_rangeley/memmap.c @@ -36,7 +36,7 @@ return tom; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *) (smm_region_start() - FSP_RESERVE_MEMORY_SIZE); } diff --git a/src/northbridge/intel/gm45/memmap.c b/src/northbridge/intel/gm45/memmap.c index 7479a78..d34820e 100644 --- a/src/northbridge/intel/gm45/memmap.c +++ b/src/northbridge/intel/gm45/memmap.c @@ -117,7 +117,7 @@ * 1 MiB alignment. As this may cause very greedy MTRR setup, push * CBMEM top downwards to 4 MiB boundary. */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t top_of_ram = ALIGN_DOWN(northbridge_get_tseg_base(), 4*MiB); return (void *) top_of_ram; diff --git a/src/northbridge/intel/haswell/memmap.c b/src/northbridge/intel/haswell/memmap.c index 007a67d..74d9292 100644 --- a/src/northbridge/intel/haswell/memmap.c +++ b/src/northbridge/intel/haswell/memmap.c @@ -34,7 +34,7 @@ return tom & ~((1 << 20) - 1); }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)smm_region_start(); } diff --git a/src/northbridge/intel/i440bx/memmap.c b/src/northbridge/intel/i440bx/memmap.c index 75a6c7e..d260af6 100644 --- a/src/northbridge/intel/i440bx/memmap.c +++ b/src/northbridge/intel/i440bx/memmap.c @@ -23,7 +23,7 @@ #include <program_loading.h> #include "i440bx.h"
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { /* Base of TSEG is top of usable DRAM */ /* diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c index 8207d06..000ac7e 100644 --- a/src/northbridge/intel/i945/memmap.c +++ b/src/northbridge/intel/i945/memmap.c @@ -71,7 +71,7 @@ * 1 MiB alignment. As this may cause very greedy MTRR setup, push * CBMEM top downwards to 4 MiB boundary. */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t top_of_ram = ALIGN_DOWN(northbridge_get_tseg_base(), 4*MiB); return (void *) top_of_ram; diff --git a/src/northbridge/intel/nehalem/memmap.c b/src/northbridge/intel/nehalem/memmap.c index 1c17b0d..5de4b80 100644 --- a/src/northbridge/intel/nehalem/memmap.c +++ b/src/northbridge/intel/nehalem/memmap.c @@ -42,7 +42,7 @@ return CONFIG_SMM_TSEG_SIZE; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *) smm_region_start(); } diff --git a/src/northbridge/intel/pineview/memmap.c b/src/northbridge/intel/pineview/memmap.c index b4fef6b..0aa70cd 100644 --- a/src/northbridge/intel/pineview/memmap.c +++ b/src/northbridge/intel/pineview/memmap.c @@ -132,7 +132,7 @@ * 1 MiB alignment. As this may cause very greedy MTRR setup, push * CBMEM top downwards to 4 MiB boundary. */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t top_of_ram = ALIGN_DOWN(northbridge_get_tseg_base(), 4*MiB); return (void *) top_of_ram; diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c index 67de344..99888fa 100644 --- a/src/northbridge/intel/sandybridge/memmap.c +++ b/src/northbridge/intel/sandybridge/memmap.c @@ -31,7 +31,7 @@ return tom; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *) smm_region_start(); } diff --git a/src/northbridge/intel/x4x/memmap.c b/src/northbridge/intel/x4x/memmap.c index 41e4912..1924ddf 100644 --- a/src/northbridge/intel/x4x/memmap.c +++ b/src/northbridge/intel/x4x/memmap.c @@ -128,7 +128,7 @@ * 1 MiB alignment. As this may cause very greedy MTRR setup, push * CBMEM top downwards to 4 MiB boundary. */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t top_of_ram = ALIGN_DOWN(northbridge_get_tseg_base(), 4*MiB); return (void *) top_of_ram; diff --git a/src/northbridge/via/vx900/memmap.c b/src/northbridge/via/vx900/memmap.c index d11dc65..3121d74 100644 --- a/src/northbridge/via/vx900/memmap.c +++ b/src/northbridge/via/vx900/memmap.c @@ -120,7 +120,7 @@ return (pci_read_config16(MCU, 0x84) & 0xfff0) >> 4; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t tolm; uintptr_t fb_size; diff --git a/src/soc/amd/picasso/memmap.c b/src/soc/amd/picasso/memmap.c index 09af7e4..82d6fb6 100644 --- a/src/soc/amd/picasso/memmap.c +++ b/src/soc/amd/picasso/memmap.c @@ -58,7 +58,7 @@ *size = BERT_REGION_MAX_SIZE; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { msr_t tom = rdmsr(TOP_MEM);
diff --git a/src/soc/amd/stoneyridge/memmap.c b/src/soc/amd/stoneyridge/memmap.c index 09af7e4..82d6fb6 100644 --- a/src/soc/amd/stoneyridge/memmap.c +++ b/src/soc/amd/stoneyridge/memmap.c @@ -58,7 +58,7 @@ *size = BERT_REGION_MAX_SIZE; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { msr_t tom = rdmsr(TOP_MEM);
diff --git a/src/soc/cavium/cn81xx/cbmem.c b/src/soc/cavium/cn81xx/cbmem.c index bb6fa18..a39bf4f 100644 --- a/src/soc/cavium/cn81xx/cbmem.c +++ b/src/soc/cavium/cn81xx/cbmem.c @@ -20,7 +20,7 @@ #include <stdlib.h> #include <symbols.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { /* Make sure not to overlap with reserved ATF scratchpad */ return (void *)((uintptr_t)_dram + (sdram_size_mb() - 1) * MiB); diff --git a/src/soc/imgtec/pistachio/cbmem.c b/src/soc/imgtec/pistachio/cbmem.c index 112df7c..92bc1ce 100644 --- a/src/soc/imgtec/pistachio/cbmem.c +++ b/src/soc/imgtec/pistachio/cbmem.c @@ -18,7 +18,7 @@ #include <stdlib.h> #include <symbols.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return _dram + (CONFIG_DRAM_SIZE_MB << 20); } diff --git a/src/soc/intel/apollolake/memmap.c b/src/soc/intel/apollolake/memmap.c index f828024..567ff1e 100644 --- a/src/soc/intel/apollolake/memmap.c +++ b/src/soc/intel/apollolake/memmap.c @@ -20,7 +20,7 @@
#include "chip.h"
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { const config_t *config; void *tolum = (void *)sa_get_tseg_base(); diff --git a/src/soc/intel/baytrail/memmap.c b/src/soc/intel/baytrail/memmap.c index d9f6160..e0aac9f 100644 --- a/src/soc/intel/baytrail/memmap.c +++ b/src/soc/intel/baytrail/memmap.c @@ -29,7 +29,7 @@ return CONFIG_SMM_TSEG_SIZE; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *) smm_region_start(); } diff --git a/src/soc/intel/braswell/memmap.c b/src/soc/intel/braswell/memmap.c index d502aed..e43c546 100644 --- a/src/soc/intel/braswell/memmap.c +++ b/src/soc/intel/braswell/memmap.c @@ -33,7 +33,7 @@ *size = smm_region_size(); }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { uintptr_t smm_base; size_t smm_size; diff --git a/src/soc/intel/broadwell/memmap.c b/src/soc/intel/broadwell/memmap.c index f4a9d0e..ad50dd3 100644 --- a/src/soc/intel/broadwell/memmap.c +++ b/src/soc/intel/broadwell/memmap.c @@ -41,7 +41,7 @@ return tom; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *) dpr_region_start(); } diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index 475b8c7..7a0d897 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -247,7 +247,7 @@ * | | * +-------------------------+ */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { struct ebda_config ebda_cfg;
diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index 9f788dd..b4761db 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -60,7 +60,7 @@ power_of_2(iqat_region_size + tseg_region_size); }
-void *cbmem_top(void) { return (void *)top_of_32bit_ram(); } +void *cbmem_top_chipset(void) { return (void *)top_of_32bit_ram(); }
static inline uintptr_t smm_region_start(void) { diff --git a/src/soc/intel/fsp_baytrail/memmap.c b/src/soc/intel/fsp_baytrail/memmap.c index 7fec7f9..d8dcf49 100644 --- a/src/soc/intel/fsp_baytrail/memmap.c +++ b/src/soc/intel/fsp_baytrail/memmap.c @@ -40,7 +40,7 @@ * @return pointer to the first byte of reserved memory */
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return find_fsp_reserved_mem(*(void **)CBMEM_FSP_HOB_PTR); } diff --git a/src/soc/intel/fsp_broadwell_de/memmap.c b/src/soc/intel/fsp_broadwell_de/memmap.c index cbd3cf7..96eb205 100644 --- a/src/soc/intel/fsp_broadwell_de/memmap.c +++ b/src/soc/intel/fsp_broadwell_de/memmap.c @@ -23,7 +23,7 @@ #include <soc/pci_devs.h> #include <device/pci_ops.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return find_fsp_reserved_mem(*(void **)CBMEM_FSP_HOB_PTR); } diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index f17f255..76a8128 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -226,7 +226,7 @@ * | | * +-------------------------+ */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { struct ebda_config ebda_cfg;
diff --git a/src/soc/intel/quark/memmap.c b/src/soc/intel/quark/memmap.c index b8b8506..9ccaf55 100644 --- a/src/soc/intel/quark/memmap.c +++ b/src/soc/intel/quark/memmap.c @@ -18,7 +18,7 @@ #include <cbmem.h> #include <soc/reg_access.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { uint32_t top_of_memory;
diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 3aea1c3..09dc6e9 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -248,7 +248,7 @@ * | | * +-------------------------+ */ -void *cbmem_top(void) +void *cbmem_top_chipset(void) { struct ebda_config ebda_cfg;
diff --git a/src/soc/mediatek/common/cbmem.c b/src/soc/mediatek/common/cbmem.c index 8906565..1a55d01 100644 --- a/src/soc/mediatek/common/cbmem.c +++ b/src/soc/mediatek/common/cbmem.c @@ -21,7 +21,7 @@
#define MAX_DRAM_ADDRESS ((uintptr_t)4 * GiB)
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)min((uintptr_t)_dram + sdram_size(), MAX_DRAM_ADDRESS); } diff --git a/src/soc/nvidia/tegra124/cbmem.c b/src/soc/nvidia/tegra124/cbmem.c index 4b52a51..ac2a92e 100644 --- a/src/soc/nvidia/tegra124/cbmem.c +++ b/src/soc/nvidia/tegra124/cbmem.c @@ -17,7 +17,7 @@ #include <soc/display.h> #include <soc/sdram.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)((sdram_max_addressable_mb() - FB_SIZE_MB) << 20UL); } diff --git a/src/soc/nvidia/tegra210/cbmem.c b/src/soc/nvidia/tegra210/cbmem.c index 63ae497..7fdde9e 100644 --- a/src/soc/nvidia/tegra210/cbmem.c +++ b/src/soc/nvidia/tegra210/cbmem.c @@ -16,7 +16,7 @@ #include <cbmem.h> #include <soc/addressmap.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { static uintptr_t addr;
diff --git a/src/soc/qualcomm/ipq40xx/cbmem.c b/src/soc/qualcomm/ipq40xx/cbmem.c index 05325cc..972c625 100644 --- a/src/soc/qualcomm/ipq40xx/cbmem.c +++ b/src/soc/qualcomm/ipq40xx/cbmem.c @@ -23,7 +23,7 @@ cbmem_backing_store_ready = 1; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { /* * In romstage, make sure that cbmem backing store is ready before diff --git a/src/soc/qualcomm/ipq806x/cbmem.c b/src/soc/qualcomm/ipq806x/cbmem.c index 9674db6..6dc92a0 100644 --- a/src/soc/qualcomm/ipq806x/cbmem.c +++ b/src/soc/qualcomm/ipq806x/cbmem.c @@ -23,7 +23,7 @@ cbmem_backing_store_ready = 1; }
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { /* * In romstage, make sure that cbmem backing store is ready before diff --git a/src/soc/qualcomm/qcs405/cbmem.c b/src/soc/qualcomm/qcs405/cbmem.c index e065409..a780c6b 100644 --- a/src/soc/qualcomm/qcs405/cbmem.c +++ b/src/soc/qualcomm/qcs405/cbmem.c @@ -15,7 +15,7 @@
#include <cbmem.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)((uintptr_t)3 * GiB); } diff --git a/src/soc/qualcomm/sc7180/cbmem.c b/src/soc/qualcomm/sc7180/cbmem.c index 597e369..fe81309 100644 --- a/src/soc/qualcomm/sc7180/cbmem.c +++ b/src/soc/qualcomm/sc7180/cbmem.c @@ -15,7 +15,7 @@
#include <cbmem.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)((uintptr_t)4 * GiB); } diff --git a/src/soc/qualcomm/sdm845/cbmem.c b/src/soc/qualcomm/sdm845/cbmem.c index 3b9ad4a..b092a1a 100644 --- a/src/soc/qualcomm/sdm845/cbmem.c +++ b/src/soc/qualcomm/sdm845/cbmem.c @@ -15,7 +15,7 @@
#include <cbmem.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)((uintptr_t)4 * GiB); } diff --git a/src/soc/rockchip/common/cbmem.c b/src/soc/rockchip/common/cbmem.c index 401f8b2..6e3aabb 100644 --- a/src/soc/rockchip/common/cbmem.c +++ b/src/soc/rockchip/common/cbmem.c @@ -19,7 +19,7 @@ #include <stdlib.h> #include <symbols.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)min((uintptr_t)_dram + sdram_size_mb() * MiB, MAX_DRAM_ADDRESS); diff --git a/src/soc/samsung/exynos5250/cbmem.c b/src/soc/samsung/exynos5250/cbmem.c index 1874495..31463b1 100644 --- a/src/soc/samsung/exynos5250/cbmem.c +++ b/src/soc/samsung/exynos5250/cbmem.c @@ -17,7 +17,7 @@ #include <cbmem.h> #include <soc/cpu.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)(get_fb_base_kb() * KiB); } diff --git a/src/soc/samsung/exynos5420/cbmem.c b/src/soc/samsung/exynos5420/cbmem.c index e1999e8..ffed589e 100644 --- a/src/soc/samsung/exynos5420/cbmem.c +++ b/src/soc/samsung/exynos5420/cbmem.c @@ -17,7 +17,7 @@ #include <soc/cpu.h> #include <stddef.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)(get_fb_base_kb() * KiB); } diff --git a/src/soc/sifive/fu540/cbmem.c b/src/soc/sifive/fu540/cbmem.c index 1c68de8..a7de16c 100644 --- a/src/soc/sifive/fu540/cbmem.c +++ b/src/soc/sifive/fu540/cbmem.c @@ -19,7 +19,7 @@ #include <stdlib.h> #include <symbols.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return (void *)min((uintptr_t)_dram + sdram_size_mb() * MiB, FU540_MAXDRAM); diff --git a/src/soc/ucb/riscv/cbmem.c b/src/soc/ucb/riscv/cbmem.c index 542e08d..143e11b 100644 --- a/src/soc/ucb/riscv/cbmem.c +++ b/src/soc/ucb/riscv/cbmem.c @@ -15,7 +15,7 @@ #include <symbols.h> #include <ramdetect.h>
-void *cbmem_top(void) +void *cbmem_top_chipset(void) { return _dram + (probe_ramsize((uintptr_t)_dram, CONFIG_DRAM_SIZE_MB) * MiB); }
-
Aaron Durbin (Code Review) -
Arthur Heymans (Code Review)
-
Julius Werner (Code Review)
-
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Patrick Georgi (Code Review)