Change in coreboot[master]: emulation/riscv: Don't probe for total DRAM

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... emulation/riscv: Don't probe for total DRAM Trying accessing memory that is not there causes an exception. Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/mainboard/emulation/qemu-riscv/Kconfig M src/mainboard/emulation/qemu-riscv/mainboard.c M src/soc/ucb/riscv/cbmem.c 3 files changed, 4 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36445/1 diff --git a/src/mainboard/emulation/qemu-riscv/Kconfig b/src/mainboard/emulation/qemu-riscv/Kconfig index fa6dccc..c3136c1 100644 --- a/src/mainboard/emulation/qemu-riscv/Kconfig +++ b/src/mainboard/emulation/qemu-riscv/Kconfig @@ -55,8 +55,8 @@ default 1 config DRAM_SIZE_MB - int - default 32768 + int "size of DRAM. Run qemu with the argument -m ***M" + default 1024 config OPENSBI_PLATFORM string diff --git a/src/mainboard/emulation/qemu-riscv/mainboard.c b/src/mainboard/emulation/qemu-riscv/mainboard.c index 88898087..eda43c9 100644 --- a/src/mainboard/emulation/qemu-riscv/mainboard.c +++ b/src/mainboard/emulation/qemu-riscv/mainboard.c @@ -27,7 +27,7 @@ die("No dev0; die\n"); } - dram_mb_detected = probe_ramsize((uintptr_t)_dram, CONFIG_DRAM_SIZE_MB); + dram_mb_detected = CONFIG_DRAM_SIZE_MB - CONFIG_ROM_SIZE; ram_resource(dev, 0, (uintptr_t)_dram / KiB, dram_mb_detected * MiB / KiB); cbmem_recovery(0); diff --git a/src/soc/ucb/riscv/cbmem.c b/src/soc/ucb/riscv/cbmem.c index 542e08d..9872760 100644 --- a/src/soc/ucb/riscv/cbmem.c +++ b/src/soc/ucb/riscv/cbmem.c @@ -13,9 +13,8 @@ #include <cbmem.h> #include <symbols.h> -#include <ramdetect.h> void *cbmem_top(void) { - return _dram + (probe_ramsize((uintptr_t)_dram, CONFIG_DRAM_SIZE_MB) * MiB); + return _dram + CONFIG_DRAM_SIZE_MB * MiB - CONFIG_ROM_SIZE; } -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newchange

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1: Code-Review+2 (2 comments) https://review.coreboot.org/c/coreboot/+/36445/1/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-riscv/Kconfig: https://review.coreboot.org/c/coreboot/+/36445/1/src/mainboard/emulation/qem... PS1, Line 58: int "size of DRAM. Run qemu with the argument -m ***M" Isn't this the main problem? Ensuring the correct arguments are passed to qemu? https://review.coreboot.org/c/coreboot/+/36445/1/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-riscv/mainboard.c: https://review.coreboot.org/c/coreboot/+/36445/1/src/mainboard/emulation/qem... PS1, Line 33: cbmem_recovery(0); This is not the right place for this. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Tue, 29 Oct 2019 23:46:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1: Code-Review-1 Actually probe_ramsize is broken. I'll send a fix. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 00:00:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1: -Code-Review
Patch Set 1: Code-Review-1
Actually probe_ramsize is broken. I'll send a fix.
NVM. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 00:47:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/36445/1/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-riscv/Kconfig: https://review.coreboot.org/c/coreboot/+/36445/1/src/mainboard/emulation/qem... PS1, Line 58: int "size of DRAM. Run qemu with the argument -m ***M"
Isn't this the main problem? Ensuring the correct arguments are passed to qemu?
Partially, trying to access memory where there is no dram throws in an exception, so probing cannot be done. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 13:46:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin@chromium.org> Gerrit-MessageType: comment

Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1: Code-Review-1 Please only use this as a work around and don't merge it. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 15:46:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1:
Patch Set 1: Code-Review-1
Please only use this as a work around and don't merge it.
What is your solution to the problem Arthur presented? -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 15:49:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-1
Please only use this as a work around and don't merge it.
What is your solution to the problem Arthur presented?
My solution is to do the same as on ARM. Change the exception handler during probing, to see if an exception was raised. Will push an initial version of it today. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 16:00:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-1
Please only use this as a work around and don't merge it.
What is your solution to the problem Arthur presented?
My solution is to do the same as on ARM. Change the exception handler during probing, to see if an exception was raised. Will push an initial version of it today.
Please have a look at #36486 -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2019 16:37:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36445 ) Change subject: emulation/riscv: Don't probe for total DRAM ...................................................................... Abandoned Probably fixed in recent qemu. -- To view, visit https://review.coreboot.org/c/coreboot/+/36445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4d9fbb727f41436798783a357b3cfb7b64d3edb Gerrit-Change-Number: 36445 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> Gerrit-MessageType: abandon
participants (3)
-
Aaron Durbin (Code Review)
-
Arthur Heymans (Code Review)
-
Philipp Hug (Code Review)