[PATCH v5 12/14] riscv: sifive: fu540: enable all cache ways from u-boot proper

[Bin Meng]

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