[U-Boot] [PATCH 00/30] General fixes / cleanup for RISC-V and improvements to qemu-riscv

This patch series includes general fixes and cleanup for RISC-V. It also adds support for booting Linux on qemu-riscv. At the moment, only single-core systems are supported. Support for multi-core systems will be added with a future patch series.
To boot Linux on qemu-riscv, Linux must be compiled into BBL as a payload. BBL must be included in a FIT image and supplied to QEMU with the -kernel parameter. Its location in memory is embedded in the device tree, which QEMU passes to u-boot. To test this, QEMU and riscv-pk (BBL) must be modified. QEMU is modified to add support for loading binary files (FIT images in this case) in addition to ELF files. riscv-pk must be modified to adjust the link address and to ignore the kernel address from the device tree. A pull request for QEMU, which implements this, is available at [1]. A modified version of riscv-pk is available at [2].
[1]: https://github.com/riscv/riscv-qemu/pull/175 [2]: https://github.com/lukasauer/riscv-pk/tree/riscv-u-boot
Lukas Auer (30): tools: .gitignore: add prelink-riscv riscv: ignore device tree binaries dts: riscv: update makefile to also clean the RISC-V dts directory riscv: rename CPU_RISCV_32/64 to match architecture names ARCH_RV32I/64I riscv: select CONFIG_PHYS_64BIT on RV64I systems riscv: add Kconfig entries for the C and A ISA extensions riscv: set -march and -mabi based on the Kconfig configuration riscv: add Kconfig entries for the code model riscv: move target selection into separate file riscv: enable -fdata-sections riscv: fix use of incorrectly sized variables riscv: make use of the barrier functions from Linux riscv: do not reimplement generic io functions riscv: complete the list of exception codes riscv: treat undefined exception codes as reserved riscv: hang on unhandled exceptions riscv: implement the invalidate_icache_* functions riscv: invalidate the instruction cache before jumping to Linux riscv: fix inconsistent use of spaces and tabs in start.S riscv: align mtvec on a 4-byte boundary riscv: remove CONFIG_INIT_CRITICAL riscv: remove unused labels in start.S riscv: do not blindly modify the mstatus CSR riscv: save hart ID and device tree passed by prior boot stage riscv: qemu: use device tree passed by prior boot stage bdinfo: riscv: print fdt_blob address riscv: qemu: support booting Linux riscv: align bootm implementation with that of other architectures dm: core: add missing prototype for ofnode_read_u64 riscv: qemu: detect and boot the kernel passed by QEMU
arch/riscv/Kconfig | 54 ++-- arch/riscv/Kconfig.board | 14 + arch/riscv/Makefile | 16 ++ arch/riscv/config.mk | 7 +- arch/riscv/cpu/cpu.c | 6 + arch/riscv/cpu/start.S | 339 +++++++++++------------- arch/riscv/dts/.gitignore | 1 + arch/riscv/include/asm/barrier.h | 67 +++++ arch/riscv/include/asm/io.h | 48 +--- arch/riscv/include/asm/posix_types.h | 6 +- arch/riscv/include/asm/types.h | 4 + arch/riscv/lib/bootm.c | 93 +++++-- arch/riscv/lib/cache.c | 10 + arch/riscv/lib/interrupts.c | 31 ++- arch/riscv/lib/setjmp.S | 2 +- board/emulation/qemu-riscv/Kconfig | 1 + board/emulation/qemu-riscv/qemu-riscv.c | 35 ++- cmd/bdinfo.c | 2 + configs/ax25-ae350_defconfig | 2 +- configs/qemu-riscv32_defconfig | 4 +- configs/qemu-riscv64_defconfig | 6 +- dts/Makefile | 2 +- include/config_distro_bootcmd.h | 8 +- include/configs/qemu-riscv.h | 13 + include/dm/ofnode.h | 10 + tools/.gitignore | 1 + 26 files changed, 494 insertions(+), 288 deletions(-) create mode 100644 arch/riscv/Kconfig.board create mode 100644 arch/riscv/dts/.gitignore create mode 100644 arch/riscv/include/asm/barrier.h

Ignore tools/prelink-riscv.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
tools/.gitignore | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/.gitignore b/tools/.gitignore index c8cdaef90c..e5ede22842 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -24,6 +24,7 @@ /mksunxiboot /mxsboot /ncb +/prelink-riscv /proftool /relocate-rela /sunxi-spl-image-builder

On Sat, Oct 20, 2018 at 6:08 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Ignore tools/prelink-riscv.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
tools/.gitignore | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Ignore all device tree binaries in arch/riscv/dts.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/dts/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 arch/riscv/dts/.gitignore
diff --git a/arch/riscv/dts/.gitignore b/arch/riscv/dts/.gitignore new file mode 100644 index 0000000000..b60ed208c7 --- /dev/null +++ b/arch/riscv/dts/.gitignore @@ -0,0 +1 @@ +*.dtb

Hi Lukas,
On Sat, Oct 20, 2018 at 6:08 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Ignore all device tree binaries in arch/riscv/dts.
I don't think this patch is necessary.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/dts/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 arch/riscv/dts/.gitignore
Regards, Bin

Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
dts/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dts/Makefile b/dts/Makefile index 9a9a3d5c98..cd6e9a968e 100644 --- a/dts/Makefile +++ b/dts/Makefile @@ -61,4 +61,4 @@ dtbs: $(obj)/dt.dtb $(obj)/dt-spl.dtb clean-files := dt.dtb.S dt-spl.dtb.S
# Let clean descend into dts directories -subdir- += ../arch/arm/dts ../arch/microblaze/dts ../arch/mips/dts ../arch/sandbox/dts ../arch/x86/dts ../arch/powerpc/dts +subdir- += ../arch/arm/dts ../arch/microblaze/dts ../arch/mips/dts ../arch/sandbox/dts ../arch/x86/dts ../arch/powerpc/dts ../arch/riscv/dts

On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
dts/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

RISC-V defines the base integer instruction sets as RV32I and RV64I. Rename CPU_RISCV_32 and CPU_RISCV_64 to ARCH_RV32I and ARCH_64I to match this convention.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/Kconfig | 16 ++++++++-------- arch/riscv/lib/setjmp.S | 2 +- configs/ax25-ae350_defconfig | 2 +- configs/qemu-riscv64_defconfig | 2 +- include/config_distro_bootcmd.h | 8 ++++---- 5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 168ca3de7c..7c76b4d664 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -20,20 +20,20 @@ source "board/AndesTech/ax25-ae350/Kconfig" source "board/emulation/qemu-riscv/Kconfig"
choice - prompt "CPU selection" - default CPU_RISCV_32 + prompt "Base ISA" + default ARCH_RV32I
-config CPU_RISCV_32 - bool "RISC-V 32-bit" +config ARCH_RV32I + bool "RV32I" select 32BIT help - Choose this option to build an U-Boot for RISCV32 architecture. + Choose this option to target the RV32I base integer instruction set.
-config CPU_RISCV_64 - bool "RISC-V 64-bit" +config ARCH_RV64I + bool "RV64I" select 64BIT help - Choose this option to build an U-Boot for RISCV64 architecture. + Choose this option to target the RV64I base integer instruction set.
endchoice
diff --git a/arch/riscv/lib/setjmp.S b/arch/riscv/lib/setjmp.S index 8f5a6a23aa..72bc9241f6 100644 --- a/arch/riscv/lib/setjmp.S +++ b/arch/riscv/lib/setjmp.S @@ -6,7 +6,7 @@ #include <config.h> #include <linux/linkage.h>
-#ifdef CONFIG_CPU_RISCV_64 +#ifdef CONFIG_ARCH_RV64I #define STORE_IDX(reg, idx) sd reg, (idx*8)(a0) #define LOAD_IDX(reg, idx) ld reg, (idx*8)(a0) #else diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig index 614ef15dc7..9282f05981 100644 --- a/configs/ax25-ae350_defconfig +++ b/configs/ax25-ae350_defconfig @@ -1,7 +1,7 @@ CONFIG_RISCV=y CONFIG_SYS_TEXT_BASE=0x00000000 CONFIG_TARGET_AX25_AE350=y -CONFIG_CPU_RISCV_64=y +CONFIG_ARCH_RV64I=y CONFIG_DISTRO_DEFAULTS=y CONFIG_NR_DRAM_BANKS=2 CONFIG_FIT=y diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig index d6c1a5d646..60b647efe8 100644 --- a/configs/qemu-riscv64_defconfig +++ b/configs/qemu-riscv64_defconfig @@ -1,6 +1,6 @@ CONFIG_RISCV=y CONFIG_TARGET_QEMU_VIRT=y -CONFIG_CPU_RISCV_64=y +CONFIG_ARCH_RV64I=y CONFIG_NR_DRAM_BANKS=1 CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 373fee78a9..54186efe7b 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -99,9 +99,9 @@ #define BOOTEFI_NAME "bootia32.efi" #elif defined(CONFIG_X86_RUN_64BIT) #define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_CPU_RISCV_32) +#elif defined(CONFIG_ARCH_RV32I) #define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_CPU_RISCV_64) +#elif defined(CONFIG_ARCH_RV64I) #define BOOTEFI_NAME "bootriscv64.efi" #endif #endif @@ -257,10 +257,10 @@ #elif defined(__i386__) #define BOOTENV_EFI_PXE_ARCH "0x6" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00006:UNDI:003000" -#elif defined(CONFIG_CPU_RISCV_32) || ((defined(__riscv) && __riscv_xlen == 32)) +#elif defined(CONFIG_ARCH_RV32I) || ((defined(__riscv) && __riscv_xlen == 32)) #define BOOTENV_EFI_PXE_ARCH "0x19" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00025:UNDI:003000" -#elif defined(CONFIG_CPU_RISCV_64) || ((defined(__riscv) && __riscv_xlen == 64)) +#elif defined(CONFIG_ARCH_RV64I) || ((defined(__riscv) && __riscv_xlen == 64)) #define BOOTENV_EFI_PXE_ARCH "0x1b" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00027:UNDI:003000" #elif defined(CONFIG_SANDBOX)

Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
RISC-V defines the base integer instruction sets as RV32I and RV64I. Rename CPU_RISCV_32 and CPU_RISCV_64 to ARCH_RV32I and ARCH_64I to match
ARCH_RV64I
this convention.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 16 ++++++++-------- arch/riscv/lib/setjmp.S | 2 +- configs/ax25-ae350_defconfig | 2 +- configs/qemu-riscv64_defconfig | 2 +- include/config_distro_bootcmd.h | 8 ++++---- 5 files changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 14:23 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
RISC-V defines the base integer instruction sets as RV32I and RV64I. Rename CPU_RISCV_32 and CPU_RISCV_64 to ARCH_RV32I and ARCH_64I to match
ARCH_RV64I
Fixed in v2.
Thanks, Lukas
this convention.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 16 ++++++++-------- arch/riscv/lib/setjmp.S | 2 +- configs/ax25-ae350_defconfig | 2 +- configs/qemu-riscv64_defconfig | 2 +- include/config_distro_bootcmd.h | 8 ++++---- 5 files changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

CONFIG_PHYS_64BIT should be enabled on RV64I systems. Select it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 7c76b4d664..b81e0d990a 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -32,6 +32,7 @@ config ARCH_RV32I config ARCH_RV64I bool "RV64I" select 64BIT + select PHYS_64BIT help Choose this option to target the RV64I base integer instruction set.

On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
CONFIG_PHYS_64BIT should be enabled on RV64I systems. Select it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Add Kconfig entries for the C (compressed instructions) and A (atomic instructions) ISA extensions. Only the C ISA extension is selectable. This matches the configuration in Linux.
The Kconfig entries are not used yet. A follow-up patch will select the appropriate compiler flags based on the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b81e0d990a..e15329c35e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,15 @@ config ARCH_RV64I
endchoice
+config RISCV_ISA_C + bool "Emit compressed instructions" + default y + help + This enables the compressed instructions ("C") ISA extension. + +config RISCV_ISA_A + def_bool y + config 32BIT bool

Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Add Kconfig entries for the C (compressed instructions) and A (atomic instructions) ISA extensions. Only the C ISA extension is selectable. This matches the configuration in Linux.
The Kconfig entries are not used yet. A follow-up patch will select the appropriate compiler flags based on the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b81e0d990a..e15329c35e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,15 @@ config ARCH_RV64I
endchoice
+config RISCV_ISA_C
bool "Emit compressed instructions"
default y
help
This enables the compressed instructions ("C") ISA extension.
This is a little bit confusing. Can we use Linux kernel's description instead like below?
Adds "C" to the ISA subsets that the toolchain is allowed to emit when building U-Boot, which results in compressed instructions in the U-Boot binary.
+config RISCV_ISA_A
def_bool y
I do not believe U-Boot need to care about 'A' extension. So this can be dropped?
config 32BIT bool
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Add Kconfig entries for the C (compressed instructions) and A (atomic instructions) ISA extensions. Only the C ISA extension is selectable. This matches the configuration in Linux.
The Kconfig entries are not used yet. A follow-up patch will select the appropriate compiler flags based on the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b81e0d990a..e15329c35e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,15 @@ config ARCH_RV64I
endchoice
+config RISCV_ISA_C
bool "Emit compressed instructions"
default y
help
This enables the compressed instructions ("C") ISA
extension.
This is a little bit confusing. Can we use Linux kernel's description instead like below?
Adds "C" to the ISA subsets that the toolchain is allowed
to emit when building U-Boot, which results in compressed instructions in the U-Boot binary.
Sure, I will change it in v2.
+config RISCV_ISA_A
def_bool y
I do not believe U-Boot need to care about 'A' extension. So this can be dropped?
That's right. The only reason it might be used in U-Boot is on multi- core systems. Linux chooses the hart to boot on with the hart lottery, which requires atomics. The first core to increment a flag wins and starts the boot process. We could do something similar in U-Boot, but I don't think it's necessary. In my patches (for the next patch series) I have added CONFIG_MAIN_HART to select the hart U-Boot should run on.
I have included it here so that I can use it in the next patch to build the ISA string. I can remove it, but that would change the current behavior, which has atomics enabled.
Thanks, Lukas
config 32BIT bool
Regards, Bin

