diff mbox series

[v2,05/11] riscv: Add option to disable writes to mcounteren

Message ID 2eef694b-a166-3d62-bfab-a39e87cecf0a@gmail.com
State New
Headers show
Series riscv: Add Sipeed Maix support | expand

Commit Message

Sean Anderson Jan. 15, 2020, 10:53 p.m. UTC
On the kendryte k210, writes to mcounteren result in an illegal instruction
exception.

Signed-off-by: Sean Anderson <seanga2 at gmail.com>
---
Changes for v2:
 Moved forward in the patch series

 arch/riscv/Kconfig   | 3 +++
 arch/riscv/cpu/cpu.c | 2 ++
 2 files changed, 5 insertions(+)

Comments

Lukas Auer Jan. 26, 2020, 10:09 p.m. UTC | #1
+ Bin, Anup, Atish


On Wed, 2020-01-15 at 17:53 -0500, Sean Anderson wrote:
> On the kendryte k210, writes to mcounteren result in an illegal instruction
> exception.
> 
> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> ---
> Changes for v2:
>  Moved forward in the patch series
> 
>  arch/riscv/Kconfig   | 3 +++
>  arch/riscv/cpu/cpu.c | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 9a7b0334c2..4f8c62dcff 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -226,6 +226,9 @@ config XIP
>  	  from a NOR flash memory without copying the code to ram.
>  	  Say yes here if U-Boot boots from flash directly.
>  
> +config SYS_RISCV_NOCOUNTER
> +	bool "Disable accesses to the mcounteren CSR"
> +

Can you rename this to something like RISCV_PRIV_1_9_1?

The k210 implements version 1.9.1 of the privileged spec (if I remember
correctly). The mcounteren CSR doesn't exist in that version and
therefore triggers the illegal instruction exception. By renaming the
config entry, it is clearer why the CSR is missing and is therefore not
accessed.
I am not too familiar with the changes between the versions of the
spec. Are there other parts of the code we need to adapt?

Thanks,
Lukas

>  config STACK_SIZE_SHIFT
>  	int
>  	default 14
> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> index e457f6acbf..df9eae663c 100644
> --- a/arch/riscv/cpu/cpu.c
> +++ b/arch/riscv/cpu/cpu.c
> @@ -89,7 +89,9 @@ int arch_cpu_init_dm(void)
>  		 * Enable perf counters for cycle, time,
>  		 * and instret counters only
>  		 */
> +#ifndef CONFIG_SYS_RISCV_NOCOUNTER
>  		csr_write(CSR_MCOUNTEREN, GENMASK(2, 0));
> +#endif
>  
>  		/* Disable paging */
>  		if (supports_extension('s'))
Sean Anderson Jan. 26, 2020, 10:24 p.m. UTC | #2
On 1/26/20 5:09 PM, Lukas Auer wrote:
> + Bin, Anup, Atish
> 
> 
> On Wed, 2020-01-15 at 17:53 -0500, Sean Anderson wrote:
>> On the kendryte k210, writes to mcounteren result in an illegal instruction
>> exception.
>>
>> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
>> ---
>> Changes for v2:
>>  Moved forward in the patch series
>>
>>  arch/riscv/Kconfig   | 3 +++
>>  arch/riscv/cpu/cpu.c | 2 ++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 9a7b0334c2..4f8c62dcff 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -226,6 +226,9 @@ config XIP
>>  	  from a NOR flash memory without copying the code to ram.
>>  	  Say yes here if U-Boot boots from flash directly.
>>  
>> +config SYS_RISCV_NOCOUNTER
>> +	bool "Disable accesses to the mcounteren CSR"
>> +
> 
> Can you rename this to something like RISCV_PRIV_1_9_1?
> 
> The k210 implements version 1.9.1 of the privileged spec (if I remember
> correctly). The mcounteren CSR doesn't exist in that version and
> therefore triggers the illegal instruction exception. By renaming the
> config entry, it is clearer why the CSR is missing and is therefore not
> accessed.

Thanks, I was not aware that the k210 was following a different spec
when I made the change. For v3 I can add this functionality back using
the old counter CSRs.

> I am not too familiar with the changes between the versions of the
> spec. Are there other parts of the code we need to adapt?

>From reading the changelog, most of the changes seem related to virtual
memory, which doesn't apply to u-boot.

--Sean
Lukas Auer Jan. 30, 2020, 10:13 p.m. UTC | #3
On Sun, 2020-01-26 at 17:24 -0500, Sean Anderson wrote:
> On 1/26/20 5:09 PM, Lukas Auer wrote:
> > + Bin, Anup, Atish
> > 
> > 
> > On Wed, 2020-01-15 at 17:53 -0500, Sean Anderson wrote:
> > > On the kendryte k210, writes to mcounteren result in an illegal instruction
> > > exception.
> > > 
> > > Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> > > ---
> > > Changes for v2:
> > >  Moved forward in the patch series
> > > 
> > >  arch/riscv/Kconfig   | 3 +++
> > >  arch/riscv/cpu/cpu.c | 2 ++
> > >  2 files changed, 5 insertions(+)
> > > 
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 9a7b0334c2..4f8c62dcff 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -226,6 +226,9 @@ config XIP
> > >  	  from a NOR flash memory without copying the code to ram.
> > >  	  Say yes here if U-Boot boots from flash directly.
> > >  
> > > +config SYS_RISCV_NOCOUNTER
> > > +	bool "Disable accesses to the mcounteren CSR"
> > > +
> > 
> > Can you rename this to something like RISCV_PRIV_1_9_1?
> > 
> > The k210 implements version 1.9.1 of the privileged spec (if I remember
> > correctly). The mcounteren CSR doesn't exist in that version and
> > therefore triggers the illegal instruction exception. By renaming the
> > config entry, it is clearer why the CSR is missing and is therefore not
> > accessed.
> 
> Thanks, I was not aware that the k210 was following a different spec
> when I made the change. For v3 I can add this functionality back using
> the old counter CSRs.
> 
> > I am not too familiar with the changes between the versions of the
> > spec. Are there other parts of the code we need to adapt?
> 
> From reading the changelog, most of the changes seem related to virtual
> memory, which doesn't apply to u-boot.
> 

Ok, great. Thanks for checking!

Regards,
Lukas
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 9a7b0334c2..4f8c62dcff 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -226,6 +226,9 @@  config XIP
 	  from a NOR flash memory without copying the code to ram.
 	  Say yes here if U-Boot boots from flash directly.
 
+config SYS_RISCV_NOCOUNTER
+	bool "Disable accesses to the mcounteren CSR"
+
 config STACK_SIZE_SHIFT
 	int
 	default 14
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index e457f6acbf..df9eae663c 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -89,7 +89,9 @@  int arch_cpu_init_dm(void)
 		 * Enable perf counters for cycle, time,
 		 * and instret counters only
 		 */
+#ifndef CONFIG_SYS_RISCV_NOCOUNTER
 		csr_write(CSR_MCOUNTEREN, GENMASK(2, 0));
+#endif
 
 		/* Disable paging */
 		if (supports_extension('s'))