| Message ID | 20200311070320.21323-13-pragnesh.patel@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V SiFive FU540 support SPL | expand |
On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel <pragnesh.patel at sifive.com> wrote: > > Enable all cache ways from u-boot proper. U-Boot > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > --- > board/sifive/fu540/Makefile | 1 + > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > board/sifive/fu540/cache.h | 13 +++++++++++++ > board/sifive/fu540/fu540.c | 6 ++++-- > 4 files changed, 38 insertions(+), 2 deletions(-) > create mode 100644 board/sifive/fu540/cache.c > create mode 100644 board/sifive/fu540/cache.h > > diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile > index b05e2f5807..3b867bbd89 100644 > --- a/board/sifive/fu540/Makefile > +++ b/board/sifive/fu540/Makefile > @@ -3,6 +3,7 @@ > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > > obj-y += fu540.o > +obj-y += cache.o > > ifdef CONFIG_SPL_BUILD > obj-y += spl.o > diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c > new file mode 100644 > index 0000000000..a0bcd2ba48 > --- /dev/null > +++ b/board/sifive/fu540/cache.c This should be put into arch/riscv/cpu/fu540/cache.c > @@ -0,0 +1,20 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2019 SiFive, Inc > + */ > +#include <asm/io.h> > + > +/* Register offsets */ > +#define CACHE_ENABLE 0x008 > + > +/* Enable ways; allow cache to use these ways */ > +void cache_enable_ways(u64 base_addr, u8 value) > +{ > + volatile u32 *enable = (volatile u32 *)(base_addr + > + CACHE_ENABLE); > + /* memory barrier */ > + mb(); > + (*enable) = value; > + /* memory barrier */ > + mb(); > +} > diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h > new file mode 100644 > index 0000000000..425124a23b > --- /dev/null > +++ b/board/sifive/fu540/cache.h arch/riscv/include/asm/arch-fu540/cache.h > @@ -0,0 +1,13 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2019 SiFive, Inc > + */ > + > +#ifndef FU540_CACHE_H > +#define FU540_CACHE_H > + > +#define CACHE_CTRL_ADDR _AC(0x2010000, UL) > + > +void cache_enable_ways(u64 base_addr, u8 value); > + > +#endif /* FU540_CACHE_H */ > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c > index 89a65eb3fb..1d6c0c9bba 100644 > --- a/board/sifive/fu540/fu540.c > +++ b/board/sifive/fu540/fu540.c > @@ -13,6 +13,8 @@ > #include <misc.h> > #include <spl.h> > > +#include "cache.h" > + > /* > * This define is a value used for error/unknown serial. > * If we really care about distinguishing errors and 0 is > @@ -111,8 +113,8 @@ int misc_init_r(void) > > int board_init(void) > { > - /* For now nothing to do here. */ > - > + /* enable all cache ways */ > + cache_enable_ways(CACHE_CTRL_ADDR, 15); > return 0; > } > Regards, Bin
On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel > <pragnesh.patel at sifive.com> wrote: > > > > Enable all cache ways from u-boot proper. > > U-Boot > > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > > --- > > board/sifive/fu540/Makefile | 1 + > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > > board/sifive/fu540/cache.h | 13 +++++++++++++ > > board/sifive/fu540/fu540.c | 6 ++++-- > > 4 files changed, 38 insertions(+), 2 deletions(-) > > create mode 100644 board/sifive/fu540/cache.c > > create mode 100644 board/sifive/fu540/cache.h > > > > diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile > > index b05e2f5807..3b867bbd89 100644 > > --- a/board/sifive/fu540/Makefile > > +++ b/board/sifive/fu540/Makefile > > @@ -3,6 +3,7 @@ > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > > obj-y += fu540.o > > +obj-y += cache.o > > > > ifdef CONFIG_SPL_BUILD > > obj-y += spl.o > > diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c > > new file mode 100644 > > index 0000000000..a0bcd2ba48 > > --- /dev/null > > +++ b/board/sifive/fu540/cache.c > > This should be put into arch/riscv/cpu/fu540/cache.c > > > @@ -0,0 +1,20 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2019 SiFive, Inc > > + */ > > +#include <asm/io.h> > > + > > +/* Register offsets */ > > +#define CACHE_ENABLE 0x008 > > + > > +/* Enable ways; allow cache to use these ways */ > > +void cache_enable_ways(u64 base_addr, u8 value) > > +{ > > + volatile u32 *enable = (volatile u32 *)(base_addr + > > + CACHE_ENABLE); > > + /* memory barrier */ > > + mb(); > > + (*enable) = value; > > + /* memory barrier */ > > + mb(); > > +} > > diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h > > new file mode 100644 > > index 0000000000..425124a23b > > --- /dev/null > > +++ b/board/sifive/fu540/cache.h > > arch/riscv/include/asm/arch-fu540/cache.h Let's not entire FU540 directory under arch/riscv/cpu directory just to have cache functions. The arch/riscv/cpu/generic is perfectly suitable for FU540. If we re-use arch/riscv/cpu/generic as-much as possible then arch/riscv will be easy to maintain in future. We can add arch/riscv/cpu/generic/cache.c which will do things FU540 specific based on "#ifdef" or "DT compatible string". > > > @@ -0,0 +1,13 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * Copyright (c) 2019 SiFive, Inc > > + */ > > + > > +#ifndef FU540_CACHE_H > > +#define FU540_CACHE_H > > + > > +#define CACHE_CTRL_ADDR _AC(0x2010000, UL) > > + > > +void cache_enable_ways(u64 base_addr, u8 value); > > + > > +#endif /* FU540_CACHE_H */ > > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c > > index 89a65eb3fb..1d6c0c9bba 100644 > > --- a/board/sifive/fu540/fu540.c > > +++ b/board/sifive/fu540/fu540.c > > @@ -13,6 +13,8 @@ > > #include <misc.h> > > #include <spl.h> > > > > +#include "cache.h" > > + > > /* > > * This define is a value used for error/unknown serial. > > * If we really care about distinguishing errors and 0 is > > @@ -111,8 +113,8 @@ int misc_init_r(void) > > > > int board_init(void) > > { > > - /* For now nothing to do here. */ > > - > > + /* enable all cache ways */ > > + cache_enable_ways(CACHE_CTRL_ADDR, 15); > > return 0; > > } > > > > Regards, > Bin Regards, Anup
Hi Anup, On Fri, Mar 13, 2020 at 6:02 PM Anup Patel <anup at brainfault.org> wrote: > > On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel > > <pragnesh.patel at sifive.com> wrote: > > > > > > Enable all cache ways from u-boot proper. > > > > U-Boot > > > > > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > > > --- > > > board/sifive/fu540/Makefile | 1 + > > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > > > board/sifive/fu540/cache.h | 13 +++++++++++++ > > > board/sifive/fu540/fu540.c | 6 ++++-- > > > 4 files changed, 38 insertions(+), 2 deletions(-) > > > create mode 100644 board/sifive/fu540/cache.c > > > create mode 100644 board/sifive/fu540/cache.h > > > > > > diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile > > > index b05e2f5807..3b867bbd89 100644 > > > --- a/board/sifive/fu540/Makefile > > > +++ b/board/sifive/fu540/Makefile > > > @@ -3,6 +3,7 @@ > > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > > > > obj-y += fu540.o > > > +obj-y += cache.o > > > > > > ifdef CONFIG_SPL_BUILD > > > obj-y += spl.o > > > diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c > > > new file mode 100644 > > > index 0000000000..a0bcd2ba48 > > > --- /dev/null > > > +++ b/board/sifive/fu540/cache.c > > > > This should be put into arch/riscv/cpu/fu540/cache.c > > > > > @@ -0,0 +1,20 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright (c) 2019 SiFive, Inc > > > + */ > > > +#include <asm/io.h> > > > + > > > +/* Register offsets */ > > > +#define CACHE_ENABLE 0x008 > > > + > > > +/* Enable ways; allow cache to use these ways */ > > > +void cache_enable_ways(u64 base_addr, u8 value) > > > +{ > > > + volatile u32 *enable = (volatile u32 *)(base_addr + > > > + CACHE_ENABLE); > > > + /* memory barrier */ > > > + mb(); > > > + (*enable) = value; > > > + /* memory barrier */ > > > + mb(); > > > +} > > > diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h > > > new file mode 100644 > > > index 0000000000..425124a23b > > > --- /dev/null > > > +++ b/board/sifive/fu540/cache.h > > > > arch/riscv/include/asm/arch-fu540/cache.h > > Let's not entire FU540 directory under arch/riscv/cpu directory > just to have cache functions. The arch/riscv/cpu/generic is perfectly > suitable for FU540. I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot SPL phase. It can be generic for S-mode U-Boot though. > > If we re-use arch/riscv/cpu/generic as-much as possible then > arch/riscv will be easy to maintain in future. > > We can add arch/riscv/cpu/generic/cache.c which will do > things FU540 specific based on "#ifdef" or "DT compatible string". > Regards, Bin
On Fri, Mar 13, 2020 at 3:52 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Anup, > > On Fri, Mar 13, 2020 at 6:02 PM Anup Patel <anup at brainfault.org> wrote: > > > > On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel > > > <pragnesh.patel at sifive.com> wrote: > > > > > > > > Enable all cache ways from u-boot proper. > > > > > > U-Boot > > > > > > > > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > > > > --- > > > > board/sifive/fu540/Makefile | 1 + > > > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > > > > board/sifive/fu540/cache.h | 13 +++++++++++++ > > > > board/sifive/fu540/fu540.c | 6 ++++-- > > > > 4 files changed, 38 insertions(+), 2 deletions(-) > > > > create mode 100644 board/sifive/fu540/cache.c > > > > create mode 100644 board/sifive/fu540/cache.h > > > > > > > > diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile > > > > index b05e2f5807..3b867bbd89 100644 > > > > --- a/board/sifive/fu540/Makefile > > > > +++ b/board/sifive/fu540/Makefile > > > > @@ -3,6 +3,7 @@ > > > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > > > > > > obj-y += fu540.o > > > > +obj-y += cache.o > > > > > > > > ifdef CONFIG_SPL_BUILD > > > > obj-y += spl.o > > > > diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c > > > > new file mode 100644 > > > > index 0000000000..a0bcd2ba48 > > > > --- /dev/null > > > > +++ b/board/sifive/fu540/cache.c > > > > > > This should be put into arch/riscv/cpu/fu540/cache.c > > > > > > > @@ -0,0 +1,20 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * Copyright (c) 2019 SiFive, Inc > > > > + */ > > > > +#include <asm/io.h> > > > > + > > > > +/* Register offsets */ > > > > +#define CACHE_ENABLE 0x008 > > > > + > > > > +/* Enable ways; allow cache to use these ways */ > > > > +void cache_enable_ways(u64 base_addr, u8 value) > > > > +{ > > > > + volatile u32 *enable = (volatile u32 *)(base_addr + > > > > + CACHE_ENABLE); > > > > + /* memory barrier */ > > > > + mb(); > > > > + (*enable) = value; > > > > + /* memory barrier */ > > > > + mb(); > > > > +} > > > > diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h > > > > new file mode 100644 > > > > index 0000000000..425124a23b > > > > --- /dev/null > > > > +++ b/board/sifive/fu540/cache.h > > > > > > arch/riscv/include/asm/arch-fu540/cache.h > > > > Let's not entire FU540 directory under arch/riscv/cpu directory > > just to have cache functions. The arch/riscv/cpu/generic is perfectly > > suitable for FU540. > > I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot > SPL phase. It can be generic for S-mode U-Boot though. Its really very easy to do. We are already doing this in Xvisor. As an example, of DT based operations in a generic board support refer: https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/generic/brd_main.c https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/generic/foundation-v8.c Using the above approach, we are able to boot same Xvisor ARM/ARM64 binary on multiple boards. > > > > > If we re-use arch/riscv/cpu/generic as-much as possible then > > arch/riscv will be easy to maintain in future. > > > > We can add arch/riscv/cpu/generic/cache.c which will do > > things FU540 specific based on "#ifdef" or "DT compatible string". > > > > Regards, > Bin Regards, Anup
On Fri, Mar 13, 2020 at 3:52 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Anup, > > On Fri, Mar 13, 2020 at 6:02 PM Anup Patel <anup at brainfault.org> wrote: > > > > On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel > > > <pragnesh.patel at sifive.com> wrote: > > > > > > > > Enable all cache ways from u-boot proper. > > > > > > U-Boot > > > > > > > > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > > > > --- > > > > board/sifive/fu540/Makefile | 1 + > > > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > > > > board/sifive/fu540/cache.h | 13 +++++++++++++ > > > > board/sifive/fu540/fu540.c | 6 ++++-- > > > > 4 files changed, 38 insertions(+), 2 deletions(-) > > > > create mode 100644 board/sifive/fu540/cache.c > > > > create mode 100644 board/sifive/fu540/cache.h > > > > > > > > diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile > > > > index b05e2f5807..3b867bbd89 100644 > > > > --- a/board/sifive/fu540/Makefile > > > > +++ b/board/sifive/fu540/Makefile > > > > @@ -3,6 +3,7 @@ > > > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > > > > > > obj-y += fu540.o > > > > +obj-y += cache.o > > > > > > > > ifdef CONFIG_SPL_BUILD > > > > obj-y += spl.o > > > > diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c > > > > new file mode 100644 > > > > index 0000000000..a0bcd2ba48 > > > > --- /dev/null > > > > +++ b/board/sifive/fu540/cache.c > > > > > > This should be put into arch/riscv/cpu/fu540/cache.c > > > > > > > @@ -0,0 +1,20 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * Copyright (c) 2019 SiFive, Inc > > > > + */ > > > > +#include <asm/io.h> > > > > + > > > > +/* Register offsets */ > > > > +#define CACHE_ENABLE 0x008 > > > > + > > > > +/* Enable ways; allow cache to use these ways */ > > > > +void cache_enable_ways(u64 base_addr, u8 value) > > > > +{ > > > > + volatile u32 *enable = (volatile u32 *)(base_addr + > > > > + CACHE_ENABLE); > > > > + /* memory barrier */ > > > > + mb(); > > > > + (*enable) = value; > > > > + /* memory barrier */ > > > > + mb(); > > > > +} > > > > diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h > > > > new file mode 100644 > > > > index 0000000000..425124a23b > > > > --- /dev/null > > > > +++ b/board/sifive/fu540/cache.h > > > > > > arch/riscv/include/asm/arch-fu540/cache.h > > > > Let's not entire FU540 directory under arch/riscv/cpu directory > > just to have cache functions. The arch/riscv/cpu/generic is perfectly > > suitable for FU540. > > I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot > SPL phase. It can be generic for S-mode U-Boot though. We can also have a light weight U-Boot driver framework for cache operations. This framework will figure-out cache operation based on SOC compatible string in root DT node or some other way. This will be useful in long-term when we have complex cache hierarchy on RISC-V SOCs. > > > > > If we re-use arch/riscv/cpu/generic as-much as possible then > > arch/riscv will be easy to maintain in future. > > > > We can add arch/riscv/cpu/generic/cache.c which will do > > things FU540 specific based on "#ifdef" or "DT compatible string". > > > > Regards, > Bin Regards, Anup
Hi Anup, On Fri, Mar 13, 2020 at 6:49 PM Anup Patel <anup at brainfault.org> wrote: > > On Fri, Mar 13, 2020 at 3:52 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > Hi Anup, > > > > On Fri, Mar 13, 2020 at 6:02 PM Anup Patel <anup at brainfault.org> wrote: > > > > > > On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel > > > > <pragnesh.patel at sifive.com> wrote: > > > > > > > > > > Enable all cache ways from u-boot proper. > > > > > > > > U-Boot > > > > > > > > > > > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > > > > > --- > > > > > board/sifive/fu540/Makefile | 1 + > > > > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > > > > > board/sifive/fu540/cache.h | 13 +++++++++++++ > > > > > board/sifive/fu540/fu540.c | 6 ++++-- > > > > > 4 files changed, 38 insertions(+), 2 deletions(-) > > > > > create mode 100644 board/sifive/fu540/cache.c > > > > > create mode 100644 board/sifive/fu540/cache.h > > > > > > > > > > diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile > > > > > index b05e2f5807..3b867bbd89 100644 > > > > > --- a/board/sifive/fu540/Makefile > > > > > +++ b/board/sifive/fu540/Makefile > > > > > @@ -3,6 +3,7 @@ > > > > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > > > > > > > > obj-y += fu540.o > > > > > +obj-y += cache.o > > > > > > > > > > ifdef CONFIG_SPL_BUILD > > > > > obj-y += spl.o > > > > > diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c > > > > > new file mode 100644 > > > > > index 0000000000..a0bcd2ba48 > > > > > --- /dev/null > > > > > +++ b/board/sifive/fu540/cache.c > > > > > > > > This should be put into arch/riscv/cpu/fu540/cache.c > > > > > > > > > @@ -0,0 +1,20 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > +/* > > > > > + * Copyright (c) 2019 SiFive, Inc > > > > > + */ > > > > > +#include <asm/io.h> > > > > > + > > > > > +/* Register offsets */ > > > > > +#define CACHE_ENABLE 0x008 > > > > > + > > > > > +/* Enable ways; allow cache to use these ways */ > > > > > +void cache_enable_ways(u64 base_addr, u8 value) > > > > > +{ > > > > > + volatile u32 *enable = (volatile u32 *)(base_addr + > > > > > + CACHE_ENABLE); > > > > > + /* memory barrier */ > > > > > + mb(); > > > > > + (*enable) = value; > > > > > + /* memory barrier */ > > > > > + mb(); > > > > > +} > > > > > diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h > > > > > new file mode 100644 > > > > > index 0000000000..425124a23b > > > > > --- /dev/null > > > > > +++ b/board/sifive/fu540/cache.h > > > > > > > > arch/riscv/include/asm/arch-fu540/cache.h > > > > > > Let's not entire FU540 directory under arch/riscv/cpu directory > > > just to have cache functions. The arch/riscv/cpu/generic is perfectly > > > suitable for FU540. > > > > I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot > > SPL phase. It can be generic for S-mode U-Boot though. > > Its really very easy to do. We are already doing this in Xvisor. > > As an example, of DT based operations in a generic board support refer: > https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/generic/brd_main.c > https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/generic/foundation-v8.c Yes, this can be easy to do if we have everything written based on DT. But we cannot always assume DT is available in SPL. Some SoCs will not allow SPL with DT support to run due to constraint resources. Even for DT, due to SPL initialization codes can be very low-level and specific to an SoC, not everything can be properly modeled by DT. Take a look at the u-boot/arch/arm directory. Things are not that easy. > > Using the above approach, we are able to boot same Xvisor ARM/ARM64 > binary on multiple boards. > Yes, I know. The same as what is done in the Linux kernel. Take x86 for example, the same kernel image can boot on almost every x86 desktop/laptop/server we have today. But we have to understand that this is built on top of BIOS which does all low-level processor / chipset initialization and hide that very well for OS. > > > > > > > > If we re-use arch/riscv/cpu/generic as-much as possible then > > > arch/riscv will be easy to maintain in future. > > > > > > We can add arch/riscv/cpu/generic/cache.c which will do > > > things FU540 specific based on "#ifdef" or "DT compatible string". > > > Regards, Bin
Hi, >-----Original Message----- >From: Bin Meng <bmeng.cn at gmail.com> >Sent: 13 March 2020 19:19 >To: Anup Patel <anup at brainfault.org> >Cc: Pragnesh Patel <pragnesh.patel at sifive.com>; U-Boot Mailing List <u- >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer Dabbelt ><palmerdabbelt at google.com>; Paul Walmsley <paul.walmsley at sifive.com>; >Jagan Teki <jagan at amarulasolutions.com>; Troy Benjegerdes ><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar >Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Palmer >Dabbelt <palmer at dabbelt.com> >Subject: Re: [PATCH v5 12/14] riscv: sifive: fu540: enable all cache ways from >u-boot proper > >Hi Anup, > >On Fri, Mar 13, 2020 at 6:49 PM Anup Patel <anup at brainfault.org> wrote: >> >> On Fri, Mar 13, 2020 at 3:52 PM Bin Meng <bmeng.cn at gmail.com> wrote: >> > >> > Hi Anup, >> > >> > On Fri, Mar 13, 2020 at 6:02 PM Anup Patel <anup at brainfault.org> wrote: >> > > >> > > On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> >wrote: >> > > > >> > > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel >> > > > <pragnesh.patel at sifive.com> wrote: >> > > > > >> > > > > Enable all cache ways from u-boot proper. >> > > > >> > > > U-Boot >> > > > >> > > > > >> > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> >> > > > > --- >> > > > > board/sifive/fu540/Makefile | 1 + >> > > > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ >> > > > > board/sifive/fu540/cache.h | 13 +++++++++++++ >> > > > > board/sifive/fu540/fu540.c | 6 ++++-- >> > > > > 4 files changed, 38 insertions(+), 2 deletions(-) create >> > > > > mode 100644 board/sifive/fu540/cache.c create mode 100644 >> > > > > board/sifive/fu540/cache.h >> > > > > >> > > > > diff --git a/board/sifive/fu540/Makefile >> > > > > b/board/sifive/fu540/Makefile index b05e2f5807..3b867bbd89 >> > > > > 100644 >> > > > > --- a/board/sifive/fu540/Makefile >> > > > > +++ b/board/sifive/fu540/Makefile >> > > > > @@ -3,6 +3,7 @@ >> > > > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. >> > > > > >> > > > > obj-y += fu540.o >> > > > > +obj-y += cache.o >> > > > > >> > > > > ifdef CONFIG_SPL_BUILD >> > > > > obj-y += spl.o >> > > > > diff --git a/board/sifive/fu540/cache.c >> > > > > b/board/sifive/fu540/cache.c new file mode 100644 index >> > > > > 0000000000..a0bcd2ba48 >> > > > > --- /dev/null >> > > > > +++ b/board/sifive/fu540/cache.c >> > > > >> > > > This should be put into arch/riscv/cpu/fu540/cache.c >> > > > >> > > > > @@ -0,0 +1,20 @@ >> > > > > +// SPDX-License-Identifier: GPL-2.0+ >> > > > > +/* >> > > > > + * Copyright (c) 2019 SiFive, Inc */ #include <asm/io.h> >> > > > > + >> > > > > +/* Register offsets */ >> > > > > +#define CACHE_ENABLE 0x008 >> > > > > + >> > > > > +/* Enable ways; allow cache to use these ways */ void >> > > > > +cache_enable_ways(u64 base_addr, u8 value) { >> > > > > + volatile u32 *enable = (volatile u32 *)(base_addr + >> > > > > + CACHE_ENABLE); >> > > > > + /* memory barrier */ >> > > > > + mb(); >> > > > > + (*enable) = value; >> > > > > + /* memory barrier */ >> > > > > + mb(); >> > > > > +} >> > > > > diff --git a/board/sifive/fu540/cache.h >> > > > > b/board/sifive/fu540/cache.h new file mode 100644 index >> > > > > 0000000000..425124a23b >> > > > > --- /dev/null >> > > > > +++ b/board/sifive/fu540/cache.h >> > > > >> > > > arch/riscv/include/asm/arch-fu540/cache.h >> > > >> > > Let's not entire FU540 directory under arch/riscv/cpu directory >> > > just to have cache functions. The arch/riscv/cpu/generic is >> > > perfectly suitable for FU540. >> > >> > I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot >> > SPL phase. It can be generic for S-mode U-Boot though. >> >> Its really very easy to do. We are already doing this in Xvisor. >> >> As an example, of DT based operations in a generic board support refer: >> https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/gene >> ric/brd_main.c >> https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/gene >> ric/foundation-v8.c > >Yes, this can be easy to do if we have everything written based on DT. >But we cannot always assume DT is available in SPL. Some SoCs will not allow >SPL with DT support to run due to constraint resources. > >Even for DT, due to SPL initialization codes can be very low-level and specific >to an SoC, not everything can be properly modeled by DT. Take a look at the >u-boot/arch/arm directory. Things are not that easy. > >> >> Using the above approach, we are able to boot same Xvisor ARM/ARM64 >> binary on multiple boards. >> > >Yes, I know. The same as what is done in the Linux kernel. Take x86 for >example, the same kernel image can boot on almost every x86 >desktop/laptop/server we have today. But we have to understand that this is >built on top of BIOS which does all low-level processor / chipset initialization >and hide that very well for OS. > IMHO just for cache, it's better not to add arch/riscv/cpu/fu540. If something that we can not handle with arch/riscv/cpu/generic then we will definitely add arch/riscv/cpu/fu540 dir. Thanks Bin and Anup for the review. >> > >> > > >> > > If we re-use arch/riscv/cpu/generic as-much as possible then >> > > arch/riscv will be easy to maintain in future. >> > > >> > > We can add arch/riscv/cpu/generic/cache.c which will do things >> > > FU540 specific based on "#ifdef" or "DT compatible string". >> > > > >Regards, >Bin
Hi Pragnesh > From: Pragnesh Patel [mailto:pragnesh.patel at sifive.com] > Sent: Tuesday, March 17, 2020 5:52 PM > To: Bin Meng; Anup Patel > Cc: U-Boot Mailing List; Atish Patra; Palmer Dabbelt; Paul Walmsley; Jagan Teki; Troy Benjegerdes; Anup Patel; Sagar Kadam; Rick Jian-Zhi Chen(???); Palmer Dabbelt > Subject: RE: [PATCH v5 12/14] riscv: sifive: fu540: enable all cache ways from u-boot proper > > > Hi, > > >-----Original Message----- > >From: Bin Meng <bmeng.cn at gmail.com> > >Sent: 13 March 2020 19:19 > >To: Anup Patel <anup at brainfault.org> > >Cc: Pragnesh Patel <pragnesh.patel at sifive.com>; U-Boot Mailing List <u- > >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer Dabbelt > ><palmerdabbelt at google.com>; Paul Walmsley <paul.walmsley at sifive.com>; > >Jagan Teki <jagan at amarulasolutions.com>; Troy Benjegerdes > ><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar > >Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Palmer > >Dabbelt <palmer at dabbelt.com> > >Subject: Re: [PATCH v5 12/14] riscv: sifive: fu540: enable all cache > >ways from u-boot proper > > > >Hi Anup, > > > >On Fri, Mar 13, 2020 at 6:49 PM Anup Patel <anup at brainfault.org> wrote: > >> > >> On Fri, Mar 13, 2020 at 3:52 PM Bin Meng <bmeng.cn at gmail.com> wrote: > >> > > >> > Hi Anup, > >> > > >> > On Fri, Mar 13, 2020 at 6:02 PM Anup Patel <anup at brainfault.org> wrote: > >> > > > >> > > On Fri, Mar 13, 2020 at 2:31 PM Bin Meng <bmeng.cn at gmail.com> > >wrote: > >> > > > > >> > > > On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel > >> > > > <pragnesh.patel at sifive.com> wrote: > >> > > > > > >> > > > > Enable all cache ways from u-boot proper. > >> > > > > >> > > > U-Boot > >> > > > > >> > > > > > >> > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> > >> > > > > --- > >> > > > > board/sifive/fu540/Makefile | 1 + > >> > > > > board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ > >> > > > > board/sifive/fu540/cache.h | 13 +++++++++++++ > >> > > > > board/sifive/fu540/fu540.c | 6 ++++-- > >> > > > > 4 files changed, 38 insertions(+), 2 deletions(-) create > >> > > > > mode 100644 board/sifive/fu540/cache.c create mode 100644 > >> > > > > board/sifive/fu540/cache.h > >> > > > > > >> > > > > diff --git a/board/sifive/fu540/Makefile > >> > > > > b/board/sifive/fu540/Makefile index b05e2f5807..3b867bbd89 > >> > > > > 100644 > >> > > > > --- a/board/sifive/fu540/Makefile > >> > > > > +++ b/board/sifive/fu540/Makefile > >> > > > > @@ -3,6 +3,7 @@ > >> > > > > # Copyright (c) 2019 Western Digital Corporation or its affiliates. > >> > > > > > >> > > > > obj-y += fu540.o > >> > > > > +obj-y += cache.o > >> > > > > > >> > > > > ifdef CONFIG_SPL_BUILD > >> > > > > obj-y += spl.o > >> > > > > diff --git a/board/sifive/fu540/cache.c > >> > > > > b/board/sifive/fu540/cache.c new file mode 100644 index > >> > > > > 0000000000..a0bcd2ba48 > >> > > > > --- /dev/null > >> > > > > +++ b/board/sifive/fu540/cache.c > >> > > > > >> > > > This should be put into arch/riscv/cpu/fu540/cache.c > >> > > > > >> > > > > @@ -0,0 +1,20 @@ > >> > > > > +// SPDX-License-Identifier: GPL-2.0+ > >> > > > > +/* > >> > > > > + * Copyright (c) 2019 SiFive, Inc */ #include <asm/io.h> > >> > > > > + > >> > > > > +/* Register offsets */ > >> > > > > +#define CACHE_ENABLE 0x008 > >> > > > > + > >> > > > > +/* Enable ways; allow cache to use these ways */ void > >> > > > > +cache_enable_ways(u64 base_addr, u8 value) { > >> > > > > + volatile u32 *enable = (volatile u32 *)(base_addr + > >> > > > > + CACHE_ENABLE); > >> > > > > + /* memory barrier */ > >> > > > > + mb(); > >> > > > > + (*enable) = value; > >> > > > > + /* memory barrier */ > >> > > > > + mb(); > >> > > > > +} > >> > > > > diff --git a/board/sifive/fu540/cache.h > >> > > > > b/board/sifive/fu540/cache.h new file mode 100644 index > >> > > > > 0000000000..425124a23b > >> > > > > --- /dev/null > >> > > > > +++ b/board/sifive/fu540/cache.h > >> > > > > >> > > > arch/riscv/include/asm/arch-fu540/cache.h > >> > > > >> > > Let's not entire FU540 directory under arch/riscv/cpu directory > >> > > just to have cache functions. The arch/riscv/cpu/generic is > >> > > perfectly suitable for FU540. > >> > > >> > I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot > >> > SPL phase. It can be generic for S-mode U-Boot though. > >> > >> Its really very easy to do. We are already doing this in Xvisor. > >> > >> As an example, of DT based operations in a generic board support refer: > >> https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/gen > >> e > >> ric/brd_main.c > >> https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/gen > >> e > >> ric/foundation-v8.c > > > >Yes, this can be easy to do if we have everything written based on DT. > >But we cannot always assume DT is available in SPL. Some SoCs will not > >allow SPL with DT support to run due to constraint resources. > > > >Even for DT, due to SPL initialization codes can be very low-level and > >specific to an SoC, not everything can be properly modeled by DT. Take > >a look at the u-boot/arch/arm directory. Things are not that easy. > > > >> > >> Using the above approach, we are able to boot same Xvisor ARM/ARM64 > >> binary on multiple boards. > >> > > > >Yes, I know. The same as what is done in the Linux kernel. Take x86 for > >example, the same kernel image can boot on almost every x86 > >desktop/laptop/server we have today. But we have to understand that > >this is built on top of BIOS which does all low-level processor / > >chipset initialization and hide that very well for OS. > > > > IMHO just for cache, it's better not to add arch/riscv/cpu/fu540. > If something that we can not handle with arch/riscv/cpu/generic then we will definitely add arch/riscv/cpu/fu540 dir. > > Thanks Bin and Anup for the review. You shall consider to put it in drivers/cache dir And parse the cache register base from DT instead of hard code (#define CACHE_CTRL_ADDR _AC(0x2010000, UL)) Also parse number of ways instead of hard code (15) // cache_enable_ways(CACHE_CTRL_ADDR, 15); It is a legacy way to define register base with hard code, better not doing that way. Thanks Rick > > >> > > >> > > > >> > > If we re-use arch/riscv/cpu/generic as-much as possible then > >> > > arch/riscv will be easy to maintain in future. > >> > > > >> > > We can add arch/riscv/cpu/generic/cache.c which will do things > >> > > FU540 specific based on "#ifdef" or "DT compatible string". > >> > > > > > >Regards, > >Bin
diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile index b05e2f5807..3b867bbd89 100644 --- a/board/sifive/fu540/Makefile +++ b/board/sifive/fu540/Makefile @@ -3,6 +3,7 @@ # Copyright (c) 2019 Western Digital Corporation or its affiliates. obj-y += fu540.o +obj-y += cache.o ifdef CONFIG_SPL_BUILD obj-y += spl.o diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c new file mode 100644 index 0000000000..a0bcd2ba48 --- /dev/null +++ b/board/sifive/fu540/cache.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2019 SiFive, Inc + */ +#include <asm/io.h> + +/* Register offsets */ +#define CACHE_ENABLE 0x008 + +/* Enable ways; allow cache to use these ways */ +void cache_enable_ways(u64 base_addr, u8 value) +{ + volatile u32 *enable = (volatile u32 *)(base_addr + + CACHE_ENABLE); + /* memory barrier */ + mb(); + (*enable) = value; + /* memory barrier */ + mb(); +} diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h new file mode 100644 index 0000000000..425124a23b --- /dev/null +++ b/board/sifive/fu540/cache.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2019 SiFive, Inc + */ + +#ifndef FU540_CACHE_H +#define FU540_CACHE_H + +#define CACHE_CTRL_ADDR _AC(0x2010000, UL) + +void cache_enable_ways(u64 base_addr, u8 value); + +#endif /* FU540_CACHE_H */ diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c index 89a65eb3fb..1d6c0c9bba 100644 --- a/board/sifive/fu540/fu540.c +++ b/board/sifive/fu540/fu540.c @@ -13,6 +13,8 @@ #include <misc.h> #include <spl.h> +#include "cache.h" + /* * This define is a value used for error/unknown serial. * If we really care about distinguishing errors and 0 is @@ -111,8 +113,8 @@ int misc_init_r(void) int board_init(void) { - /* For now nothing to do here. */ - + /* enable all cache ways */ + cache_enable_ways(CACHE_CTRL_ADDR, 15); return 0; }
Enable all cache ways from u-boot proper. Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> --- board/sifive/fu540/Makefile | 1 + board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ board/sifive/fu540/cache.h | 13 +++++++++++++ board/sifive/fu540/fu540.c | 6 ++++-- 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 board/sifive/fu540/cache.c create mode 100644 board/sifive/fu540/cache.h