Hi Lukas,
On Wed, Oct 24, 2018 at 11:21 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Add Kconfig entries for the C (compressed instructions) and A (atomic instructions) ISA extensions. Only the C ISA extension is selectable. This matches the configuration in Linux.
The Kconfig entries are not used yet. A follow-up patch will select the appropriate compiler flags based on the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b81e0d990a..e15329c35e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,15 @@ config ARCH_RV64I
endchoice
+config RISCV_ISA_C
bool "Emit compressed instructions"
default y
help
This enables the compressed instructions ("C") ISA
extension.
This is a little bit confusing. Can we use Linux kernel's description instead like below?
Adds "C" to the ISA subsets that the toolchain is allowed
to emit when building U-Boot, which results in compressed instructions in the U-Boot binary.
Sure, I will change it in v2.
+config RISCV_ISA_A
def_bool y
I do not believe U-Boot need to care about 'A' extension. So this can be dropped?
That's right. The only reason it might be used in U-Boot is on multi- core systems. Linux chooses the hart to boot on with the hart lottery, which requires atomics. The first core to increment a flag wins and starts the boot process. We could do something similar in U-Boot, but I don't think it's necessary. In my patches (for the next patch series) I have added CONFIG_MAIN_HART to select the hart U-Boot should run on.
Thanks for the additional details. I think we will need CONFIG_MAIN_HART to select the hart U-Boot should run on instead of Linux's lottery game, since we need handle situation like SiFive Freedom U540 where hart0 is something that does not run Linux. But even with CONFIG_MAIN_HART, U-Boot still doesn't need support 'A', right?
I have included it here so that I can use it in the next patch to build the ISA string. I can remove it, but that would change the current behavior, which has atomics enabled.
Regards, Bin

Hi Bin,
On Wed, 2018-10-24 at 23:32 +0800, Bin Meng wrote:
Hi Lukas,
On Wed, Oct 24, 2018 at 11:21 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Add Kconfig entries for the C (compressed instructions) and A (atomic instructions) ISA extensions. Only the C ISA extension is selectable. This matches the configuration in Linux.
The Kconfig entries are not used yet. A follow-up patch will select the appropriate compiler flags based on the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b81e0d990a..e15329c35e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,15 @@ config ARCH_RV64I
endchoice
+config RISCV_ISA_C
bool "Emit compressed instructions"
default y
help
This enables the compressed instructions ("C") ISA
extension.
This is a little bit confusing. Can we use Linux kernel's description instead like below?
Adds "C" to the ISA subsets that the toolchain is
allowed to emit when building U-Boot, which results in compressed instructions in the U-Boot binary.
Sure, I will change it in v2.
+config RISCV_ISA_A
def_bool y
I do not believe U-Boot need to care about 'A' extension. So this can be dropped?
That's right. The only reason it might be used in U-Boot is on multi- core systems. Linux chooses the hart to boot on with the hart lottery, which requires atomics. The first core to increment a flag wins and starts the boot process. We could do something similar in U-Boot, but I don't think it's necessary. In my patches (for the next patch series) I have added CONFIG_MAIN_HART to select the hart U-Boot should run on.
Thanks for the additional details. I think we will need CONFIG_MAIN_HART to select the hart U-Boot should run on instead of Linux's lottery game, since we need handle situation like SiFive Freedom U540 where hart0 is something that does not run Linux. But even with CONFIG_MAIN_HART, U-Boot still doesn't need support 'A', right?
Yes that's right, at least I can't think of a reason for why we might need it. Should I drop it then?
Thanks, Lukas
I have included it here so that I can use it in the next patch to build the ISA string. I can remove it, but that would change the current behavior, which has atomics enabled.
Regards, Bin

Hi Lukas,
On Wed, Oct 24, 2018 at 11:41 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Wed, 2018-10-24 at 23:32 +0800, Bin Meng wrote:
Hi Lukas,
On Wed, Oct 24, 2018 at 11:21 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Add Kconfig entries for the C (compressed instructions) and A (atomic instructions) ISA extensions. Only the C ISA extension is selectable. This matches the configuration in Linux.
The Kconfig entries are not used yet. A follow-up patch will select the appropriate compiler flags based on the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b81e0d990a..e15329c35e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,15 @@ config ARCH_RV64I
endchoice
+config RISCV_ISA_C
bool "Emit compressed instructions"
default y
help
This enables the compressed instructions ("C") ISA
extension.
This is a little bit confusing. Can we use Linux kernel's description instead like below?
Adds "C" to the ISA subsets that the toolchain is
allowed to emit when building U-Boot, which results in compressed instructions in the U-Boot binary.
Sure, I will change it in v2.
+config RISCV_ISA_A
def_bool y
I do not believe U-Boot need to care about 'A' extension. So this can be dropped?
That's right. The only reason it might be used in U-Boot is on multi- core systems. Linux chooses the hart to boot on with the hart lottery, which requires atomics. The first core to increment a flag wins and starts the boot process. We could do something similar in U-Boot, but I don't think it's necessary. In my patches (for the next patch series) I have added CONFIG_MAIN_HART to select the hart U-Boot should run on.
Thanks for the additional details. I think we will need CONFIG_MAIN_HART to select the hart U-Boot should run on instead of Linux's lottery game, since we need handle situation like SiFive Freedom U540 where hart0 is something that does not run Linux. But even with CONFIG_MAIN_HART, U-Boot still doesn't need support 'A', right?
Yes that's right, at least I can't think of a reason for why we might need it. Should I drop it then?
Since we can use such to construct the "-march" string, let's keep this.
Regards, Bin

Use the new Kconfig entries to construct the ISA string for the -march compiler flag. The -mabi compiler flag is selected based on the base integer instruction set.
With this change, the C (compressed instructions) ISA extension is now enabled for all boards with CONFIG_RISCV_ISA_C set. Buildman reports a decrease in binary size of 71590 bytes.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/Makefile | 13 +++++++++++++ arch/riscv/config.mk | 4 ---- 2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 8fb6a889d8..6fb292d0b4 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -3,6 +3,19 @@ # Copyright (C) 2017 Andes Technology Corporation. # Rick Chen, Andes Technology Corporation rick@andestech.com
+riscv-march-$(CONFIG_ARCH_RV32I) := rv32im +riscv-march-$(CONFIG_ARCH_RV64I) := rv64im +riscv-march-$(CONFIG_RISCV_ISA_A) := $(riscv-march-y)a +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c + +riscv-mabi-$(CONFIG_ARCH_RV64I) := lp64 +riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32 + +arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y) + +PLATFORM_CPPFLAGS += $(arch-y) +CFLAGS_EFI += $(arch-y) + head-y := arch/riscv/cpu/start.o
libs-y += arch/riscv/cpu/ diff --git a/arch/riscv/config.mk b/arch/riscv/config.mk index ed9eb0c24c..9088b9ef2c 100644 --- a/arch/riscv/config.mk +++ b/arch/riscv/config.mk @@ -14,16 +14,12 @@ 64bit-emul := elf64lriscv
ifdef CONFIG_32BIT -PLATFORM_CPPFLAGS += -march=rv32ima -mabi=ilp32 PLATFORM_LDFLAGS += -m $(32bit-emul) -CFLAGS_EFI += -march=rv32ima -mabi=ilp32 EFI_LDS := elf_riscv32_efi.lds endif
ifdef CONFIG_64BIT -PLATFORM_CPPFLAGS += -march=rv64ima -mabi=lp64 PLATFORM_LDFLAGS += -m $(64bit-emul) -CFLAGS_EFI += -march=rv64ima -mabi=lp64 EFI_LDS := elf_riscv64_efi.lds endif

Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Use the new Kconfig entries to construct the ISA string for the -march compiler flag. The -mabi compiler flag is selected based on the base integer instruction set.
With this change, the C (compressed instructions) ISA extension is now enabled for all boards with CONFIG_RISCV_ISA_C set. Buildman reports a decrease in binary size of 71590 bytes.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Makefile | 13 +++++++++++++ arch/riscv/config.mk | 4 ---- 2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 8fb6a889d8..6fb292d0b4 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -3,6 +3,19 @@ # Copyright (C) 2017 Andes Technology Corporation. # Rick Chen, Andes Technology Corporation rick@andestech.com
+riscv-march-$(CONFIG_ARCH_RV32I) := rv32im +riscv-march-$(CONFIG_ARCH_RV64I) := rv64im +riscv-march-$(CONFIG_RISCV_ISA_A) := $(riscv-march-y)a +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
+riscv-mabi-$(CONFIG_ARCH_RV64I) := lp64 +riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32
+arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y)
+PLATFORM_CPPFLAGS += $(arch-y) +CFLAGS_EFI += $(arch-y)
The concept of this patch is good. However the usage of := is a bit odd, since it makes people think the latter will override the former one, however it is not.
Can we get rid of these riscv-mach-xxx, instead using something like this:
ifeq ($(CONFIG_RISCV_ISA_A),y) ARCH_A = a endif ifeq ($(CONFIG_RISCV_ISA_C),y) ARCH_C = c endif
ifeq ($(CONFIG_ARCH_RV32I),y) BITS = 32 ABI_I = i endif ifeq ($(CONFIG_ARCH_RV64I),y) BITS = 64 endif
PLATFORM_CPPFLAGS += -march=rv$(BITS)im$(ARCH_A)$(ARCH_C) -mabi=$(ABI_I)lp$(BITS)
head-y := arch/riscv/cpu/start.o
libs-y += arch/riscv/cpu/ diff --git a/arch/riscv/config.mk b/arch/riscv/config.mk index ed9eb0c24c..9088b9ef2c 100644 --- a/arch/riscv/config.mk +++ b/arch/riscv/config.mk @@ -14,16 +14,12 @@ 64bit-emul := elf64lriscv
ifdef CONFIG_32BIT -PLATFORM_CPPFLAGS += -march=rv32ima -mabi=ilp32 PLATFORM_LDFLAGS += -m $(32bit-emul) -CFLAGS_EFI += -march=rv32ima -mabi=ilp32 EFI_LDS := elf_riscv32_efi.lds endif
ifdef CONFIG_64BIT -PLATFORM_CPPFLAGS += -march=rv64ima -mabi=lp64 PLATFORM_LDFLAGS += -m $(64bit-emul) -CFLAGS_EFI += -march=rv64ima -mabi=lp64 EFI_LDS := elf_riscv64_efi.lds endif
--
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Use the new Kconfig entries to construct the ISA string for the -march compiler flag. The -mabi compiler flag is selected based on the base integer instruction set.
With this change, the C (compressed instructions) ISA extension is now enabled for all boards with CONFIG_RISCV_ISA_C set. Buildman reports a decrease in binary size of 71590 bytes.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Makefile | 13 +++++++++++++ arch/riscv/config.mk | 4 ---- 2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 8fb6a889d8..6fb292d0b4 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -3,6 +3,19 @@ # Copyright (C) 2017 Andes Technology Corporation. # Rick Chen, Andes Technology Corporation rick@andestech.com
+riscv-march-$(CONFIG_ARCH_RV32I) := rv32im +riscv-march-$(CONFIG_ARCH_RV64I) := rv64im +riscv-march-$(CONFIG_RISCV_ISA_A) := $(riscv-march-y)a +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
+riscv-mabi-$(CONFIG_ARCH_RV64I) := lp64 +riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32
+arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y)
+PLATFORM_CPPFLAGS += $(arch-y) +CFLAGS_EFI += $(arch-y)
The concept of this patch is good. However the usage of := is a bit odd, since it makes people think the latter will override the former one, however it is not.
Can we get rid of these riscv-mach-xxx, instead using something like this:
ifeq ($(CONFIG_RISCV_ISA_A),y) ARCH_A = a endif ifeq ($(CONFIG_RISCV_ISA_C),y) ARCH_C = c endif
ifeq ($(CONFIG_ARCH_RV32I),y) BITS = 32 ABI_I = i endif ifeq ($(CONFIG_ARCH_RV64I),y) BITS = 64 endif
PLATFORM_CPPFLAGS += -march=rv$(BITS)im$(ARCH_A)$(ARCH_C) -mabi=$(ABI_I)lp$(BITS)
That makes sense. Yes I can change it to something like that. One small change I would do is to try and keep any constant ISA / ABI strings (so rv and im) out of PLATFORM_CPPFLAGS to keep the configuration more in one place. So something like this. What do you think?
ifeq ($(CONFIG_ARCH_RV32I),y) ARCH_BASE = rv32im ABI = ilp32 endif ifeq ($(CONFIG_ARCH_RV64I),y) ARCH_BASE = rv64im ABI = lp64 endif
PLATFORM_CPPFLAGS += -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C) -mabi=$(ABI)
Thanks, Lukas
head-y := arch/riscv/cpu/start.o
libs-y += arch/riscv/cpu/ diff --git a/arch/riscv/config.mk b/arch/riscv/config.mk index ed9eb0c24c..9088b9ef2c 100644 --- a/arch/riscv/config.mk +++ b/arch/riscv/config.mk @@ -14,16 +14,12 @@ 64bit-emul := elf64lriscv
ifdef CONFIG_32BIT -PLATFORM_CPPFLAGS += -march=rv32ima -mabi=ilp32 PLATFORM_LDFLAGS += -m $(32bit-emul) -CFLAGS_EFI += -march=rv32ima -mabi=ilp32 EFI_LDS := elf_riscv32_efi.lds endif
ifdef CONFIG_64BIT -PLATFORM_CPPFLAGS += -march=rv64ima -mabi=lp64 PLATFORM_LDFLAGS += -m $(64bit-emul) -CFLAGS_EFI += -march=rv64ima -mabi=lp64 EFI_LDS := elf_riscv64_efi.lds endif
--
Regards, Bin

Hi Lukas,
On Wed, Oct 24, 2018 at 11:57 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Use the new Kconfig entries to construct the ISA string for the -march compiler flag. The -mabi compiler flag is selected based on the base integer instruction set.
With this change, the C (compressed instructions) ISA extension is now enabled for all boards with CONFIG_RISCV_ISA_C set. Buildman reports a decrease in binary size of 71590 bytes.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Makefile | 13 +++++++++++++ arch/riscv/config.mk | 4 ---- 2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 8fb6a889d8..6fb292d0b4 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -3,6 +3,19 @@ # Copyright (C) 2017 Andes Technology Corporation. # Rick Chen, Andes Technology Corporation rick@andestech.com
+riscv-march-$(CONFIG_ARCH_RV32I) := rv32im +riscv-march-$(CONFIG_ARCH_RV64I) := rv64im +riscv-march-$(CONFIG_RISCV_ISA_A) := $(riscv-march-y)a +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
+riscv-mabi-$(CONFIG_ARCH_RV64I) := lp64 +riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32
+arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y)
+PLATFORM_CPPFLAGS += $(arch-y) +CFLAGS_EFI += $(arch-y)
The concept of this patch is good. However the usage of := is a bit odd, since it makes people think the latter will override the former one, however it is not.
Can we get rid of these riscv-mach-xxx, instead using something like this:
ifeq ($(CONFIG_RISCV_ISA_A),y) ARCH_A = a endif ifeq ($(CONFIG_RISCV_ISA_C),y) ARCH_C = c endif
ifeq ($(CONFIG_ARCH_RV32I),y) BITS = 32 ABI_I = i endif ifeq ($(CONFIG_ARCH_RV64I),y) BITS = 64 endif
PLATFORM_CPPFLAGS += -march=rv$(BITS)im$(ARCH_A)$(ARCH_C) -mabi=$(ABI_I)lp$(BITS)
That makes sense. Yes I can change it to something like that. One small change I would do is to try and keep any constant ISA / ABI strings (so rv and im) out of PLATFORM_CPPFLAGS to keep the configuration more in one place. So something like this. What do you think?
This looks good. Thanks!
ifeq ($(CONFIG_ARCH_RV32I),y) ARCH_BASE = rv32im ABI = ilp32 endif ifeq ($(CONFIG_ARCH_RV64I),y) ARCH_BASE = rv64im ABI = lp64 endif
PLATFORM_CPPFLAGS += -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C) -mabi=$(ABI)
Regards, Bin

RISC-V has two code models, medium low (medlow) and medium any (medany). Medlow limits addressable memory to a single 2 GiB range between the absolute addresses -2 GiB and +2 GiB. Medany limits addressable memory to any single 2 GiB address range. By default, medlow is selected on 32-bit systems and medany on 64-bit systems. This matches the configuration in Linux.
The -mcmodel compiler flag is selected according to the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/Kconfig | 19 +++++++++++++++++++ arch/riscv/Makefile | 7 +++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index e15329c35e..ce07fb4b55 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,25 @@ config ARCH_RV64I
endchoice
+choice + prompt "Code Model" + default CMODEL_MEDLOW if 32BIT + default CMODEL_MEDANY if 64BIT + +config CMODEL_MEDLOW + bool "medium low code model" + help + U-Boot and its statically defined symbols must lie within a single 2 GiB + address range and must lie between absolute addresses -2 GiB and +2 GiB. + +config CMODEL_MEDANY + bool "medium any code model" + help + U-Boot and its statically defined symbols must be within any single 2 GiB + address range. + +endchoice + config RISCV_ISA_C bool "Emit compressed instructions" default y diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 6fb292d0b4..da6e50bd14 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -13,8 +13,11 @@ riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32
arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y)
-PLATFORM_CPPFLAGS += $(arch-y) -CFLAGS_EFI += $(arch-y) +cmodel-$(CONFIG_CMODEL_MEDLOW) := -mcmodel=medlow +cmodel-$(CONFIG_CMODEL_MEDANY) := -mcmodel=medany + +PLATFORM_CPPFLAGS += $(arch-y) $(cmodel-y) +CFLAGS_EFI += $(arch-y) $(cmodel-y)
head-y := arch/riscv/cpu/start.o

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
RISC-V has two code models, medium low (medlow) and medium any (medany). Medlow limits addressable memory to a single 2 GiB range between the absolute addresses -2 GiB and +2 GiB. Medany limits addressable memory to any single 2 GiB address range. By default, medlow is selected on 32-bit systems and medany on 64-bit systems. This matches the configuration in Linux.
The -mcmodel compiler flag is selected according to the Kconfig configuration.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 19 +++++++++++++++++++ arch/riscv/Makefile | 7 +++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index e15329c35e..ce07fb4b55 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -38,6 +38,25 @@ config ARCH_RV64I
endchoice
+choice
prompt "Code Model"
default CMODEL_MEDLOW if 32BIT
default CMODEL_MEDANY if 64BIT
+config CMODEL_MEDLOW
bool "medium low code model"
help
U-Boot and its statically defined symbols must lie within a single 2 GiB
address range and must lie between absolute addresses -2 GiB and +2 GiB.
+config CMODEL_MEDANY
bool "medium any code model"
help
U-Boot and its statically defined symbols must be within any single 2 GiB
address range.
+endchoice
config RISCV_ISA_C bool "Emit compressed instructions" default y diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 6fb292d0b4..da6e50bd14 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -13,8 +13,11 @@ riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32
arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y)
-PLATFORM_CPPFLAGS += $(arch-y) -CFLAGS_EFI += $(arch-y) +cmodel-$(CONFIG_CMODEL_MEDLOW) := -mcmodel=medlow +cmodel-$(CONFIG_CMODEL_MEDANY) := -mcmodel=medany
+PLATFORM_CPPFLAGS += $(arch-y) $(cmodel-y) +CFLAGS_EFI += $(arch-y) $(cmodel-y)
head-y := arch/riscv/cpu/start.o
See my comments for patch [07/30]. We can do similar thing to the code model.
Regards, Bin

Move the target selection into a separate file (Kconfig.board) to avoid clutter once we support more boards.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/Kconfig | 17 ++--------------- arch/riscv/Kconfig.board | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 arch/riscv/Kconfig.board
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index ce07fb4b55..10d17a0e18 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -4,21 +4,6 @@ menu "RISC-V architecture" config SYS_ARCH default "riscv"
-choice - prompt "Target select" - optional - -config TARGET_AX25_AE350 - bool "Support ax25-ae350" - -config TARGET_QEMU_VIRT - bool "Support QEMU Virt Board" - -endchoice - -source "board/AndesTech/ax25-ae350/Kconfig" -source "board/emulation/qemu-riscv/Kconfig" - choice prompt "Base ISA" default ARCH_RV32I @@ -72,4 +57,6 @@ config 32BIT config 64BIT bool
+source "arch/riscv/Kconfig.board" + endmenu diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board new file mode 100644 index 0000000000..fcada760c8 --- /dev/null +++ b/arch/riscv/Kconfig.board @@ -0,0 +1,14 @@ +choice + prompt "Target select" + optional + +config TARGET_AX25_AE350 + bool "Support ax25-ae350" + +config TARGET_QEMU_VIRT + bool "Support QEMU Virt Board" + +endchoice + +source "board/AndesTech/ax25-ae350/Kconfig" +source "board/emulation/qemu-riscv/Kconfig"

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Move the target selection into a separate file (Kconfig.board) to avoid clutter once we support more boards.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 17 ++--------------- arch/riscv/Kconfig.board | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 arch/riscv/Kconfig.board
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index ce07fb4b55..10d17a0e18 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -4,21 +4,6 @@ menu "RISC-V architecture" config SYS_ARCH default "riscv"
-choice
prompt "Target select"
optional
-config TARGET_AX25_AE350
bool "Support ax25-ae350"
-config TARGET_QEMU_VIRT
bool "Support QEMU Virt Board"
-endchoice
-source "board/AndesTech/ax25-ae350/Kconfig" -source "board/emulation/qemu-riscv/Kconfig"
choice prompt "Base ISA" default ARCH_RV32I @@ -72,4 +57,6 @@ config 32BIT config 64BIT bool
+source "arch/riscv/Kconfig.board"
I am OK with moving board one to a separate file, though it looks no other arch uses scuh convention in U-Boot :)
However, with this change, it lost the capability of overriding an architecture defined Kconfig option at board level.
I have a patch @ http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907... to express such capability.
endmenu diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board new file mode 100644 index 0000000000..fcada760c8 --- /dev/null +++ b/arch/riscv/Kconfig.board @@ -0,0 +1,14 @@ +choice
prompt "Target select"
optional
+config TARGET_AX25_AE350
bool "Support ax25-ae350"
+config TARGET_QEMU_VIRT
bool "Support QEMU Virt Board"
+endchoice
+source "board/AndesTech/ax25-ae350/Kconfig"
+source "board/emulation/qemu-riscv/Kconfig"
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 15:22 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Move the target selection into a separate file (Kconfig.board) to avoid clutter once we support more boards.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 17 ++--------------- arch/riscv/Kconfig.board | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 arch/riscv/Kconfig.board
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index ce07fb4b55..10d17a0e18 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -4,21 +4,6 @@ menu "RISC-V architecture" config SYS_ARCH default "riscv"
-choice
prompt "Target select"
optional
-config TARGET_AX25_AE350
bool "Support ax25-ae350"
-config TARGET_QEMU_VIRT
bool "Support QEMU Virt Board"
-endchoice
-source "board/AndesTech/ax25-ae350/Kconfig" -source "board/emulation/qemu-riscv/Kconfig"
choice prompt "Base ISA" default ARCH_RV32I @@ -72,4 +57,6 @@ config 32BIT config 64BIT bool
+source "arch/riscv/Kconfig.board"
I am OK with moving board one to a separate file, though it looks no other arch uses scuh convention in U-Boot :)
However, with this change, it lost the capability of overriding an architecture defined Kconfig option at board level.
I have a patch @
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907...
to express such capability.
This should be easy to fix by just moving the source statement, right?
Thanks, Lukas
endmenu diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board new file mode 100644 index 0000000000..fcada760c8 --- /dev/null +++ b/arch/riscv/Kconfig.board @@ -0,0 +1,14 @@ +choice
prompt "Target select"
optional
+config TARGET_AX25_AE350
bool "Support ax25-ae350"
+config TARGET_QEMU_VIRT
bool "Support QEMU Virt Board"
+endchoice
+source "board/AndesTech/ax25-ae350/Kconfig"
+source "board/emulation/qemu-riscv/Kconfig"
Regards, Bin

