[U-Boot] [PATCH 06/30] riscv: add Kconfig entries for the C and A ISA extensions

[Bin Meng]

Hi Lukas,

On Wed, Oct 24, 2018 at 11:21 PM Auer, Lukas
<lukas.auer at 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 at 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 at 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