Change in coreboot[master]: mb/emulation/qemu-armv7: Add MMU support

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44394 ) Change subject: mb/emulation/qemu-armv7: Add MMU support ...................................................................... mb/emulation/qemu-armv7: Add MMU support Extend the memlayout and configure the MMU in the bootblock. While there's no real benefit on emulated machines, it alignes the qemu mainboard with real hardware. Tested using uImage/FIT. Still able to boot into GNU/Linux 5.5. Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Signed-off-by: Patrick Rudolph <siro@das-labor.org> --- M src/mainboard/emulation/qemu-armv7/Makefile.inc A src/mainboard/emulation/qemu-armv7/bootblock.c M src/mainboard/emulation/qemu-armv7/memlayout.ld 3 files changed, 54 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44394/1 diff --git a/src/mainboard/emulation/qemu-armv7/Makefile.inc b/src/mainboard/emulation/qemu-armv7/Makefile.inc index 65fdfe1..1fcbef9 100644 --- a/src/mainboard/emulation/qemu-armv7/Makefile.inc +++ b/src/mainboard/emulation/qemu-armv7/Makefile.inc @@ -4,6 +4,8 @@ romstage-y += cbmem.c +bootblock-y += bootblock.c + bootblock-y += media.c romstage-y += media.c ramstage-y += media.c diff --git a/src/mainboard/emulation/qemu-armv7/bootblock.c b/src/mainboard/emulation/qemu-armv7/bootblock.c new file mode 100644 index 0000000..0b011e1 --- /dev/null +++ b/src/mainboard/emulation/qemu-armv7/bootblock.c @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/cache.h> +#include <bootblock_common.h> +#include <symbols.h> + +void bootblock_soc_init(void) +{ + mmu_init(); + +#define MMU_CONF_SYMBOL(x, y) extern u8 _##x[]; extern u8 _e##x[];\ + if (REGION_SIZE(x) < MiB) { \ + mmu_config_range_kb(((uintptr_t)(_##x))/KiB, REGION_SIZE(x)/KiB, (y)); \ + } else { \ + mmu_config_range(((uintptr_t)(_##x))/MiB, REGION_SIZE(x)/MiB, (y)); \ + } + MMU_CONF_SYMBOL(flash, DCACHE_OFF) + MMU_CONF_SYMBOL(sysregs_uart_timer, DCACHE_OFF) + MMU_CONF_SYMBOL(pl111, DCACHE_OFF) + MMU_CONF_SYMBOL(sp805, DCACHE_OFF) + MMU_CONF_SYMBOL(nor_flash1, DCACHE_OFF) + MMU_CONF_SYMBOL(nor_flash2, DCACHE_OFF) + MMU_CONF_SYMBOL(videoram, DCACHE_OFF) + MMU_CONF_SYMBOL(ethernet, DCACHE_OFF) + MMU_CONF_SYMBOL(usb, DCACHE_OFF) +#undef MMU_CONF_SYMBOL + + mmu_config_range(((uintptr_t)_sram)/MiB, + REGION_SIZE(sram)/MiB, DCACHE_WRITEBACK); + /* Maximum supported DRAM size is 1 GiB, use 2 GiB for ramdetect */ + mmu_config_range(((uintptr_t)_dram)/MiB, 2*(GiB/MiB), DCACHE_WRITEBACK); + + dcache_mmu_enable(); +} + diff --git a/src/mainboard/emulation/qemu-armv7/memlayout.ld b/src/mainboard/emulation/qemu-armv7/memlayout.ld index 4ddc6d2..b0408ed 100644 --- a/src/mainboard/emulation/qemu-armv7/memlayout.ld +++ b/src/mainboard/emulation/qemu-armv7/memlayout.ld @@ -5,15 +5,6 @@ #include <arch/header.ld> /* - * Memory map for qemu vexpress-a9 since - * 6ec1588e09770ac7e9c60194faff6101111fc7f0 (Jul 2014): - * - * 0x0000_0000: NOR flash - * 0x1000_0000: I/O map address - * 0x6000_0000: RAM - */ - -/* * This map is designed to work with new qemu vexpress memory layout and * with -bios option which neatly puts coreboot into flash and so payloads * can find CBFS and we don't risk overwriting CBFS. @@ -26,18 +17,30 @@ */ SECTIONS { - /* TODO: does this thing emulate SRAM? */ - + /* + * Memory map for qemu vexpress-a9 since + * 6ec1588e09770ac7e9c60194faff6101111fc7f0 (Jul 2014): + */ REGION(flash, 0, CONFIG_ROM_SIZE, 4K) + REGION(sysregs_uart_timer, 0x10000000, 0x20000, 4K) + REGION(pl111, 0x10020000, 0x40000, 4K) + REGION(sp805, 0x100e4000, 0x1000, 4K) + REGION(nor_flash1, 0x40000000, 0x4000000, 4K) + REGION(nor_flash2, 0x44000000, 0x4000000, 4K) + SRAM_START(0x48000000) + SRAM_END(0x4C000000) + REGION(videoram, 0x4c000000, 0x1000000, 4K) + REGION(ethernet, 0x4e000000, 0x1000000, 4K) + REGION(usb, 0x4f000000, 0x1000000, 4K) DRAM_START(0x60000000) STACK(0x60000000, 64K) BOOTBLOCK(0x60010000, 128K) FMAP_CACHE(0x60030000, 4K) TIMESTAMP(0x60031000, 1K) - /* TODO: Implement MMU support and move TTB to a better location. */ - TTB(0x60034000, 16K) - ROMSTAGE(0x60038000, 128K) + TTB_SUBTABLES(0x60032000, 4K) + TTB(0x60034000, 32K) + ROMSTAGE(0x60040000, 128K) RAMSTAGE(0x60060000, 16M) POSTRAM_CBFS_CACHE(0x61060000, 8M) } -- To view, visit https://review.coreboot.org/c/coreboot/+/44394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Gerrit-Change-Number: 44394 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: newchange

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44394 to look at the new patch set (#2). Change subject: mb/emulation/qemu-armv7: Add MMU support ...................................................................... mb/emulation/qemu-armv7: Add MMU support Extend the memlayout and configure the MMU in the bootblock. While there's no real benefit on emulated machines, it alignes the qemu mainboard with real hardware. Tested using uImage/FIT. Still able to boot into GNU/Linux 5.5. Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Signed-off-by: Patrick Rudolph <siro@das-labor.org> --- M src/mainboard/emulation/qemu-armv7/Makefile.inc A src/mainboard/emulation/qemu-armv7/bootblock.c M src/mainboard/emulation/qemu-armv7/memlayout.ld 3 files changed, 53 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44394/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/44394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Gerrit-Change-Number: 44394 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44394 ) Change subject: mb/emulation/qemu-armv7: Add MMU support ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-armv7/bootblock.c: https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... PS2, Line 17: MMU_CONF_SYMBOL(flash, DCACHE_OFF) btw, if you want you could implement a DCACHE_READONLY access type for flash regions like this (which caches reads but aborts on a write). The page table format would support it. https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... PS2, Line 25: MMU_CONF_SYMBOL(usb, DCACHE_OFF) Why not just follow what other Arm SoCs do (mmu_config_range(0, 4096, DCACHE_OFF))? Trying to separate out every single MMIO device seems very messy (and this is just an emulator, normally you would have more devices) and there's honestly no real benefit. It can be nice to block off the first page if that's possible (so you'd write (1, 4096, DCACHE_OFF)) to catch null pointer accesses, but that doesn't work here because you have your flash there... and for everything else, I think the chance that you can catch a rare pointer arithmetic bug that just happens to hit one of your unused parts isn't high enough to warrant the complexity. Also, the mmu_config_range_kb() thing should better only be used where you *really* need it, because it is further restricted by the size of TTB_SUBTABLES. -- To view, visit https://review.coreboot.org/c/coreboot/+/44394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Gerrit-Change-Number: 44394 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 13 Aug 2020 01:40:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44394 ) Change subject: mb/emulation/qemu-armv7: Add MMU support ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-armv7/bootblock.c: https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... PS2, Line 25: MMU_CONF_SYMBOL(usb, DCACHE_OFF)
Why not just follow what other Arm SoCs do (mmu_config_range(0, 4096, DCACHE_OFF))? Trying to separa […] the benefit of having multiple IO regions is that you don't need to define the MMIO address again, you could use the symbol.
-- To view, visit https://review.coreboot.org/c/coreboot/+/44394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Gerrit-Change-Number: 44394 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 17 Aug 2020 17:01:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner@chromium.org> Gerrit-MessageType: comment

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44394 ) Change subject: mb/emulation/qemu-armv7: Add MMU support ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-armv7/bootblock.c: https://review.coreboot.org/c/coreboot/+/44394/2/src/mainboard/emulation/qem... PS2, Line 25: MMU_CONF_SYMBOL(usb, DCACHE_OFF)
the benefit of having multiple IO regions is that you don't need to define the MMIO address again, y […] I mean... it's just not what we've done on all the other platforms, honestly. We usually just have a <soc/addressmap.h> header to #define all those base addresses. This may have some performance advantage because when a function accesses a specific register the compiler can compute (base address + offset) at compile time, whereas memlayout symbols can only be resolved at runtime.
I would suggest to just follow the pattern used by the other platforms for consistency. -- To view, visit https://review.coreboot.org/c/coreboot/+/44394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Gerrit-Change-Number: 44394 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 17 Aug 2020 23:22:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner@chromium.org> Comment-In-Reply-To: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: comment

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44394?usp=email ) Change subject: mb/emulation/qemu-armv7: Add MMU support ...................................................................... Abandoned This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author. -- To view, visit https://review.coreboot.org/c/coreboot/+/44394?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: Ic5a22bc8670dd11aa7bd05b88740c4f07bad3108 Gerrit-Change-Number: 44394 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: abandon
participants (3)
-
Julius Werner (Code Review)
-
Martin L Roth (Code Review)
-
Patrick Rudolph (Code Review)