Hi Lukas,
On Thu, Oct 25, 2018 at 7:39 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Mon, 2018-10-22 at 15:22 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Move the target selection into a separate file (Kconfig.board) to avoid clutter once we support more boards.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 17 ++--------------- arch/riscv/Kconfig.board | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 arch/riscv/Kconfig.board
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index ce07fb4b55..10d17a0e18 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -4,21 +4,6 @@ menu "RISC-V architecture" config SYS_ARCH default "riscv"
-choice
prompt "Target select"
optional
-config TARGET_AX25_AE350
bool "Support ax25-ae350"
-config TARGET_QEMU_VIRT
bool "Support QEMU Virt Board"
-endchoice
-source "board/AndesTech/ax25-ae350/Kconfig" -source "board/emulation/qemu-riscv/Kconfig"
choice prompt "Base ISA" default ARCH_RV32I @@ -72,4 +57,6 @@ config 32BIT config 64BIT bool
+source "arch/riscv/Kconfig.board"
I am OK with moving board one to a separate file, though it looks no other arch uses scuh convention in U-Boot :)
However, with this change, it lost the capability of overriding an architecture defined Kconfig option at board level.
I have a patch @
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907...
to express such capability.
This should be easy to fix by just moving the source statement, right?
Yes. You can drop this patch.
Regards, Bin

Enable the -fdata-sections compiler option for RISC-V. Buildman reports the binary size decrease from this as 8365.3 bytes.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/config.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/config.mk b/arch/riscv/config.mk index 9088b9ef2c..b9487097f6 100644 --- a/arch/riscv/config.mk +++ b/arch/riscv/config.mk @@ -27,7 +27,8 @@ CONFIG_STANDALONE_LOAD_ADDR = 0x00000000 \ -T $(srctree)/examples/standalone/riscv.lds
PLATFORM_CPPFLAGS += -ffixed-gp -fpic -PLATFORM_RELFLAGS += -fno-common -gdwarf-2 -ffunction-sections +PLATFORM_RELFLAGS += -fno-common -gdwarf-2 -ffunction-sections \ + -fdata-sections LDFLAGS_u-boot += --gc-sections -static -pie
EFI_CRT0 := crt0_riscv_efi.o

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Enable the -fdata-sections compiler option for RISC-V. Buildman reports the binary size decrease from this as 8365.3 bytes.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/config.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

The RISC-V arch incorrectly uses 32-bit instead of 64-bit variables in several places. Fix this. In addition, BITS_PER_LONG is set to 64 on RV64I systems.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/include/asm/io.h | 6 +++--- arch/riscv/include/asm/posix_types.h | 6 +++--- arch/riscv/include/asm/types.h | 4 ++++ arch/riscv/lib/interrupts.c | 10 +++++----- 4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h index f4a76d8720..472814a13e 100644 --- a/arch/riscv/include/asm/io.h +++ b/arch/riscv/include/asm/io.h @@ -74,12 +74,12 @@ static inline phys_addr_t virt_to_phys(void *vaddr) #define __arch_getb(a) (*(unsigned char *)(a)) #define __arch_getw(a) (*(unsigned short *)(a)) #define __arch_getl(a) (*(unsigned int *)(a)) -#define __arch_getq(a) (*(unsigned long *)(a)) +#define __arch_getq(a) (*(unsigned long long *)(a))
#define __arch_putb(v, a) (*(unsigned char *)(a) = (v)) #define __arch_putw(v, a) (*(unsigned short *)(a) = (v)) #define __arch_putl(v, a) (*(unsigned int *)(a) = (v)) -#define __arch_putq(v, a) (*(unsigned long *)(a) = (v)) +#define __arch_putq(v, a) (*(unsigned long long *)(a) = (v))
#define __raw_writeb(v, a) __arch_putb(v, a) #define __raw_writew(v, a) __arch_putw(v, a) @@ -152,7 +152,7 @@ static inline u32 readl(const volatile void __iomem *addr)
static inline u64 readq(const volatile void __iomem *addr) { - u32 val; + u64 val;
val = __arch_getq(addr); __iormb(); diff --git a/arch/riscv/include/asm/posix_types.h b/arch/riscv/include/asm/posix_types.h index 7438dbeb03..0fc052082a 100644 --- a/arch/riscv/include/asm/posix_types.h +++ b/arch/riscv/include/asm/posix_types.h @@ -37,10 +37,10 @@ typedef unsigned short __kernel_gid_t; #ifdef __GNUC__ typedef __SIZE_TYPE__ __kernel_size_t; #else -typedef unsigned int __kernel_size_t; +typedef unsigned long __kernel_size_t; #endif -typedef int __kernel_ssize_t; -typedef int __kernel_ptrdiff_t; +typedef long __kernel_ssize_t; +typedef long __kernel_ptrdiff_t; typedef long __kernel_time_t; typedef long __kernel_suseconds_t; typedef long __kernel_clock_t; diff --git a/arch/riscv/include/asm/types.h b/arch/riscv/include/asm/types.h index bd8627196d..403cf9a48f 100644 --- a/arch/riscv/include/asm/types.h +++ b/arch/riscv/include/asm/types.h @@ -21,7 +21,11 @@ typedef unsigned short umode_t; */ #ifdef __KERNEL__
+#ifdef CONFIG_ARCH_RV64I +#define BITS_PER_LONG 64 +#else #define BITS_PER_LONG 32 +#endif
#include <stddef.h>
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 0a0995a7af..1c16980a64 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -12,7 +12,7 @@ #include <asm/system.h> #include <asm/encoding.h>
-static void _exit_trap(int code, uint epc, struct pt_regs *regs); +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs);
int interrupt_init(void) { @@ -34,9 +34,9 @@ int disable_interrupts(void) return 0; }
-uint handle_trap(uint mcause, uint epc, struct pt_regs *regs) +ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs) { - uint is_int; + ulong is_int;
is_int = (mcause & MCAUSE_INT); if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT)) @@ -60,7 +60,7 @@ __attribute__((weak)) void timer_interrupt(struct pt_regs *regs) { }
-static void _exit_trap(int code, uint epc, struct pt_regs *regs) +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs) { static const char * const exception_code[] = { "Instruction address misaligned", @@ -70,6 +70,6 @@ static void _exit_trap(int code, uint epc, struct pt_regs *regs) "Load address misaligned" };
- printf("exception code: %d , %s , epc %08x , ra %08lx\n", + printf("exception code: %ld , %s , epc %016lx , ra %016lx\n", code, exception_code[code], epc, regs->ra); }

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
The RISC-V arch incorrectly uses 32-bit instead of 64-bit variables in several places. Fix this. In addition, BITS_PER_LONG is set to 64 on RV64I systems.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/include/asm/io.h | 6 +++--- arch/riscv/include/asm/posix_types.h | 6 +++--- arch/riscv/include/asm/types.h | 4 ++++ arch/riscv/lib/interrupts.c | 10 +++++----- 4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h index f4a76d8720..472814a13e 100644 --- a/arch/riscv/include/asm/io.h +++ b/arch/riscv/include/asm/io.h @@ -74,12 +74,12 @@ static inline phys_addr_t virt_to_phys(void *vaddr) #define __arch_getb(a) (*(unsigned char *)(a)) #define __arch_getw(a) (*(unsigned short *)(a)) #define __arch_getl(a) (*(unsigned int *)(a)) -#define __arch_getq(a) (*(unsigned long *)(a)) +#define __arch_getq(a) (*(unsigned long long *)(a))
#define __arch_putb(v, a) (*(unsigned char *)(a) = (v)) #define __arch_putw(v, a) (*(unsigned short *)(a) = (v)) #define __arch_putl(v, a) (*(unsigned int *)(a) = (v)) -#define __arch_putq(v, a) (*(unsigned long *)(a) = (v)) +#define __arch_putq(v, a) (*(unsigned long long *)(a) = (v))
#define __raw_writeb(v, a) __arch_putb(v, a) #define __raw_writew(v, a) __arch_putw(v, a) @@ -152,7 +152,7 @@ static inline u32 readl(const volatile void __iomem *addr)
static inline u64 readq(const volatile void __iomem *addr) {
u32 val;
u64 val; val = __arch_getq(addr); __iormb();
diff --git a/arch/riscv/include/asm/posix_types.h b/arch/riscv/include/asm/posix_types.h index 7438dbeb03..0fc052082a 100644 --- a/arch/riscv/include/asm/posix_types.h +++ b/arch/riscv/include/asm/posix_types.h @@ -37,10 +37,10 @@ typedef unsigned short __kernel_gid_t; #ifdef __GNUC__ typedef __SIZE_TYPE__ __kernel_size_t; #else -typedef unsigned int __kernel_size_t; +typedef unsigned long __kernel_size_t; #endif -typedef int __kernel_ssize_t; -typedef int __kernel_ptrdiff_t; +typedef long __kernel_ssize_t; +typedef long __kernel_ptrdiff_t; typedef long __kernel_time_t; typedef long __kernel_suseconds_t; typedef long __kernel_clock_t; diff --git a/arch/riscv/include/asm/types.h b/arch/riscv/include/asm/types.h index bd8627196d..403cf9a48f 100644 --- a/arch/riscv/include/asm/types.h +++ b/arch/riscv/include/asm/types.h @@ -21,7 +21,11 @@ typedef unsigned short umode_t; */ #ifdef __KERNEL__
+#ifdef CONFIG_ARCH_RV64I +#define BITS_PER_LONG 64 +#else #define BITS_PER_LONG 32 +#endif
#include <stddef.h>
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 0a0995a7af..1c16980a64 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -12,7 +12,7 @@ #include <asm/system.h> #include <asm/encoding.h>
-static void _exit_trap(int code, uint epc, struct pt_regs *regs); +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs);
int interrupt_init(void) { @@ -34,9 +34,9 @@ int disable_interrupts(void) return 0; }
-uint handle_trap(uint mcause, uint epc, struct pt_regs *regs) +ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs) {
uint is_int;
ulong is_int; is_int = (mcause & MCAUSE_INT); if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT))
@@ -60,7 +60,7 @@ __attribute__((weak)) void timer_interrupt(struct pt_regs *regs) { }
-static void _exit_trap(int code, uint epc, struct pt_regs *regs) +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs) { static const char * const exception_code[] = { "Instruction address misaligned", @@ -70,6 +70,6 @@ static void _exit_trap(int code, uint epc, struct pt_regs *regs) "Load address misaligned" };
printf("exception code: %d , %s , epc %08x , ra %08lx\n",
printf("exception code: %ld , %s , epc %016lx , ra %016lx\n",
We should not print 16 digits on a RV32 system.
code, exception_code[code], epc, regs->ra);
}
Others look good to me.
Regards, Bin

Replace the barrier functions in arch/riscv/include/asm/io.h with those defined in barrier.h, which is imported from Linux. This version is modified to remove the include statement of asm-generic/barrier.h, which is not available in u-boot or required.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de --- Checkpatch reports style errors in barrier.h. I did not fix them, because I imported the file from Linux. Is that the correct way to handle this?
arch/riscv/include/asm/barrier.h | 67 ++++++++++++++++++++++++++++++++ arch/riscv/include/asm/io.h | 11 ++---- 2 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 arch/riscv/include/asm/barrier.h
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h new file mode 100644 index 0000000000..a3f60a8458 --- /dev/null +++ b/arch/riscv/include/asm/barrier.h @@ -0,0 +1,67 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2012 ARM Ltd. + * Copyright (C) 2013 Regents of the University of California + * Copyright (C) 2017 SiFive + * + * Taken from Linux arch/riscv/include/asm/barrier.h, which is based on + * arch/arm/include/asm/barrier.h + */ + +#ifndef _ASM_RISCV_BARRIER_H +#define _ASM_RISCV_BARRIER_H + +#ifndef __ASSEMBLY__ + +#define nop() __asm__ __volatile__ ("nop") + +#define RISCV_FENCE(p, s) \ + __asm__ __volatile__ ("fence " #p "," #s : : : "memory") + +/* These barriers need to enforce ordering on both devices or memory. */ +#define mb() RISCV_FENCE(iorw,iorw) +#define rmb() RISCV_FENCE(ir,ir) +#define wmb() RISCV_FENCE(ow,ow) + +/* These barriers do not need to enforce ordering on devices, just memory. */ +#define __smp_mb() RISCV_FENCE(rw,rw) +#define __smp_rmb() RISCV_FENCE(r,r) +#define __smp_wmb() RISCV_FENCE(w,w) + +#define __smp_store_release(p, v) \ +do { \ + compiletime_assert_atomic_type(*p); \ + RISCV_FENCE(rw,w); \ + WRITE_ONCE(*p, v); \ +} while (0) + +#define __smp_load_acquire(p) \ +({ \ + typeof(*p) ___p1 = READ_ONCE(*p); \ + compiletime_assert_atomic_type(*p); \ + RISCV_FENCE(r,rw); \ + ___p1; \ +}) + +/* + * This is a very specific barrier: it's currently only used in two places in + * the kernel, both in the scheduler. See include/linux/spinlock.h for the two + * orderings it guarantees, but the "critical section is RCsc" guarantee + * mandates a barrier on RISC-V. The sequence looks like: + * + * lr.aq lock + * sc lock <= LOCKED + * smp_mb__after_spinlock() + * // critical section + * lr lock + * sc.rl lock <= UNLOCKED + * + * The AQ/RL pair provides a RCpc critical section, but there's not really any + * way we can take advantage of that here because the ordering is only enforced + * on that one lock. Thus, we're just doing a full fence. + */ +#define smp_mb__after_spinlock() RISCV_FENCE(rw,rw) + +#endif /* __ASSEMBLY__ */ + +#endif /* _ASM_RISCV_BARRIER_H */ diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h index 472814a13e..d01ed5bc9f 100644 --- a/arch/riscv/include/asm/io.h +++ b/arch/riscv/include/asm/io.h @@ -10,6 +10,7 @@ #ifdef __KERNEL__
#include <linux/types.h> +#include <asm/barrier.h> #include <asm/byteorder.h>
static inline void sync(void) @@ -91,13 +92,9 @@ static inline phys_addr_t virt_to_phys(void *vaddr) #define __raw_readl(a) __arch_getl(a) #define __raw_readq(a) __arch_getq(a)
-/* - * TODO: The kernel offers some more advanced versions of barriers, it might - * have some advantages to use them instead of the simple one here. - */ -#define dmb() __asm__ __volatile__ ("" : : : "memory") -#define __iormb() dmb() -#define __iowmb() dmb() +#define dmb() mb() +#define __iormb() rmb() +#define __iowmb() wmb()
static inline void writeb(u8 val, volatile void __iomem *addr) {

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Replace the barrier functions in arch/riscv/include/asm/io.h with those defined in barrier.h, which is imported from Linux. This version is modified to remove the include statement of asm-generic/barrier.h, which is not available in u-boot or required.
nits: U-Boot
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
Checkpatch reports style errors in barrier.h. I did not fix them, because I imported the file from Linux. Is that the correct way to handle this?
I think this is OK.
arch/riscv/include/asm/barrier.h | 67 ++++++++++++++++++++++++++++++++ arch/riscv/include/asm/io.h | 11 ++---- 2 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 arch/riscv/include/asm/barrier.h
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 15:36 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Replace the barrier functions in arch/riscv/include/asm/io.h with those defined in barrier.h, which is imported from Linux. This version is modified to remove the include statement of asm-generic/barrier.h, which is not available in u-boot or required.
nits: U-Boot
Fixed.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
Checkpatch reports style errors in barrier.h. I did not fix them, because I imported the file from Linux. Is that the correct way to handle this?
I think this is OK.
Ok, I will keep it as is.
Thanks, Lukas
arch/riscv/include/asm/barrier.h | 67 ++++++++++++++++++++++++++++++++ arch/riscv/include/asm/io.h | 11 ++---- 2 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 arch/riscv/include/asm/barrier.h
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

RISC-V u-boot reimplements the generic io functions from asm-generic/io.h. Remove the redundant implementation and include the generic io.h instead.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/include/asm/io.h | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-)
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h index d01ed5bc9f..acf5a96449 100644 --- a/arch/riscv/include/asm/io.h +++ b/arch/riscv/include/asm/io.h @@ -17,16 +17,6 @@ static inline void sync(void) { }
-/* - * Given a physical address and a length, return a virtual address - * that can be used to access the memory range with the caching - * properties specified by "flags". - */ -#define MAP_NOCACHE (0) -#define MAP_WRCOMBINE (0) -#define MAP_WRBACK (0) -#define MAP_WRTHROUGH (0) - #ifdef CONFIG_ARCH_MAP_SYSMEM static inline void *map_sysmem(phys_addr_t paddr, unsigned long len) { @@ -49,24 +39,6 @@ static inline phys_addr_t map_to_sysmem(const void *ptr) } #endif
-static inline void * -map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags) -{ - return (void *)paddr; -} - -/* - * Take down a mapping set up by map_physmem(). - */ -static inline void unmap_physmem(void *vaddr, unsigned long flags) -{ -} - -static inline phys_addr_t virt_to_phys(void *vaddr) -{ - return (phys_addr_t)(vaddr); -} - /* * Generic virtual read/write. Note that we don't support half-word * read/writes. We define __arch_*[bl] here, and leave __arch_*w @@ -484,4 +456,7 @@ out:
#endif /* __mem_isa */ #endif /* __KERNEL__ */ + +#include <asm-generic/io.h> + #endif /* __ASM_RISCV_IO_H */

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
RISC-V u-boot reimplements the generic io functions from
nits: U-Boot
asm-generic/io.h. Remove the redundant implementation and include the generic io.h instead.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/include/asm/io.h | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 15:36 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
RISC-V u-boot reimplements the generic io functions from
nits: U-Boot
Fixed in v2.
Thanks, Lukas
asm-generic/io.h. Remove the redundant implementation and include the generic io.h instead.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/include/asm/io.h | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

Only the first four exception codes are defined. Add the missing exception codes from the definition in RISC-V Privileged Architecture Version 1.10.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/lib/interrupts.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 1c16980a64..3d830c3273 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -67,7 +67,18 @@ static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs) "Instruction access fault", "Illegal instruction", "Breakpoint", - "Load address misaligned" + "Load address misaligned", + "Load access fault", + "Store/AMO address misaligned", + "Store/AMO access fault", + "Environment call from U-mode", + "Environment call from S-mode", + "Reserved", + "Environment call from M-mode", + "Instruction page fault", + "Load page fault", + "Reserved", + "Store/AMO page fault", };
printf("exception code: %ld , %s , epc %016lx , ra %016lx\n",

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Only the first four exception codes are defined. Add the missing exception codes from the definition in RISC-V Privileged Architecture Version 1.10.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/lib/interrupts.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Undefined exception codes currently lead to an out-of-bounds array access. Prevent this by treating undefined exception codes as "reserved".
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/lib/interrupts.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 3d830c3273..32d0598750 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -81,6 +81,10 @@ static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs) "Store/AMO page fault", };
- printf("exception code: %ld , %s , epc %016lx , ra %016lx\n", - code, exception_code[code], epc, regs->ra); + if (code < ARRAY_SIZE(exception_code)) { + printf("exception code: %ld , %s , epc %016lx , ra %016lx\n", + code, exception_code[code], epc, regs->ra); + } else { + printf("Reserved\n"); + } }

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Undefined exception codes currently lead to an out-of-bounds array access. Prevent this by treating undefined exception codes as "reserved".
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/lib/interrupts.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hang on unhandled exceptions to prevent execution in a faulty state.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/lib/interrupts.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 32d0598750..3b74b76c70 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -87,4 +87,6 @@ static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs) } else { printf("Reserved\n"); } + + hang(); }

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Hang on unhandled exceptions to prevent execution in a faulty state.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/lib/interrupts.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Implement the functions invalidate_icache_range() and invalidate_icache_all().
RISC-V does not have instructions for explicit cache-control. The functions in this patch are implemented with the memory ordering instruction for synchronizing the instruction and data streams. This may be implemented as a cache flush or invalidate on simple processors, others may only invalidate the relevant cache lines.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/lib/cache.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c index 1d67c49c2c..d642a38a07 100644 --- a/arch/riscv/lib/cache.c +++ b/arch/riscv/lib/cache.c @@ -12,6 +12,16 @@ void flush_dcache_range(unsigned long start, unsigned long end)
void invalidate_icache_range(unsigned long start, unsigned long end) { + /* + * RISC-V does not have an instruction for invalidating parts of the + * instruction cache. Invalidate all of it instead. + */ + invalidate_icache_all(); +} + +void invalidate_icache_all(void) +{ + asm volatile ("fence.i" ::: "memory"); }
void invalidate_dcache_range(unsigned long start, unsigned long end)

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Implement the functions invalidate_icache_range() and invalidate_icache_all().
RISC-V does not have instructions for explicit cache-control. The functions in this patch are implemented with the memory ordering instruction for synchronizing the instruction and data streams. This may be implemented as a cache flush or invalidate on simple processors, others may only invalidate the relevant cache lines.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/lib/cache.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/lib/bootm.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c index a7a9fb921b..bc1d4b2864 100644 --- a/arch/riscv/lib/bootm.c +++ b/arch/riscv/lib/bootm.c @@ -38,6 +38,7 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) return 1;
kernel = (void (*)(ulong, void *))images->ep; + invalidate_icache_all();
bootstage_mark(BOOTSTAGE_ID_RUN_OS);

