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

[Bin Meng]

Hi Lukas,

On Tue, Sep 18, 2018 at 5:59 AM Auer, Lukas
<lukas.auer at aisec.fraunhofer.de> wrote:
>
> 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, let's leave this for now. Currently it seems only prints "GC"
extension is enough (IMAFDC).

> 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.
>

Let's see how things go. My personal preference would be only using
U-Boot though :)

Regards,
Bin