Hi Bin,
On Fri, 2018-11-30 at 17:48 +0800, Bin Meng wrote:
Hi Lukas,
On Fri, Nov 16, 2018 at 7:10 AM Auer, Lukas lukas.auer@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.cn@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.
Ok, makes sense.
}/* 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.
Hardware performance counters can be used as part of side-channel attacks. It is therefore a good idea to disable them by default. Here, I would restrict them to accesses from the supervisor mode. If needed, software running in supervisor mode can still enable access for the user mode.
Thanks, Lukas
/* Disable paging */if (supports_extension('s'))csr_write(sptbr, 0);sptbr has been renamed to satp.
Good catch! Will fix in v2.
Regards, Bin