Start.S uses both tabs and spaces after instructions. Fix this by only using tabs after instructions.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/cpu/start.S | 322 ++++++++++++++++++++--------------------- 1 file changed, 161 insertions(+), 161 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 7cd7755190..bd5904500c 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -16,56 +16,56 @@ #include <asm/encoding.h>
#ifdef CONFIG_32BIT -#define LREG lw -#define SREG sw -#define REGBYTES 4 +#define LREG lw +#define SREG sw +#define REGBYTES 4 #define RELOC_TYPE R_RISCV_32 #define SYM_INDEX 0x8 #define SYM_SIZE 0x10 #else -#define LREG ld -#define SREG sd -#define REGBYTES 8 +#define LREG ld +#define SREG sd +#define REGBYTES 8 #define RELOC_TYPE R_RISCV_64 #define SYM_INDEX 0x20 #define SYM_SIZE 0x18 #endif
-.section .text +.section .text .globl _start _start: - j handle_reset + j handle_reset
nmi_vector: - j nmi_vector + j nmi_vector
trap_vector: - j trap_entry + j trap_entry
.global trap_entry handle_reset: - li t0, CONFIG_SYS_SDRAM_BASE - SREG a2, 0(t0) - la t0, trap_entry - csrw mtvec, t0 - csrwi mstatus, 0 - csrwi mie, 0 + li t0, CONFIG_SYS_SDRAM_BASE + SREG a2, 0(t0) + la t0, trap_entry + csrw mtvec, t0 + csrwi mstatus, 0 + csrwi mie, 0
/* * Do CPU critical regs init only at reboot, * not when booting from ram */ #ifdef CONFIG_INIT_CRITICAL - jal cpu_init_crit /* Do CPU critical regs init */ + jal cpu_init_crit /* Do CPU critical regs init */ #endif
/* * Set stackpointer in internal/ex RAM to call board_init_f */ call_board_init_f: - li t0, -16 - li t1, CONFIG_SYS_INIT_SP_ADDR - and sp, t1, t0 /* force 16 byte alignment */ + li t0, -16 + li t1, CONFIG_SYS_INIT_SP_ADDR + and sp, t1, t0 /* force 16 byte alignment */
#ifdef CONFIG_DEBUG_UART jal debug_uart_init @@ -77,9 +77,9 @@ call_board_init_f_0: mv sp, a0 jal board_init_f_init_reserve
- mv a0, zero /* a0 <-- boot_flags = 0 */ - la t5, board_init_f - jr t5 /* jump to board_init_f() */ + mv a0, zero /* a0 <-- boot_flags = 0 */ + la t5, board_init_f + jr t5 /* jump to board_init_f() */
/* * void relocate_code (addr_sp, gd, addr_moni) @@ -90,200 +90,200 @@ call_board_init_f_0: */ .globl relocate_code relocate_code: - mv s2, a0 /* save addr_sp */ - mv s3, a1 /* save addr of gd */ - mv s4, a2 /* save addr of destination */ + mv s2, a0 /* save addr_sp */ + mv s3, a1 /* save addr of gd */ + mv s4, a2 /* save addr of destination */
/* *Set up the stack */ stack_setup: - mv sp, s2 - la t0, _start - sub t6, s4, t0 /* t6 <- relocation offset */ - beq t0, s4, clear_bss /* skip relocation */ + mv sp, s2 + la t0, _start + sub t6, s4, t0 /* t6 <- relocation offset */ + beq t0, s4, clear_bss /* skip relocation */
- mv t1, s4 /* t1 <- scratch for copy_loop */ - la t3, __bss_start - sub t3, t3, t0 /* t3 <- __bss_start_ofs */ - add t2, t0, t3 /* t2 <- source end address */ + mv t1, s4 /* t1 <- scratch for copy_loop */ + la t3, __bss_start + sub t3, t3, t0 /* t3 <- __bss_start_ofs */ + add t2, t0, t3 /* t2 <- source end address */
copy_loop: - LREG t5, 0(t0) - addi t0, t0, REGBYTES - SREG t5, 0(t1) - addi t1, t1, REGBYTES - blt t0, t2, copy_loop + LREG t5, 0(t0) + addi t0, t0, REGBYTES + SREG t5, 0(t1) + addi t1, t1, REGBYTES + blt t0, t2, copy_loop
/* * Update dynamic relocations after board_init_f */ fix_rela_dyn: - la t1, __rel_dyn_start - la t2, __rel_dyn_end - beq t1, t2, clear_bss - add t1, t1, t6 /* t1 <- rela_dyn_start in RAM */ - add t2, t2, t6 /* t2 <- rela_dyn_end in RAM */ + la t1, __rel_dyn_start + la t2, __rel_dyn_end + beq t1, t2, clear_bss + add t1, t1, t6 /* t1 <- rela_dyn_start in RAM */ + add t2, t2, t6 /* t2 <- rela_dyn_end in RAM */
/* * skip first reserved entry: address, type, addend */ - bne t1, t2, 7f + bne t1, t2, 7f
6: - LREG t5, -(REGBYTES*2)(t1) /* t5 <-- relocation info:type */ - li t3, R_RISCV_RELATIVE /* reloc type R_RISCV_RELATIVE */ - bne t5, t3, 8f /* skip non-RISCV_RELOC entries */ - LREG t3, -(REGBYTES*3)(t1) - LREG t5, -(REGBYTES)(t1) /* t5 <-- addend */ - add t5, t5, t6 /* t5 <-- location to fix up in RAM */ - add t3, t3, t6 /* t3 <-- location to fix up in RAM */ - SREG t5, 0(t3) + LREG t5, -(REGBYTES*2)(t1) /* t5 <-- relocation info:type */ + li t3, R_RISCV_RELATIVE /* reloc type R_RISCV_RELATIVE */ + bne t5, t3, 8f /* skip non-RISCV_RELOC entries */ + LREG t3, -(REGBYTES*3)(t1) + LREG t5, -(REGBYTES)(t1) /* t5 <-- addend */ + add t5, t5, t6 /* t5 <-- location to fix up in RAM */ + add t3, t3, t6 /* t3 <-- location to fix up in RAM */ + SREG t5, 0(t3) 7: - addi t1, t1, (REGBYTES*3) - ble t1, t2, 6b + addi t1, t1, (REGBYTES*3) + ble t1, t2, 6b
8: - la t4, __dyn_sym_start - add t4, t4, t6 + la t4, __dyn_sym_start + add t4, t4, t6
9: - LREG t5, -(REGBYTES*2)(t1) /* t5 <-- relocation info:type */ - srli t0, t5, SYM_INDEX /* t0 <--- sym table index */ - andi t5, t5, 0xFF /* t5 <--- relocation type */ - li t3, RELOC_TYPE - bne t5, t3, 10f /* skip non-addned entries */ + LREG t5, -(REGBYTES*2)(t1) /* t5 <-- relocation info:type */ + srli t0, t5, SYM_INDEX /* t0 <--- sym table index */ + andi t5, t5, 0xFF /* t5 <--- relocation type */ + li t3, RELOC_TYPE + bne t5, t3, 10f /* skip non-addned entries */
- LREG t3, -(REGBYTES*3)(t1) - li t5, SYM_SIZE - mul t0, t0, t5 - add s1, t4, t0 - LREG t5, REGBYTES(s1) - add t5, t5, t6 /* t5 <-- location to fix up in RAM */ - add t3, t3, t6 /* t3 <-- location to fix up in RAM */ - SREG t5, 0(t3) + LREG t3, -(REGBYTES*3)(t1) + li t5, SYM_SIZE + mul t0, t0, t5 + add s1, t4, t0 + LREG t5, REGBYTES(s1) + add t5, t5, t6 /* t5 <-- location to fix up in RAM */ + add t3, t3, t6 /* t3 <-- location to fix up in RAM */ + SREG t5, 0(t3) 10: - addi t1, t1, (REGBYTES*3) - ble t1, t2, 9b + addi t1, t1, (REGBYTES*3) + ble t1, t2, 9b
/* * trap update */ - la t0, trap_entry - add t0, t0, t6 - csrw mtvec, t0 + la t0, trap_entry + add t0, t0, t6 + csrw mtvec, t0
clear_bss: - la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ - add t0, t0, t6 /* t0 <- rel __bss_start in RAM */ - la t1, __bss_end /* t1 <- rel __bss_end in FLASH */ - add t1, t1, t6 /* t1 <- rel __bss_end in RAM */ - li t2, 0x00000000 /* clear */ - beq t0, t1, call_board_init_r + la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ + add t0, t0, t6 /* t0 <- rel __bss_start in RAM */ + la t1, __bss_end /* t1 <- rel __bss_end in FLASH */ + add t1, t1, t6 /* t1 <- rel __bss_end in RAM */ + li t2, 0x00000000 /* clear */ + beq t0, t1, call_board_init_r
clbss_l: - SREG t2, 0(t0) /* clear loop... */ - addi t0, t0, REGBYTES - bne t0, t1, clbss_l + SREG t2, 0(t0) /* clear loop... */ + addi t0, t0, REGBYTES + bne t0, t1, clbss_l
/* * We are done. Do not return, instead branch to second part of board * initialization, now running from RAM. */ call_board_init_r: - la t0, board_init_r - mv t4, t0 /* offset of board_init_r() */ - add t4, t4, t6 /* real address of board_init_r() */ + la t0, board_init_r + mv t4, t0 /* offset of board_init_r() */ + add t4, t4, t6 /* real address of board_init_r() */ /* * setup parameters for board_init_r */ - mv a0, s3 /* gd_t */ - mv a1, s4 /* dest_addr */ + mv a0, s3 /* gd_t */ + mv a1, s4 /* dest_addr */
/* * jump to it ... */ - jr t4 /* jump to board_init_r() */ + jr t4 /* jump to board_init_r() */
/* * trap entry */ trap_entry: - addi sp, sp, -32*REGBYTES - SREG x1, 1*REGBYTES(sp) - SREG x2, 2*REGBYTES(sp) - SREG x3, 3*REGBYTES(sp) - SREG x4, 4*REGBYTES(sp) - SREG x5, 5*REGBYTES(sp) - SREG x6, 6*REGBYTES(sp) - SREG x7, 7*REGBYTES(sp) - SREG x8, 8*REGBYTES(sp) - SREG x9, 9*REGBYTES(sp) - SREG x10, 10*REGBYTES(sp) - SREG x11, 11*REGBYTES(sp) - SREG x12, 12*REGBYTES(sp) - SREG x13, 13*REGBYTES(sp) - SREG x14, 14*REGBYTES(sp) - SREG x15, 15*REGBYTES(sp) - SREG x16, 16*REGBYTES(sp) - SREG x17, 17*REGBYTES(sp) - SREG x18, 18*REGBYTES(sp) - SREG x19, 19*REGBYTES(sp) - SREG x20, 20*REGBYTES(sp) - SREG x21, 21*REGBYTES(sp) - SREG x22, 22*REGBYTES(sp) - SREG x23, 23*REGBYTES(sp) - SREG x24, 24*REGBYTES(sp) - SREG x25, 25*REGBYTES(sp) - SREG x26, 26*REGBYTES(sp) - SREG x27, 27*REGBYTES(sp) - SREG x28, 28*REGBYTES(sp) - SREG x29, 29*REGBYTES(sp) - SREG x30, 30*REGBYTES(sp) - SREG x31, 31*REGBYTES(sp) - csrr a0, mcause - csrr a1, mepc - mv a2, sp - jal handle_trap - csrw mepc, a0 + addi sp, sp, -32*REGBYTES + SREG x1, 1*REGBYTES(sp) + SREG x2, 2*REGBYTES(sp) + SREG x3, 3*REGBYTES(sp) + SREG x4, 4*REGBYTES(sp) + SREG x5, 5*REGBYTES(sp) + SREG x6, 6*REGBYTES(sp) + SREG x7, 7*REGBYTES(sp) + SREG x8, 8*REGBYTES(sp) + SREG x9, 9*REGBYTES(sp) + SREG x10, 10*REGBYTES(sp) + SREG x11, 11*REGBYTES(sp) + SREG x12, 12*REGBYTES(sp) + SREG x13, 13*REGBYTES(sp) + SREG x14, 14*REGBYTES(sp) + SREG x15, 15*REGBYTES(sp) + SREG x16, 16*REGBYTES(sp) + SREG x17, 17*REGBYTES(sp) + SREG x18, 18*REGBYTES(sp) + SREG x19, 19*REGBYTES(sp) + SREG x20, 20*REGBYTES(sp) + SREG x21, 21*REGBYTES(sp) + SREG x22, 22*REGBYTES(sp) + SREG x23, 23*REGBYTES(sp) + SREG x24, 24*REGBYTES(sp) + SREG x25, 25*REGBYTES(sp) + SREG x26, 26*REGBYTES(sp) + SREG x27, 27*REGBYTES(sp) + SREG x28, 28*REGBYTES(sp) + SREG x29, 29*REGBYTES(sp) + SREG x30, 30*REGBYTES(sp) + SREG x31, 31*REGBYTES(sp) + csrr a0, mcause + csrr a1, mepc + mv a2, sp + jal handle_trap + csrw mepc, a0
/* * Remain in M-mode after mret */ - li t0, MSTATUS_MPP - csrs mstatus, t0 - LREG x1, 1*REGBYTES(sp) - LREG x2, 2*REGBYTES(sp) - LREG x3, 3*REGBYTES(sp) - LREG x4, 4*REGBYTES(sp) - LREG x5, 5*REGBYTES(sp) - LREG x6, 6*REGBYTES(sp) - LREG x7, 7*REGBYTES(sp) - LREG x8, 8*REGBYTES(sp) - LREG x9, 9*REGBYTES(sp) - LREG x10, 10*REGBYTES(sp) - LREG x11, 11*REGBYTES(sp) - LREG x12, 12*REGBYTES(sp) - LREG x13, 13*REGBYTES(sp) - LREG x14, 14*REGBYTES(sp) - LREG x15, 15*REGBYTES(sp) - LREG x16, 16*REGBYTES(sp) - LREG x17, 17*REGBYTES(sp) - LREG x18, 18*REGBYTES(sp) - LREG x19, 19*REGBYTES(sp) - LREG x20, 20*REGBYTES(sp) - LREG x21, 21*REGBYTES(sp) - LREG x22, 22*REGBYTES(sp) - LREG x23, 23*REGBYTES(sp) - LREG x24, 24*REGBYTES(sp) - LREG x25, 25*REGBYTES(sp) - LREG x26, 26*REGBYTES(sp) - LREG x27, 27*REGBYTES(sp) - LREG x28, 28*REGBYTES(sp) - LREG x29, 29*REGBYTES(sp) - LREG x30, 30*REGBYTES(sp) - LREG x31, 31*REGBYTES(sp) - addi sp, sp, 32*REGBYTES + li t0, MSTATUS_MPP + csrs mstatus, t0 + LREG x1, 1*REGBYTES(sp) + LREG x2, 2*REGBYTES(sp) + LREG x3, 3*REGBYTES(sp) + LREG x4, 4*REGBYTES(sp) + LREG x5, 5*REGBYTES(sp) + LREG x6, 6*REGBYTES(sp) + LREG x7, 7*REGBYTES(sp) + LREG x8, 8*REGBYTES(sp) + LREG x9, 9*REGBYTES(sp) + LREG x10, 10*REGBYTES(sp) + LREG x11, 11*REGBYTES(sp) + LREG x12, 12*REGBYTES(sp) + LREG x13, 13*REGBYTES(sp) + LREG x14, 14*REGBYTES(sp) + LREG x15, 15*REGBYTES(sp) + LREG x16, 16*REGBYTES(sp) + LREG x17, 17*REGBYTES(sp) + LREG x18, 18*REGBYTES(sp) + LREG x19, 19*REGBYTES(sp) + LREG x20, 20*REGBYTES(sp) + LREG x21, 21*REGBYTES(sp) + LREG x22, 22*REGBYTES(sp) + LREG x23, 23*REGBYTES(sp) + LREG x24, 24*REGBYTES(sp) + LREG x25, 25*REGBYTES(sp) + LREG x26, 26*REGBYTES(sp) + LREG x27, 27*REGBYTES(sp) + LREG x28, 28*REGBYTES(sp) + LREG x29, 29*REGBYTES(sp) + LREG x30, 30*REGBYTES(sp) + LREG x31, 31*REGBYTES(sp) + addi sp, sp, 32*REGBYTES mret
#ifdef CONFIG_INIT_CRITICAL

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Start.S uses both tabs and spaces after instructions. Fix this by only
nits: start.S
using tabs after instructions.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 322 ++++++++++++++++++++--------------------- 1 file changed, 161 insertions(+), 161 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin

The machine trap-vector base address (mtvec) must be aligned on a 4-byte boundary. Add the necessary align directive to trap_entry.
This patch also removes the global directive for trap_entry, which is not required.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/cpu/start.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index bd5904500c..88b4aaa1c0 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -42,7 +42,6 @@ nmi_vector: trap_vector: j trap_entry
-.global trap_entry handle_reset: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) @@ -208,6 +207,7 @@ call_board_init_r: /* * trap entry */ +.align 2 trap_entry: addi sp, sp, -32*REGBYTES SREG x1, 1*REGBYTES(sp)

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
The machine trap-vector base address (mtvec) must be aligned on a 4-byte boundary. Add the necessary align directive to trap_entry.
I don't think this explicit alignment is needed because the instructions before trap_entry are already on 4-byte boundary.
This patch also removes the global directive for trap_entry, which is not required.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Regards, Bin

