Asami Doi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33387 )
Change subject: mainboard/emulation/qemu-aarch64: Add new board for ARMv8 ......................................................................
Patch Set 11:
(8 comments)
This change is ready for review.
https://review.coreboot.org/#/c/33387/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33387/11//COMMIT_MSG@9 PS11, Line 9: WIP
Still a WIP?
Sorry, this is still WIP. The state changed to ready automatically when I replied.
https://review.coreboot.org/#/c/33387/11/src/arch/arm64/armv8/cache.c File src/arch/arm64/armv8/cache.c:
https://review.coreboot.org/#/c/33387/11/src/arch/arm64/armv8/cache.c@a42 PS11, Line 42:
I don't see any functional changes to this method (other than removing the static). […]
The result of this function doesn't change.
The reason why I changed this function is that the "static unsigned int" should be stored the BSS(or DATA) region but the current implementation is read-only and can't use a heap region.
In my understanding, we have 2 options. 1. Not use global variables in the bootblock. In this case, I read the code from the ROM directly and I can't use a heap region. 2. Relocate the bootblock code to DRAM. In this case, I can use a heap region but I need to write a relocating code in bootblock_custom.S.
This change tried to do the first solution.
Patrick suggested the second solution. ( https://review.coreboot.org/c/coreboot/+/33387/6/src/mainboard/emulation/qem... )
I personally want to do the first solution because it's simpler than the second one. What do you think?
https://review.coreboot.org/#/c/33387/11/src/cpu/armltd/cortex-a53/Kconfig File src/cpu/armltd/cortex-a53/Kconfig:
https://review.coreboot.org/#/c/33387/11/src/cpu/armltd/cortex-a53/Kconfig@8 PS11, Line 8: CPU_ARMLTD_CORTEX_A53
Remove the the empty branch.
Ack
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... File src/mainboard/emulation/qemu-aarch64/bootblock_custom.S:
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... PS11, Line 6: * SPDX-License-Identifier: GPL-2.0-or-later
I think coreboot is typically a GPL-2.0-only project? […]
According to a lint checker, coreboot seems to support a GPL-2.0-or-later. I got lint errors when I used GPL-2.0-only.
https://github.com/coreboot/coreboot/blob/master/util/lint/lint-000-license-...
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... PS11, Line 20: EL1
I thought you were able to run this in EL3?
Yes, I could run QEMU in EL3. I will remove this function.
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... File src/mainboard/emulation/qemu-aarch64/cbmem.c:
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... PS11, Line 21: 0x60000000
#define this constant and all the other constants.
Ack
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... File src/mainboard/emulation/qemu-aarch64/mainboard.c:
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... PS11, Line 24: 0x4c000000
#define register addresses
Ack
https://review.coreboot.org/#/c/33387/11/src/mainboard/emulation/qemu-aarch6... PS11, Line 31: 4
#define regiser offsets
I'm still working on this file so I will do it later. Thank you for your advice.