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

[Arthur Heymans (Code Review)]

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 <[e-mail]>
---
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 <[e-mail]>
Gerrit-MessageType: newchange

[Aaron Durbin (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Tue, 29 Oct 2019 23:46:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 00:00:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 00:47:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 13:46:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <[e-mail]>
Gerrit-MessageType: comment

[Philipp Hug (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 15:46:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Aaron Durbin (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 15:49:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Philipp Hug (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 16:00:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Philipp Hug (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 16:37:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-MessageType: abandon