CONFIG_INIT_CRITICAL is deprecated and not used for RISC-V. Remove it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/cpu/start.S | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 88b4aaa1c0..f375a9316e 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -50,13 +50,6 @@ handle_reset: csrwi mstatus, 0 csrwi mie, 0
-/* - * Do CPU critical regs init only at reboot, - * not when booting from ram - */ -#ifdef CONFIG_INIT_CRITICAL - jal cpu_init_crit /* Do CPU critical regs init */ -#endif
/* * Set stackpointer in internal/ex RAM to call board_init_f @@ -286,7 +279,3 @@ trap_entry: addi sp, sp, 32*REGBYTES mret
-#ifdef CONFIG_INIT_CRITICAL -cpu_init_crit: - ret -#endif

Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
CONFIG_INIT_CRITICAL is deprecated and not used for RISC-V. Remove it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 11 ----------- 1 file changed, 11 deletions(-)
Please see my patch which does more :)
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=1f1c216d426a6c219...
Regards, Bin

Hi Lukas,
On Mon, Oct 22, 2018 at 5:19 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
CONFIG_INIT_CRITICAL is deprecated and not used for RISC-V. Remove it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 11 ----------- 1 file changed, 11 deletions(-)
Please see my patch which does more :)
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=1f1c216d426a6c219...
You can take my patch above in your v2 if you want.
Regards, Bin

Hi Bin,
On Thu, 2018-10-25 at 10:57 +0800, Bin Meng wrote:
Hi Lukas,
On Mon, Oct 22, 2018 at 5:19 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
CONFIG_INIT_CRITICAL is deprecated and not used for RISC-V. Remove it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 11 ----------- 1 file changed, 11 deletions(-)
Please see my patch which does more :)
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=1f1c216d426a6c219...
You can take my patch above in your v2 if you want.
Regards, Bin
That looks very good! Yes, I'll replace this patch with yours in v2.
Thanks, Lukas

The labels nmi_vector, trap_vector and handle_reset in start.S are not used for RISC-V. Remove them.
While we are here, also remove the code from the beginning of start.S, which stores the contents of a2 to memory. Only registers a0 and a1 contain information from the previous boot stage. There is therefore no reason for saving a2.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/cpu/start.S | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index f375a9316e..851a1d0870 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -34,17 +34,6 @@ .section .text .globl _start _start: - j handle_reset - -nmi_vector: - j nmi_vector - -trap_vector: - j trap_entry - -handle_reset: - li t0, CONFIG_SYS_SDRAM_BASE - SREG a2, 0(t0) la t0, trap_entry csrw mtvec, t0 csrwi mstatus, 0

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
The labels nmi_vector, trap_vector and handle_reset in start.S are not used for RISC-V. Remove them.
While we are here, also remove the code from the beginning of start.S, which stores the contents of a2 to memory. Only registers a0 and a1 contain information from the previous boot stage. There is therefore no reason for saving a2.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 11 ----------- 1 file changed, 11 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

The mstatus CSR includes WPRI (writes preserve values, reads ignore values) fields and must therefore not be set to zero without preserving these fields. It is not apparent why mstatus is set to zero here since it is not required for u-boot to run. Remove it.
This instruction and others encode zero as an immediate. RISC-V has the zero register for this purpose. Replace the immediates with the zero register.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/cpu/start.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 851a1d0870..4fa663c6d6 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -36,9 +36,9 @@ _start: la t0, trap_entry csrw mtvec, t0 - csrwi mstatus, 0 - csrwi mie, 0
+ /* mask all interrupts */ + csrw mie, zero
/* * Set stackpointer in internal/ex RAM to call board_init_f @@ -159,11 +159,10 @@ clear_bss: add t0, t0, t6 /* t0 <- rel __bss_start in RAM */ la t1, __bss_end /* t1 <- rel __bss_end in FLASH */ add t1, t1, t6 /* t1 <- rel __bss_end in RAM */ - li t2, 0x00000000 /* clear */ beq t0, t1, call_board_init_r
clbss_l: - SREG t2, 0(t0) /* clear loop... */ + SREG zero, 0(t0) /* clear loop... */ addi t0, t0, REGBYTES bne t0, t1, clbss_l

On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
The mstatus CSR includes WPRI (writes preserve values, reads ignore values) fields and must therefore not be set to zero without preserving these fields. It is not apparent why mstatus is set to zero here since it is not required for u-boot to run. Remove it.
nits: U-Boot
This instruction and others encode zero as an immediate. RISC-V has the zero register for this purpose. Replace the immediates with the zero register.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/start.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Store the hart ID and device tree passed by the prior boot stage (in a0 and a1) in registers s0 and s1. Replace one use of s1 in start.S to avoid overwriting it.
The device tree is also stored in memory to make it available to u-boot with the configuration CONFIG_OF_PRIOR_STAGE.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/cpu/cpu.c | 6 ++++++ arch/riscv/cpu/start.S | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index ae57fb8313..d9f820c44c 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,12 @@ #include <common.h> #include <asm/csr.h>
+/* + * prior_stage_fdt_address must be stored in the data section since it is used + * before the bss section is available. + */ +phys_addr_t prior_stage_fdt_address __attribute__((section(".data"))); + enum { ISA_INVALID = 0, ISA_32BIT, diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 4fa663c6d6..8f96c5dbe8 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -34,6 +34,10 @@ .section .text .globl _start _start: + /* save hart id and dtb pointer */ + mv s0, a0 + mv s1, a1 + la t0, trap_entry csrw mtvec, t0
@@ -56,6 +60,10 @@ call_board_init_f_0: mv a0, sp jal board_init_f_alloc_reserve mv sp, a0 + + la t0, prior_stage_fdt_address + SREG s1, 0(t0) + jal board_init_f_init_reserve
mv a0, zero /* a0 <-- boot_flags = 0 */ @@ -138,8 +146,8 @@ fix_rela_dyn: LREG t3, -(REGBYTES*3)(t1) li t5, SYM_SIZE mul t0, t0, t5 - add s1, t4, t0 - LREG t5, REGBYTES(s1) + add s5, t4, t0 + LREG t5, REGBYTES(s5) add t5, t5, t6 /* t5 <-- location to fix up in RAM */ add t3, t3, t6 /* t3 <-- location to fix up in RAM */ SREG t5, 0(t3)

On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Store the hart ID and device tree passed by the prior boot stage (in a0 and a1) in registers s0 and s1. Replace one use of s1 in start.S to avoid overwriting it.
The device tree is also stored in memory to make it available to u-boot
nits: U-Boot
with the configuration CONFIG_OF_PRIOR_STAGE.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/cpu/cpu.c | 6 ++++++ arch/riscv/cpu/start.S | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

QEMU provides a device tree, which is passed to u-boot using register a1. We are now able to directly select the device tree with the configuration CONFIG_OF_PRIOR_STAGE. Replace the hard-coded address in qemu-riscv with it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
board/emulation/qemu-riscv/qemu-riscv.c | 11 ----------- configs/qemu-riscv32_defconfig | 2 +- configs/qemu-riscv64_defconfig | 2 +- 3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c index 041e716c9b..a7dc1d847e 100644 --- a/board/emulation/qemu-riscv/qemu-riscv.c +++ b/board/emulation/qemu-riscv/qemu-riscv.c @@ -6,18 +6,7 @@ #include <common.h> #include <fdtdec.h>
-#define MROM_FDT_ADDR 0x1020 - int board_init(void) { return 0; } - -void *board_fdt_blob_setup(void) -{ - /* - * QEMU loads a generated DTB for us immediately - * after the reset vectors in the MROM - */ - return (void *)MROM_FDT_ADDR; -} diff --git a/configs/qemu-riscv32_defconfig b/configs/qemu-riscv32_defconfig index ff1fb1f30e..72c54a3dd5 100644 --- a/configs/qemu-riscv32_defconfig +++ b/configs/qemu-riscv32_defconfig @@ -3,4 +3,4 @@ CONFIG_TARGET_QEMU_VIRT=y CONFIG_NR_DRAM_BANKS=1 CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y -CONFIG_OF_BOARD=y +CONFIG_OF_PRIOR_STAGE=y diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig index 60b647efe8..44766c38f6 100644 --- a/configs/qemu-riscv64_defconfig +++ b/configs/qemu-riscv64_defconfig @@ -4,4 +4,4 @@ CONFIG_ARCH_RV64I=y CONFIG_NR_DRAM_BANKS=1 CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y -CONFIG_OF_BOARD=y +CONFIG_OF_PRIOR_STAGE=y

On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
QEMU provides a device tree, which is passed to u-boot using register
nits: U-Boot
a1. We are now able to directly select the device tree with the configuration CONFIG_OF_PRIOR_STAGE. Replace the hard-coded address in qemu-riscv with it.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
board/emulation/qemu-riscv/qemu-riscv.c | 11 ----------- configs/qemu-riscv32_defconfig | 2 +- configs/qemu-riscv64_defconfig | 2 +- 3 files changed, 2 insertions(+), 13 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Print the address of the u-boot device tree.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
cmd/bdinfo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index 60b438766d..a9692f7662 100644 --- a/cmd/bdinfo.c +++ b/cmd/bdinfo.c @@ -430,6 +430,8 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) print_num("reloc off", gd->reloc_off); print_eth_ip_addr(); print_baudrate(); + if (gd->fdt_blob) + print_num("fdt_blob", (ulong)gd->fdt_blob);
return 0; }

Hi Lukas,
On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Print the address of the u-boot device tree.
This is unnecessary as it is already done by 'fdt' command.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
cmd/bdinfo.c | 2 ++ 1 file changed, 2 insertions(+)
Regards, Bin

Support booting Linux (as payload of BBL) from FIT images. For this, the default CONFIG_SYS_BOOTM_LEN is increased to 16 MB, and the environment variables fdt_high and initrd_high are set to mark the device tree and initrd as in-place.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
configs/qemu-riscv32_defconfig | 1 + configs/qemu-riscv64_defconfig | 1 + include/configs/qemu-riscv.h | 6 ++++++ 3 files changed, 8 insertions(+)
diff --git a/configs/qemu-riscv32_defconfig b/configs/qemu-riscv32_defconfig index 72c54a3dd5..b55644378a 100644 --- a/configs/qemu-riscv32_defconfig +++ b/configs/qemu-riscv32_defconfig @@ -1,6 +1,7 @@ CONFIG_RISCV=y CONFIG_TARGET_QEMU_VIRT=y CONFIG_NR_DRAM_BANKS=1 +CONFIG_FIT=y CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y CONFIG_OF_PRIOR_STAGE=y diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig index 44766c38f6..a542ac4893 100644 --- a/configs/qemu-riscv64_defconfig +++ b/configs/qemu-riscv64_defconfig @@ -2,6 +2,7 @@ CONFIG_RISCV=y CONFIG_TARGET_QEMU_VIRT=y CONFIG_ARCH_RV64I=y CONFIG_NR_DRAM_BANKS=1 +CONFIG_FIT=y CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y CONFIG_OF_PRIOR_STAGE=y diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h index d279c233b2..ba6a18f2e6 100644 --- a/include/configs/qemu-riscv.h +++ b/include/configs/qemu-riscv.h @@ -15,7 +15,13 @@
#define CONFIG_SYS_MALLOC_LEN SZ_8M
+#define CONFIG_SYS_BOOTM_LEN SZ_16M + /* Environment options */ #define CONFIG_ENV_SIZE SZ_4K
+#define CONFIG_EXTRA_ENV_SETTINGS \ + "fdt_high=0xffffffffffffffff\0" \ + "initrd_high=0xffffffffffffffff\0" + #endif /* __CONFIG_H */

On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Support booting Linux (as payload of BBL) from FIT images. For this, the default CONFIG_SYS_BOOTM_LEN is increased to 16 MB, and the environment variables fdt_high and initrd_high are set to mark the device tree and initrd as in-place.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
configs/qemu-riscv32_defconfig | 1 + configs/qemu-riscv64_defconfig | 1 + include/configs/qemu-riscv.h | 6 ++++++ 3 files changed, 8 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

