Change in coreboot[master]: Fixed configuration for DDR Size discovered by Coreboot

[Marc Karasek (Code Review)]

Marc Karasek has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38904 )


Change subject: Fixed configuration for DDR Size discovered by Coreboot
......................................................................

Fixed configuration for DDR Size discovered by Coreboot

For qemu-riscv value was incorrect.  It was set to 32768 which
in turn was being used as 32768MB (32+GB), instead of the 32MB
it was intended to represent.

Change value so it is a select option based on in menuconfig
based on what you want coreboot to "discover" on boot.

Current three options are :
128MB
256MB
512MB

NOTE:  You must specify greater than what is chosen from
the above items on the qemu cmdline.

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
M src/mainboard/emulation/qemu-riscv/Kconfig
1 file changed, 27 insertions(+), 1 deletion(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/1

diff --git a/src/mainboard/emulation/qemu-riscv/Kconfig b/src/mainboard/emulation/qemu-riscv/Kconfig
index 4d4c900..2a119a3 100644
--- a/src/mainboard/emulation/qemu-riscv/Kconfig
+++ b/src/mainboard/emulation/qemu-riscv/Kconfig
@@ -54,9 +54,35 @@
 	int
 	default 1
 
+choice
+	prompt "DRAM SIZE MB"
+	help
+	  Select the size of the DDR coreboot will find.
+	  Note: You must specify a -m <size> greater than
+	  what is chosen here.
+
+	config COREBOOT_DDR_SIZE_128M
+	  bool "128 MB"
+	  help
+	    Choose this option if you want coreboot to find 127MB DDR
+
+	config COREBOOT_DDR_SIZE_256M
+	  bool "256 MB"
+	  help
+	    Choose this option if you want coreboot to find 255MB DDR
+
+	config COREBOOT_DDR_SIZE_512M
+	  bool "512 MB"
+	  help
+	    Choose this option if you want coreboot to find 512MB DDR
+endchoice
+
 config DRAM_SIZE_MB
 	int
-	default 32768
+	default 32
+	default 32 if COREBOOT_DDR_SIZE_128M
+	default 64 if COREBOOT_DDR_SIZE_256M
+	default 128 if COREBOOT_DDR_SIZE_512M
 
 config OPENSBI_PLATFORM
 	string

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 1
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-MessageType: newchange

Change in coreboot[master]: BUG#254: Fixed configuration for DDR Size discovered by Coreboot

[Marc Karasek (Code Review)]

Hello ron minnich, build bot (Jenkins), Philipp Hug, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/38904

to look at the new patch set (#2).

Change subject: BUG#254: Fixed configuration for DDR Size discovered by Coreboot
......................................................................

BUG#254: Fixed configuration for DDR Size discovered by Coreboot

For qemu-riscv value was incorrect.  It was set to 32768 which
in turn was being used as 32768MB (32+GB), instead of the 32MB
it was intended to represent.

Change value so it is a select option based on in menuconfig
based on what you want coreboot to "discover" on boot.

Current three options are :
128MB
256MB
512MB

NOTE:  You must specify greater than what is chosen from
the above items on the qemu cmdline.

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
M src/mainboard/emulation/qemu-riscv/Kconfig
1 file changed, 27 insertions(+), 1 deletion(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/2
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 2
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-MessageType: newpatchset

Change in coreboot[master]: BUG#254: Fixed configuration for DDR Size discovered by coreboot

[Marc Karasek (Code Review)]

Hello ron minnich, build bot (Jenkins), Philipp Hug, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/38904

to look at the new patch set (#3).

Change subject: BUG#254: Fixed configuration for DDR Size discovered by coreboot
......................................................................

BUG#254: Fixed configuration for DDR Size discovered by coreboot

For qemu-riscv value was incorrect.  It was set to 32768 which
in turn was being used as 32768MB (32+GB), instead of the 32MB
it was intended to represent.

Change value so it is a select option based on in menuconfig
based on what you want coreboot to "discover" on boot.

Current three options are :
128MB
256MB
512MB

NOTE:  You must specify greater than what is chosen from
the above items on the qemu cmdline.

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
M src/mainboard/emulation/qemu-riscv/Kconfig
1 file changed, 27 insertions(+), 1 deletion(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/3
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 3
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-MessageType: newpatchset

Change in coreboot[master]: BUG#254:qemu/riscv Fixed configuration for DDR Size discovered

[Marc Karasek (Code Review)]

Hello ron minnich, build bot (Jenkins), Philipp Hug, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/38904

to look at the new patch set (#4).

Change subject: BUG#254:qemu/riscv Fixed configuration for DDR Size discovered
......................................................................

BUG#254:qemu/riscv Fixed configuration for DDR Size discovered

For qemu-riscv value was incorrect.  It was set to 32768 which
in turn was being used as 32768MB (32+GB), instead of the 32MB
it was intended to represent.

Change value so it is a select option based on in menuconfig
based on what you want coreboot to "discover" on boot.

Current three options are :
128MB
256MB
512MB

NOTE:  You must specify greater than what is chosen from
the above items on the qemu cmdline.

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
M src/mainboard/emulation/qemu-riscv/Kconfig
1 file changed, 27 insertions(+), 1 deletion(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/4
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 4
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-MessageType: newpatchset

Change in coreboot[master]: BUG#254 : Fixed configuration for DDR Size discovered by coreboot

[Marc Karasek (Code Review)]

Hello ron minnich, build bot (Jenkins), Philipp Hug, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/38904

to look at the new patch set (#5).

Change subject: BUG#254 : Fixed configuration for DDR Size discovered by coreboot
......................................................................

BUG#254 : Fixed configuration for DDR Size discovered by coreboot

For qemu-riscv value was incorrect.  It was set to 32768 which
in turn was being used as 32768MB (32+GB), instead of the 32MB
it was intended to represent.

Change value so it is a select option based in menuconfig
based on what you want coreboot to "discover" on boot.

Four options are :
256MB
512MB
1024MB
2048MB

NOTE:  You must specify greater than or equal to what is
chosen from the above items on the qemu cmdline.

This value is used in both romstage and ramstage to set the amount
of DDR present.  This will be the above value minus the size of the
boot rom.

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
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, 37 insertions(+), 4 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/5
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 5
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-MessageType: newpatchset

Change in coreboot[master]: BUG#254 : Fixed configuration for DDR Size discovered by coreboot

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38904 )

Change subject: BUG#254 : Fixed configuration for DDR Size discovered by coreboot
......................................................................


Patch Set 5: Code-Review+1

(8 comments)

Welcome to coreboot! Thank you for reporting and fixing the issue.

https://review.coreboot.org/c/coreboot/+/38904/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38904/5//COMMIT_MSG@7 
PS5, Line 7: BUG#254 : Fixed configuration for DDR Size discovered by coreboot
Thank you. Please use something like below as commit message summary.
> qemu-riscv: Fix discovered DDR size

https://review.coreboot.org/c/coreboot/+/38904/5//COMMIT_MSG@9 
PS5, Line 9: For qemu-riscv value was incorrect.  It was set to 32768 which
           : in turn was being used as 32768MB (32+GB), instead of the 32MB
           : it was intended to represent.
You could use present tense.


https://review.coreboot.org/c/coreboot/+/38904/5//COMMIT_MSG@16 
PS5, Line 16: are :
Please remove the space before the colon.


https://review.coreboot.org/c/coreboot/+/38904/5//COMMIT_MSG@28 
PS5, Line 28: 
Please add:
> Fixes: https://ticket.coreboot.org/issues/254

Also:
> TEST=…

with the command you tested this with.


https://review.coreboot.org/c/coreboot/+/38904/5/src/mainboard/emulation/qem... 
File src/mainboard/emulation/qemu-riscv/Kconfig:

https://review.coreboot.org/c/coreboot/+/38904/5/src/mainboard/emulation/qem... 
PS5, Line 62: 	  on the qemu cmdline.
Please move this on the line above.


https://review.coreboot.org/c/coreboot/+/38904/5/src/mainboard/emulation/qem... 
PS5, Line 67: 	    Choose this option if you want coreboot to find 256MB DDR
Please add a dot/period at the end.


https://review.coreboot.org/c/coreboot/+/38904/5/src/mainboard/emulation/qem... 
File src/mainboard/emulation/qemu-riscv/mainboard.c:

https://review.coreboot.org/c/coreboot/+/38904/5/src/mainboard/emulation/qem... 
PS5, Line 30: 1024
KiB


https://review.coreboot.org/c/coreboot/+/38904/5/src/soc/ucb/riscv/cbmem.c 
File src/soc/ucb/riscv/cbmem.c:

https://review.coreboot.org/c/coreboot/+/38904/5/src/soc/ucb/riscv/cbmem.c@2... 
PS5, Line 20: 1024
KiB



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 5
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Paul Menzel <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-Comment-Date: Mon, 17 Feb 2020 15:08:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in coreboot[master]: qemu-riscv: Fix discovered DDR size

[Marc Karasek (Code Review)]

Hello ron minnich, Paul Menzel, build bot (Jenkins), Philipp Hug, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/38904

to look at the new patch set (#6).

Change subject: qemu-riscv: Fix discovered DDR size
......................................................................

qemu-riscv: Fix discovered DDR size

qemu-riscv value for DDR is incorrect.  It was set to 32768 which
in turn is being used as 32768MB (32+GB), instead of the 32MB
it is intended to represent.

Change value so it is a select option based in menuconfig
based on what you want coreboot to "discover" on boot.

Four options are :
256MB
512MB
1024MB
2048MB

NOTE:  You must specify greater than or equal to what is
chosen from the above items on the qemu cmdline.

This value is used in both romstage and ramstage to set the amount
of DDR present.  This will be the above value minus the size of the
boot rom.

Fixes: https://ticket.coreboot.org/issues/254

TEST = qemu-system-riscv64 -M virt -m 1024M -nographic \
	-bios build/coreboot.rom

TEST = qemu-system-riscv64 -M virt -m 1024M -nographic \
	-kernel build/coreboot.elf

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
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, 36 insertions(+), 4 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/6
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 6
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Paul Menzel <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-MessageType: newpatchset

Change in coreboot[master]: qemu-riscv: Fix discovered DDR size

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38904 )

Change subject: qemu-riscv: Fix discovered DDR size
......................................................................


Patch Set 6:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38904/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38904/6//COMMIT_MSG@9 
PS6, Line 9: qemu-riscv value for DDR is incorrect.  It was set to 32768 which
that's not true. it was always intended to cover 32GiB.


https://review.coreboot.org/c/coreboot/+/38904/6//COMMIT_MSG@14 
PS6, Line 14: based on what you want coreboot to "discover" on boot.
it doesn't discover, you are hardcoding values


https://review.coreboot.org/c/coreboot/+/38904/6//COMMIT_MSG@23 
PS6, Line 23: chosen from the above items on the qemu cmdline.
Please also update the documentation.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 6
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Paul Menzel <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-CC: Patrick Rudolph <[e-mail]>
Gerrit-Comment-Date: Tue, 18 Feb 2020 07:42:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in coreboot[master]: qemu-riscv: Fix 'Discovered' DDR size

[Marc Karasek (Code Review)]

Hello ron minnich, Paul Menzel, build bot (Jenkins), Philipp Hug, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/38904

to look at the new patch set (#7).

Change subject: qemu-riscv: Fix 'Discovered' DDR size
......................................................................

qemu-riscv: Fix 'Discovered' DDR size

qemu-riscv value for DDR is not correct in how it was being used in code.
It was set to 32768 which in turn is being used as 32768MB (32+GB).  This
is causing an expection when trying to access 32GiB boundary when only 1GiB
is present in the system

Change value so it is a select option based in menuconfig
based on what you want coreboot to use on boot.

Four options are :
256MB
512MB
1024MB
2048MB

NOTE:  You must specify greater than or equal to what is
chosen from the above items on the qemu cmdline.

This value is used in both romstage and ramstage to set the amount
of DDR present.  This will be the above value minus the size of the
boot rom.

Updated Documentation for above change
Fixes: https://ticket.coreboot.org/issues/254

TEST = qemu-system-riscv64 -M virt -m 1024M -nographic \
	-bios build/coreboot.rom

TEST = qemu-system-riscv64 -M virt -m 1024M -nographic \
	-kernel build/coreboot.elf

Signed-off-by: Marc Karasek <[e-mail]>
Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
---
M Documentation/mainboard/emulation/qemu-riscv.md
M src/mainboard/emulation/qemu-riscv/Kconfig
M src/mainboard/emulation/qemu-riscv/mainboard.c
M src/soc/ucb/riscv/cbmem.c
4 files changed, 47 insertions(+), 6 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38904/7
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 7
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Paul Menzel <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: ron minnich <[e-mail]>
Gerrit-CC: Patrick Rudolph <[e-mail]>
Gerrit-MessageType: newpatchset

[M] Change in coreboot[master]: qemu-riscv: Fix 'Discovered' DDR size

[Stefan Reinauer (Code Review)]

Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38904?usp=email )

Change subject: qemu-riscv: Fix 'Discovered' DDR size
......................................................................


Abandoned
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38904?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idef44eb8baf3e89d7c74fa452fc60b7beefa2c48
Gerrit-Change-Number: 38904
Gerrit-PatchSet: 7
Gerrit-Owner: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Marc Karasek <[e-mail]>
Gerrit-Reviewer: Paul Menzel <[e-mail]>
Gerrit-Reviewer: Philipp Hug <[e-mail]>
Gerrit-Reviewer: Ron Minnich <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Patrick Rudolph <[e-mail]>
Gerrit-MessageType: abandon