Philipp Hug has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
WIP: riscv/mb/qemu: fix DRAM probing
Current version of qemu raise an exception when accessing invalid memory. Modify the probing code to temporary redirect the exception handler like on ARM platform.
TEST=qemu detects RAM size correctly Change-Id: I25860f688c7546714f6fdbce8c8f96da6400813c Signed-off-by: Philipp Hug philipp@hug.cx --- M src/arch/riscv/Makefile.inc M src/arch/riscv/include/arch/exception.h A src/arch/riscv/ramdetect.c M src/arch/riscv/trap_handler.c M src/arch/riscv/trap_util.S M src/lib/ramdetect.c 6 files changed, 57 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/36486/1
diff --git a/src/arch/riscv/Makefile.inc b/src/arch/riscv/Makefile.inc index 0039fab..7465a8f 100644 --- a/src/arch/riscv/Makefile.inc +++ b/src/arch/riscv/Makefile.inc @@ -101,6 +101,7 @@ romstage-y += stages.c romstage-y += misc.c romstage-$(ARCH_RISCV_PMP) += pmp.c +romstage-y += ramdetect.c romstage-y += smp.c romstage-y += \ $(top)/src/lib/memchr.c \ @@ -142,6 +143,7 @@ ramstage-y += virtual_memory.c ramstage-y += stages.c ramstage-y += misc.c +ramstage-y += ramdetect.c ramstage-y += smp.c ramstage-y += boot.c ramstage-y += tables.c diff --git a/src/arch/riscv/include/arch/exception.h b/src/arch/riscv/include/arch/exception.h index 6fbbdf0..cdca582 100644 --- a/src/arch/riscv/include/arch/exception.h +++ b/src/arch/riscv/include/arch/exception.h @@ -53,7 +53,7 @@ }
void redirect_trap(void); -void trap_handler(trapframe *tf); +void default_trap_handler(trapframe *tf); void handle_supervisor_call(trapframe *tf); void handle_misaligned(trapframe *tf);
diff --git a/src/arch/riscv/ramdetect.c b/src/arch/riscv/ramdetect.c new file mode 100644 index 0000000..47153ab --- /dev/null +++ b/src/arch/riscv/ramdetect.c @@ -0,0 +1,46 @@ +/* + * This file is part of the coreboot project. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <arch/exception.h> +#include <types.h> +#include <console/console.h> +#include <device/mmio.h> +#include <ramdetect.h> +#include <arch/smp/spinlock.h> + +static enum { + ABORT_CHECKER_NOT_TRIGGERED, + ABORT_CHECKER_TRIGGERED, +} abort_state = ABORT_CHECKER_NOT_TRIGGERED; + +extern void(*trap_handler)(trapframe *tf); + +#define insn_size 4 +static void ramcheck_trap_handler(trapframe *tf) +{ + printk(BIOS_DEBUG, "TRAP 0x%lx!!!\n", tf->epc); + abort_state = ABORT_CHECKER_TRIGGERED; + + /* + * skip read instruction. + * currenctly hardcoded to 32bit instruction size + */ + write_csr(mepc, read_csr(mepc) + insn_size); +} + +int probe_mb(const uintptr_t dram_start, const uintptr_t size) +{ + uintptr_t addr = dram_start + (size * MiB) - sizeof(uint32_t); + void *ptr = (void *)addr; + + abort_state = ABORT_CHECKER_NOT_TRIGGERED; + trap_handler = ramcheck_trap_handler; + barrier(); + read32(ptr); + trap_handler = default_trap_handler; + barrier(); + return abort_state == ABORT_CHECKER_NOT_TRIGGERED; +} diff --git a/src/arch/riscv/trap_handler.c b/src/arch/riscv/trap_handler.c index 6b39fab..1d51268 100644 --- a/src/arch/riscv/trap_handler.c +++ b/src/arch/riscv/trap_handler.c @@ -118,7 +118,10 @@ break; } } -void trap_handler(trapframe *tf) + +void(*trap_handler)(trapframe *tf) = default_trap_handler; + +void default_trap_handler(trapframe *tf) { write_csr(mscratch, tf); if (tf->cause & 0x8000000000000000ULL) { diff --git a/src/arch/riscv/trap_util.S b/src/arch/riscv/trap_util.S index 67e917c..93f5afd 100644 --- a/src/arch/riscv/trap_util.S +++ b/src/arch/riscv/trap_util.S @@ -120,7 +120,8 @@ save_tf
mv a0, sp - jal trap_handler + ld t0, trap_handler + jalr t0
restore_regs addi sp, sp, MENTRY_FRAME_SIZE diff --git a/src/lib/ramdetect.c b/src/lib/ramdetect.c index 2c83092..eaf6190 100644 --- a/src/lib/ramdetect.c +++ b/src/lib/ramdetect.c @@ -58,6 +58,8 @@ if (saved_result) return saved_result;
+ printk(BIOS_DEBUG, "RAMDETECT: Starting\n"); + /* Find the MSB + 1. */ size_t tmp = probe_size; do {
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36486/1/src/arch/riscv/ramdetect.c File src/arch/riscv/ramdetect.c:
https://review.coreboot.org/c/coreboot/+/36486/1/src/arch/riscv/ramdetect.c@... PS1, Line 19: extern void(*trap_handler)(trapframe *tf); missing space after return type
https://review.coreboot.org/c/coreboot/+/36486/1/src/arch/riscv/trap_handler... File src/arch/riscv/trap_handler.c:
https://review.coreboot.org/c/coreboot/+/36486/1/src/arch/riscv/trap_handler... PS1, Line 122: void(*trap_handler)(trapframe *tf) = default_trap_handler; missing space after return type
Hello ron minnich, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36486
to look at the new patch set (#2).
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
WIP: riscv/mb/qemu: fix DRAM probing
Current version of qemu raise an exception when accessing invalid memory. Modify the probing code to temporary redirect the exception handler like on ARM platform.
TEST=qemu detects RAM size correctly Change-Id: I25860f688c7546714f6fdbce8c8f96da6400813c Signed-off-by: Philipp Hug philipp@hug.cx --- M src/arch/riscv/Makefile.inc M src/arch/riscv/include/arch/exception.h A src/arch/riscv/ramdetect.c M src/arch/riscv/trap_handler.c M src/arch/riscv/trap_util.S M src/lib/ramdetect.c 6 files changed, 57 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/36486/2
Marc Karasek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 2: Code-Review-1
This still relies on the Kconfig value matching what is on the cmdline for qemu
https://review.coreboot.org/c/coreboot/+/38904 allows you to specify the DDR size so at least you can match what the qemu line with the coreboot code. Not dynamic but better than nothing
Marc Karasek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 2:
How did you get this to compile? There is no trap_handler.o in romstage and if you add it it fails to build. handle_sbi is undefined..
Marc Karasek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 2:
We do not need to scan when using qemu. The ddr size is known and can be a fixed value.
See https://review.coreboot.org/c/coreboot/+/38904
This makes much more sense to have an option to compile coreboot for qemu-riscv with the value you want to use then specify this (or a smaller value) on the cmdline. No need to muck with exception handlers, etc..
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 2:
It looks like there's a bug in qemu as mepc seems to have no influence on pc on mret. @Philipp any idea what could be wrong?
Attention is currently required from: Philipp Hug. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: I tried rebasing this on master but it now fails to update the trap handler. With the same compiler this still works. I'm at a loss here... Do you have any idea?
Attention is currently required from: Philipp Hug. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: WIP: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
I tried rebasing this on master but it now fails to update the trap handler. With the same compiler this still works. I'm at a loss here... Do you have any idea?
nvm. It looks like the exception handler does not get installed in romstage, so it's using the bootblock one...
Attention is currently required from: Philipp Hug, Arthur Heymans.
Arthur Heymans has uploaded a new patch set (#3) to the change originally created by Philipp Hug. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: riscv/mb/qemu: fix DRAM probing ......................................................................
riscv/mb/qemu: fix DRAM probing
Current version of qemu raise an exception when accessing invalid memory. Modify the probing code to temporary redirect the exception handler like on ARM platform.
TEST=qemu detects RAM size correctly Change-Id: I25860f688c7546714f6fdbce8c8f96da6400813c Signed-off-by: Philipp Hug philipp@hug.cx Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/riscv/Makefile.inc M src/arch/riscv/include/arch/exception.h A src/arch/riscv/ramdetect.c M src/arch/riscv/trap_handler.c M src/arch/riscv/trap_util.S M src/lib/imd_cbmem.c M src/lib/ramdetect.c 7 files changed, 75 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/36486/3
Attention is currently required from: Philipp Hug, Arthur Heymans.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36486 )
Change subject: riscv/mb/qemu: fix DRAM probing ......................................................................
Patch Set 3:
(5 comments)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161259): https://review.coreboot.org/c/coreboot/+/36486/comment/992a9596_c916d1b2 PS3, Line 8: Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161259): https://review.coreboot.org/c/coreboot/+/36486/comment/fc078581_033e0937 PS3, Line 9: Current version of qemu raise an exception when accessing invalid memory. Possible unwrapped commit description (prefer a maximum 72 chars per line)
File src/arch/riscv/ramdetect.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161259): https://review.coreboot.org/c/coreboot/+/36486/comment/3068a977_0967d5e8 PS3, Line 19: extern void(*trap_handler)(trapframe *tf); missing space after return type
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161259): https://review.coreboot.org/c/coreboot/+/36486/comment/9c1e1bd2_ee9d0878 PS3, Line 44: printk(BIOS_DEBUG, "%lx is %s DRAM\n", dram_start + size * MiB, abort_state == ABORT_CHECKER_NOT_TRIGGERED ? "" : "not"); line length of 129 exceeds 96 columns
File src/arch/riscv/trap_handler.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161259): https://review.coreboot.org/c/coreboot/+/36486/comment/3a3ad6d2_7f9dadc9 PS3, Line 112: void(*trap_handler)(trapframe *tf) = default_trap_handler; missing space after return type
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Marc Karasek (Code Review)
-
Patrick Rudolph (Code Review)
-
Philipp Hug (Code Review)