The bootm implementation of RISC-V diverges from that of other architectures. Update it to match the implementation of other architectures. The ARM implementation is used as a reference.
This adds the following features and changes to RISC-V. * Add support for the BOOTM_STATE_OS_FAKE_GO command * Call the remove function on devices with the removal flag set before booting Linux * Force disconnect USB devices from the host before booting Linux * Print and add bootstage information to the device tree before booting Linux
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
arch/riscv/lib/bootm.c | 94 ++++++++++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 22 deletions(-)
diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c index bc1d4b2864..b4e18a768a 100644 --- a/arch/riscv/lib/bootm.c +++ b/arch/riscv/lib/bootm.c @@ -8,6 +8,8 @@
#include <common.h> #include <command.h> +#include <dm.h> +#include <dm/root.h> #include <image.h> #include <u-boot/zlib.h> #include <asm/byteorder.h> @@ -24,27 +26,41 @@ int arch_fixup_fdt(void *blob) return 0; }
-int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) +/** + * announce_and_cleanup() - Print message and prepare for kernel boot + * + * @fake: non-zero to do everything except actually boot + */ +static void announce_and_cleanup(int fake) { - void (*kernel)(ulong hart, void *dtb); - - /* - * allow the PREP bootm subcommand, it is required for bootm to work - */ - if (flag & BOOTM_STATE_OS_PREP) - return 0; + printf("\nStarting kernel ...%s\n\n", fake ? + "(fake run for tracing)" : ""); + bootstage_mark_name(BOOTSTAGE_ID_BOOTM_HANDOFF, "start_kernel"); +#ifdef CONFIG_BOOTSTAGE_FDT + bootstage_fdt_add_report(); +#endif +#ifdef CONFIG_BOOTSTAGE_REPORT + bootstage_report(); +#endif
- if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) - return 1; +#ifdef CONFIG_USB_DEVICE + udc_disconnect(); +#endif
- kernel = (void (*)(ulong, void *))images->ep; - invalidate_icache_all(); + board_quiesce_devices();
- bootstage_mark(BOOTSTAGE_ID_RUN_OS); + /* + * Call remove function of all devices with a removal flag set. + * This may be useful for last-stage operations, like cancelling + * of DMA operation or releasing device internal buffers. + */ + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);
- debug("## Transferring control to Linux (at address %08lx) ...\n", - (ulong)kernel); + cleanup_before_linux(); +}
+static void boot_prep_linux(bootm_headers_t *images) +{ if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) { #ifdef CONFIG_OF_LIBFDT debug("using: FDT\n"); @@ -53,17 +69,51 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) hang(); } #endif + } else { + printf("Device tree not found or missing FDT support\n"); + hang(); } +}
- /* we assume that the kernel is in place */ - printf("\nStarting kernel ...\n\n"); +static void boot_jump_linux(bootm_headers_t *images, int flag) +{ + void (*kernel)(ulong hart, void *dtb); + int fake = (flag & BOOTM_STATE_OS_FAKE_GO);
- cleanup_before_linux(); + kernel = (void (*)(ulong, void *))images->ep; + invalidate_icache_all(); + + bootstage_mark(BOOTSTAGE_ID_RUN_OS); + + debug("## Transferring control to Linux (at address %08lx) ...\n", + (ulong)kernel); + + announce_and_cleanup(fake); + + if (!fake) { + if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) + kernel(csr_read(mhartid), images->ft_addr); + } +}
- if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) - kernel(csr_read(mhartid), images->ft_addr); +int do_bootm_linux(int flag, int argc, char * const argv[], + bootm_headers_t *images) +{ + /* No need for those on RISC-V */ + if (flag & BOOTM_STATE_OS_BD_T || flag & BOOTM_STATE_OS_CMDLINE) + return -1;
- /* does not return */ + if (flag & BOOTM_STATE_OS_PREP) { + boot_prep_linux(images); + return 0; + }
- return 1; + if (flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)) { + boot_jump_linux(images, flag); + return 0; + } + + boot_prep_linux(images); + boot_jump_linux(images, flag); + return 0; }

On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
The bootm implementation of RISC-V diverges from that of other architectures. Update it to match the implementation of other architectures. The ARM implementation is used as a reference.
This adds the following features and changes to RISC-V.
- Add support for the BOOTM_STATE_OS_FAKE_GO command
- Call the remove function on devices with the removal flag set before
booting Linux
- Force disconnect USB devices from the host before booting Linux
- Print and add bootstage information to the device tree before booting
Linux
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/lib/bootm.c | 94 ++++++++++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 22 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
include/dm/ofnode.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 2fc9fa39a3..a7b8609cf4 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -216,6 +216,16 @@ static inline int ofnode_read_s32(ofnode node, const char *propname, return ofnode_read_u32(node, propname, (u32 *)out_value); }
+/** + * ofnode_read_u64() - Read a 64-bit integer from a property + * + * @node: valid node reference to read property from + * @propname: name of the property to read from + * @outp: place to put value (if found) + * @return 0 if OK, -ve on error + */ +int ofnode_read_u64(ofnode node, const char *propname, u64 *outp); + /** * ofnode_read_u32_default() - Read a 32-bit integer from a property *

Hi Lukas,
On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
include/dm/ofnode.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 2fc9fa39a3..a7b8609cf4 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -216,6 +216,16 @@ static inline int ofnode_read_s32(ofnode node, const char *propname, return ofnode_read_u32(node, propname, (u32 *)out_value); }
+/**
- ofnode_read_u64() - Read a 64-bit integer from a property
- @node: valid node reference to read property from
- @propname: name of the property to read from
- @outp: place to put value (if found)
- @return 0 if OK, -ve on error
- */
+int ofnode_read_u64(ofnode node, const char *propname, u64 *outp);
Can you please put this at the same location in ofnode.c? eg: put after ofnode_read_s32_default()
/**
- ofnode_read_u32_default() - Read a 32-bit integer from a property
--
Regards, Bin

Hi Bin,
On Mon, 2018-10-22 at 17:35 +0800, Bin Meng wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
include/dm/ofnode.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 2fc9fa39a3..a7b8609cf4 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -216,6 +216,16 @@ static inline int ofnode_read_s32(ofnode node, const char *propname, return ofnode_read_u32(node, propname, (u32 *)out_value); }
+/**
- ofnode_read_u64() - Read a 64-bit integer from a property
- @node: valid node reference to read property from
- @propname: name of the property to read from
- @outp: place to put value (if found)
- @return 0 if OK, -ve on error
- */
+int ofnode_read_u64(ofnode node, const char *propname, u64 *outp);
Can you please put this at the same location in ofnode.c? eg: put after ofnode_read_s32_default()
Yes, I will fix this in v2.
Thanks, Lukas
/**
- ofnode_read_u32_default() - Read a 32-bit integer from a
property
--
Regards, Bin

QEMU embeds the location of the kernel image in the device tree. Store this address in the environment as variable kernel_start and use it in CONFIG_BOOTCOMMAND to boot the kernel.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de ---
board/emulation/qemu-riscv/Kconfig | 1 + board/emulation/qemu-riscv/qemu-riscv.c | 30 +++++++++++++++++++++++++ configs/qemu-riscv32_defconfig | 1 + configs/qemu-riscv64_defconfig | 1 + include/configs/qemu-riscv.h | 7 ++++++ 5 files changed, 40 insertions(+)
diff --git a/board/emulation/qemu-riscv/Kconfig b/board/emulation/qemu-riscv/Kconfig index af23363fcf..aba3e757a1 100644 --- a/board/emulation/qemu-riscv/Kconfig +++ b/board/emulation/qemu-riscv/Kconfig @@ -18,5 +18,6 @@ config SYS_TEXT_BASE config BOARD_SPECIFIC_OPTIONS # dummy def_bool y imply SYS_NS16550 + imply BOARD_LATE_INIT
endif diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c index a7dc1d847e..ed1a0e7729 100644 --- a/board/emulation/qemu-riscv/qemu-riscv.c +++ b/board/emulation/qemu-riscv/qemu-riscv.c @@ -4,9 +4,39 @@ */
#include <common.h> +#include <dm.h> #include <fdtdec.h>
int board_init(void) { return 0; } + +int board_late_init(void) +{ + ulong kernel_start; + ofnode chosen_node; + int ret; + + chosen_node = ofnode_path("/chosen"); + if (!ofnode_valid(chosen_node)) { + printf("No chosen node found\n"); + return 0; + } + +#ifdef CONFIG_ARCH_RV64I + ret = ofnode_read_u64(chosen_node, "riscv,kernel-start", + (u64 *)&kernel_start); +#else + ret = ofnode_read_u32(chosen_node, "riscv,kernel-start", + (u32 *)&kernel_start); +#endif + if (ret) { + printf("Can't find kernel start address in device tree\n"); + return 0; + } + + env_set_hex("kernel_start", kernel_start); + + return 0; +} diff --git a/configs/qemu-riscv32_defconfig b/configs/qemu-riscv32_defconfig index b55644378a..13dd53a550 100644 --- a/configs/qemu-riscv32_defconfig +++ b/configs/qemu-riscv32_defconfig @@ -4,4 +4,5 @@ CONFIG_NR_DRAM_BANKS=1 CONFIG_FIT=y CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y +CONFIG_HUSH_PARSER=y CONFIG_OF_PRIOR_STAGE=y diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig index a542ac4893..06eb3042fa 100644 --- a/configs/qemu-riscv64_defconfig +++ b/configs/qemu-riscv64_defconfig @@ -5,4 +5,5 @@ CONFIG_NR_DRAM_BANKS=1 CONFIG_FIT=y CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y +CONFIG_HUSH_PARSER=y CONFIG_OF_PRIOR_STAGE=y diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h index ba6a18f2e6..2d5ac6a181 100644 --- a/include/configs/qemu-riscv.h +++ b/include/configs/qemu-riscv.h @@ -24,4 +24,11 @@ "fdt_high=0xffffffffffffffff\0" \ "initrd_high=0xffffffffffffffff\0"
+#define CONFIG_BOOTCOMMAND \ + "if env exists kernel_start; then " \ + "bootm ${kernel_start};" \ + "else " \ + "echo Kernel address not found in the device tree;" \ + "fi;" + #endif /* __CONFIG_H */

On Sat, Oct 20, 2018 at 6:11 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
QEMU embeds the location of the kernel image in the device tree. Store this address in the environment as variable kernel_start and use it in CONFIG_BOOTCOMMAND to boot the kernel.
Signed-off-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
board/emulation/qemu-riscv/Kconfig | 1 + board/emulation/qemu-riscv/qemu-riscv.c | 30 +++++++++++++++++++++++++ configs/qemu-riscv32_defconfig | 1 + configs/qemu-riscv64_defconfig | 1 + include/configs/qemu-riscv.h | 7 ++++++ 5 files changed, 40 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Lukas,
On Sat, Oct 20, 2018 at 6:08 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
This patch series includes general fixes and cleanup for RISC-V. It also adds support for booting Linux on qemu-riscv. At the moment, only single-core systems are supported. Support for multi-core systems will be added with a future patch series.
To boot Linux on qemu-riscv, Linux must be compiled into BBL as a payload. BBL must be included in a FIT image and supplied to QEMU with the -kernel parameter. Its location in memory is embedded in the device tree, which QEMU passes to u-boot. To test this, QEMU and riscv-pk (BBL) must be modified. QEMU is modified to add support for loading binary files (FIT images in this case) in addition to ELF files. riscv-pk must be modified to adjust the link address and to ignore the kernel address from the device tree. A pull request for QEMU, which implements this, is available at [1]. A modified version of riscv-pk is available at [2].
Thanks for these patches. I have reviewed all patches and sent out my comments. Good work!
Regards, Bin

Hi Lukas,
On Mon, Oct 22, 2018 at 5:37 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:08 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
This patch series includes general fixes and cleanup for RISC-V. It also adds support for booting Linux on qemu-riscv. At the moment, only single-core systems are supported. Support for multi-core systems will be added with a future patch series.
To boot Linux on qemu-riscv, Linux must be compiled into BBL as a payload. BBL must be included in a FIT image and supplied to QEMU with the -kernel parameter. Its location in memory is embedded in the device tree, which QEMU passes to u-boot. To test this, QEMU and riscv-pk (BBL) must be modified. QEMU is modified to add support for loading binary files (FIT images in this case) in addition to ELF files. riscv-pk must be modified to adjust the link address and to ignore the kernel address from the device tree. A pull request for QEMU, which implements this, is available at [1]. A modified version of riscv-pk is available at [2].
Thanks for these patches. I have reviewed all patches and sent out my comments. Good work!
When you spin the v2 series, is it possible to rebase the series on top of u-boot-dm/next [1] branch, which contains the virtio support, so that we can test QEMU RISC-V with virtio? Thanks!
[1] http://git.denx.de/?p=u-boot/u-boot-dm.git;a=shortlog;h=refs/heads/next
Regards, Bin

Hi Bin,
On Fri, 2018-10-26 at 21:20 +0800, Bin Meng wrote:
Hi Lukas,
On Mon, Oct 22, 2018 at 5:37 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Lukas,
On Sat, Oct 20, 2018 at 6:08 AM Lukas Auer lukas.auer@aisec.fraunhofer.de wrote:
This patch series includes general fixes and cleanup for RISC-V. It also adds support for booting Linux on qemu-riscv. At the moment, only single-core systems are supported. Support for multi-core systems will be added with a future patch series.
To boot Linux on qemu-riscv, Linux must be compiled into BBL as a payload. BBL must be included in a FIT image and supplied to QEMU with the -kernel parameter. Its location in memory is embedded in the device tree, which QEMU passes to u-boot. To test this, QEMU and riscv-pk (BBL) must be modified. QEMU is modified to add support for loading binary files (FIT images in this case) in addition to ELF files. riscv-pk must be modified to adjust the link address and to ignore the kernel address from the device tree. A pull request for QEMU, which implements this, is available at [1]. A modified version of riscv-pk is available at [2].
Thanks for these patches. I have reviewed all patches and sent out my comments. Good work!
When you spin the v2 series, is it possible to rebase the series on top of u-boot-dm/next [1] branch, which contains the virtio support, so that we can test QEMU RISC-V with virtio? Thanks!
[1] http://git.denx.de/?p=u-boot/u-boot-dm.git;a=shortlog;h=refs/heads/next
Regards, Bin
Thank you for your reviews! Yes, I will do that for v2.
Thanks, Lukas
participants (3)
-
Auer, Lukas
-
Bin Meng
-
Lukas Auer