Hi Lukas,
On Fri, Nov 16, 2018 at 7:10 AM Auer, Lukas
<lukas.a...@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> > Implement arch_cpu_init() to do some basic architecture level cpu
> > initialization, like FPU enable, etc.
> >
> > Signed-off-by: Bin Meng <bmeng...@gmail.com>
> > ---
> >
> > arch/riscv/cpu/cpu.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > index d9f820c..4e508cf 100644
> > --- a/arch/riscv/cpu/cpu.c
> > +++ b/arch/riscv/cpu/cpu.c
> > @@ -5,6 +5,7 @@
> >
> > #include <common.h>
> > #include <asm/csr.h>
> > +#include <asm/encoding.h>
> >
> > /*
> > * prior_stage_fdt_address must be stored in the data section since
> > it is used
> > @@ -53,3 +54,23 @@ int print_cpuinfo(void)
> >
> > return 0;
> > }
> > +
> > +int arch_cpu_init(void)
> > +{
> > + /* Enable FPU */
> > + if (supports_extension('d') || supports_extension('f')) {
> > + csr_write(mstatus, MSTATUS_FS);
>
> This should use csr_set(), so that we don't overwrite the other mstatus
> entries.
>
Ah, yes!
> > + csr_write(fcsr, 0);
>
> BBL also clears the floating point registers before clearing fcsr.
> Coreboot does neither of these, I am not sure if they are required.
>
It's not required. I believe this is just for sanitizing these registers.
> > + }
> > +
> > + /* Enable user/supervisor use of perf counters */
> > + if (supports_extension('s'))
> > + csr_write(scounteren, -1);
> > + csr_write(mcounteren, -1);
>
> Should we really enable all counters, and for both user and supervisor
> code? I would tend to enable only the ones we need. How about only
> enabling the cycle, time, and instret counters (the rest are not
> currently used in the kernel) and only for supervisor code (so in
> mcounteren)?
>
Yes, not all of them are used by current kernel, but they might be
used in the future. To enable all of them is not a bad idea, unless
turning it on brings some consequences.
> > +
> > + /* Disable paging */
> > + if (supports_extension('s'))
> > + csr_write(sptbr, 0);
>
> sptbr has been renamed to satp.
>
Good catch! Will fix in v2.
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
