Re: [U-Boot] [PATCH v2 16/20] riscv: Do some basic architecture level cpu initialization

[Bin Meng]

Hi Lukas,

On Tue, Dec 11, 2018 at 8:01 AM Auer, Lukas
 wrote:
>
> Hi Bin,
>
> On Fri, 2018-12-07 at 06:14 -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 
> >
> > ---
> >
> > Changes in v2:
> > - use csr_set() to set MSTATUS_FS
> > - only enabling the cycle, time, and instret counters
> > - change to use satp
> >
> >  arch/riscv/cpu/cpu.c | 19 +++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > index 3858e51..194799c 100644
> > --- a/arch/riscv/cpu/cpu.c
> > +++ b/arch/riscv/cpu/cpu.c
> > @@ -7,6 +7,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  /*
> >   * prior_stage_fdt_address must be stored in the data section since
> > it is used
> > @@ -67,3 +68,21 @@ int arch_early_init_r(void)
> >
> >   return 0;
> >  }
> > +
> > +int arch_cpu_init(void)
> > +{
> > + /* Enable FPU */
> > + if (supports_extension('d') || supports_extension('f')) {
>
> supports_extension does not work when running in supervisor mode
> (unless BBL is patched). Can we maybe use the CPU driver here?
>

Yes, I think so. Will change to use info provided by the CPU driver in v3.

> > + csr_set(MODE_PREFIX(status), MSTATUS_FS);
> > + csr_write(fcsr, 0);
> > + }
> > +
> > + /* Enable perf counters for cycle, time, and instret counters
> > only */
> > + csr_write(MODE_PREFIX(counteren), GENMASK(2, 0));
>
> I would tend to only enable this in mcounteren, so for the supervisor-
> mode. Linux can still enable the counters for user-mode if required.
>

OK.

> > +
> > + /* Disable paging */
> > + if (supports_extension('s'))
> > + csr_write(satp, 0);
>
> This should only be done, when running in machine mode. In supervisor
> mode this would cause issues if we have something other than an
> identity-mapping or paging disabled.
>

I doubt we want to enable paging for S-mode U-Boot, but I am OK to do
such in M-mode only.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 16/20] riscv: Do some basic architecture level cpu initialization

[Auer, Lukas]

Hi Bin,

On Fri, 2018-12-07 at 06:14 -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 
> 
> ---
> 
> Changes in v2:
> - use csr_set() to set MSTATUS_FS
> - only enabling the cycle, time, and instret counters
> - change to use satp
> 
>  arch/riscv/cpu/cpu.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> index 3858e51..194799c 100644
> --- a/arch/riscv/cpu/cpu.c
> +++ b/arch/riscv/cpu/cpu.c
> @@ -7,6 +7,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * prior_stage_fdt_address must be stored in the data section since
> it is used
> @@ -67,3 +68,21 @@ int arch_early_init_r(void)
>  
>   return 0;
>  }
> +
> +int arch_cpu_init(void)
> +{
> + /* Enable FPU */
> + if (supports_extension('d') || supports_extension('f')) {

supports_extension does not work when running in supervisor mode
(unless BBL is patched). Can we maybe use the CPU driver here?

> + csr_set(MODE_PREFIX(status), MSTATUS_FS);
> + csr_write(fcsr, 0);
> + }
> +
> + /* Enable perf counters for cycle, time, and instret counters
> only */
> + csr_write(MODE_PREFIX(counteren), GENMASK(2, 0));

I would tend to only enable this in mcounteren, so for the supervisor-
mode. Linux can still enable the counters for user-mode if required.

> +
> + /* Disable paging */
> + if (supports_extension('s'))
> + csr_write(satp, 0);

This should only be done, when running in machine mode. In supervisor
mode this would cause issues if we have something other than an
identity-mapping or paging disabled.

Thanks,
Lukas

> +
> + return 0;
> +}
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 16/20] riscv: Do some basic architecture level cpu initialization

[Bin Meng]

Implement arch_cpu_init() to do some basic architecture level cpu
initialization, like FPU enable, etc.

Signed-off-by: Bin Meng 

---

Changes in v2:
- use csr_set() to set MSTATUS_FS
- only enabling the cycle, time, and instret counters
- change to use satp

 arch/riscv/cpu/cpu.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index 3858e51..194799c 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * prior_stage_fdt_address must be stored in the data section since it is used
@@ -67,3 +68,21 @@ int arch_early_init_r(void)
 
return 0;
 }
+
+int arch_cpu_init(void)
+{
+   /* Enable FPU */
+   if (supports_extension('d') || supports_extension('f')) {
+   csr_set(MODE_PREFIX(status), MSTATUS_FS);
+   csr_write(fcsr, 0);
+   }
+
+   /* Enable perf counters for cycle, time, and instret counters only */
+   csr_write(MODE_PREFIX(counteren), GENMASK(2, 0));
+
+   /* Disable paging */
+   if (supports_extension('s'))
+   csr_write(satp, 0);
+
+   return 0;
+}
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot