[U-Boot] [RESEND PATCH v2 08/15] riscv: Add a helper routine to print CPU information

[Auer, Lukas]

Hi Bin,

On Mon, 2018-09-17 at 12:55 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Mon, Sep 17, 2018 at 4:54 AM Auer, Lukas
> <lukas.auer at aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Mon, 2018-09-10 at 21:54 -0700, Bin Meng wrote:
> > > This adds a helper routine to print CPU information. Currently
> > > it prints all the instruction set extensions that the processor
> > > core supports.
> > > 
> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > ---
> > > 
> > > Changes in v2: None
> > > 
> > >  arch/riscv/Makefile          |   1 +
> > >  arch/riscv/cpu/Makefile      |   5 ++
> > >  arch/riscv/cpu/cpu.c         |  49 +++++++++++++++++
> > >  arch/riscv/include/asm/csr.h | 124
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 179 insertions(+)
> > >  create mode 100644 arch/riscv/cpu/Makefile
> > >  create mode 100644 arch/riscv/cpu/cpu.c
> > >  create mode 100644 arch/riscv/include/asm/csr.h
> > > 
> > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > index 084888a..af432e1 100644
> > > --- a/arch/riscv/Makefile
> > > +++ b/arch/riscv/Makefile
> > > @@ -5,5 +5,6 @@
> > > 
> > >  head-y := arch/riscv/cpu/$(CPU)/start.o
> > > 
> > > +libs-y += arch/riscv/cpu/
> > >  libs-y += arch/riscv/cpu/$(CPU)/
> > >  libs-y += arch/riscv/lib/
> > > diff --git a/arch/riscv/cpu/Makefile b/arch/riscv/cpu/Makefile
> > > new file mode 100644
> > > index 0000000..63de163
> > > --- /dev/null
> > > +++ b/arch/riscv/cpu/Makefile
> > > @@ -0,0 +1,5 @@
> > > +# SPDX-License-Identifier: GPL-2.0+
> > > +#
> > > +# Copyright (C) 2018, Bin Meng <bmeng.cn at gmail.com>
> > > +
> > > +obj-y += cpu.o
> > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > > new file mode 100644
> > > index 0000000..ae57fb8
> > > --- /dev/null
> > > +++ b/arch/riscv/cpu/cpu.c
> > > @@ -0,0 +1,49 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2018, Bin Meng <bmeng.cn at gmail.com>
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <asm/csr.h>
> > > +
> > > +enum {
> > > +     ISA_INVALID = 0,
> > > +     ISA_32BIT,
> > > +     ISA_64BIT,
> > > +     ISA_128BIT
> > > +};
> > > +
> > > +static const char * const isa_bits[] = {
> > > +     [ISA_INVALID] = NULL,
> > > +     [ISA_32BIT]   = "32",
> > > +     [ISA_64BIT]   = "64",
> > > +     [ISA_128BIT]  = "128"
> > > +};
> > > +
> > > +static inline bool supports_extension(char ext)
> > > +{
> > > +     return csr_read(misa) & (1 << (ext - 'a'));
> > > +}
> > > +
> > > +int print_cpuinfo(void)
> > > +{
> > > +     char name[32];
> > > +     char *s = name;
> > > +     int bit;
> > > +
> > > +     s += sprintf(name, "rv");
> > > +     bit = csr_read(misa) >> (sizeof(long) * 8 - 2);
> > > +     s += sprintf(s, isa_bits[bit]);
> > > +
> > > +     supports_extension('i') ? *s++ = 'i' : 'r';
> > > +     supports_extension('m') ? *s++ = 'm' : 'i';
> > > +     supports_extension('a') ? *s++ = 'a' : 's';
> > > +     supports_extension('f') ? *s++ = 'f' : 'c';
> > > +     supports_extension('d') ? *s++ = 'd' : '-';
> > > +     supports_extension('c') ? *s++ = 'c' : 'v';
> > > +     *s++ = '\0';
> > 
> > Why are you not using the ISA string "riscv,isa" from the device
> > tree?
> > This way, the code is not limited to machine mode and we do not
> > have
> > update it if the set of extensions to check for changes.
> 
> I wanted to use hardware provided information whenever possible.
> Reading from DT can be a last resort. I think we wanted to have U-
> Boot
> running in machine mode, no?
> 
> Regards,
> Bin
> 

Ok, I see. I don't entirely like that this has to be updated if we want
to display new extensions and it's currently actually missing the 'u'
(user) and 's' (supervisor) extensions, which are present in the misa
of RISC-V QEMU. It is fine for now, I think.

Yes, we want to run u-boot in machine mode for now, especially if we
want to add an SBI implementation to u-boot. Not much is needed to run
it in supervisor mode though, which is why I mentioned this. It also
allows us to use an SBI implementation external to u-boot, should we
want / need that.

Thanks,
Lukas


Reviewed-by: Lukas Auer <lukas.auer at aisec.fraunhofer.de>