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

2020-01-30 Thread Lukas Auer
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 
> > > ---
> > > 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


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

2020-01-26 Thread Sean Anderson
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 
>> ---
>> 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


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

2020-01-26 Thread Lukas Auer
+ 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 
> ---
> 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'))


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

2020-01-15 Thread Sean Anderson
On the kendryte k210, writes to mcounteren result in an illegal instruction
exception.

Signed-off-by: Sean Anderson 
---
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"
+
 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'))
-- 
2.24.1