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 
---
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;
 }

[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.

[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.

[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.

[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.

[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.

[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?

[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.

[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

[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.