| Message ID | 39c496a7-bc32-8257-86b6-4a3cfc49e333@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: Add Sipeed Maix support | expand |
Hi Sean, On Mon, Feb 3, 2020 at 4:05 AM Sean Anderson <seanga2 at gmail.com> wrote: > > Some older processors (notably the Kendryte K210) use an older version of the > RISC-V privileged specification. The primary changes between the old and new are > in virtual memory, and in the merging of three separate counter enable CSRs. > Using the new CSR on an old processor causes an illegal instruction exception. > This patch adds an option to use the old CSRs instead of the new one. > > Signed-off-by: Sean Anderson <seanga2 at gmail.com> > --- > Changes for v3: > - Renamed from "riscv: Add option to disable writes to mcounteren" > - Added original functionality back for older priv specs. > Changes for v2: > - Moved forward in the patch series > > arch/riscv/Kconfig | 10 ++++++++++ > arch/riscv/cpu/cpu.c | 6 ++++++ > arch/riscv/include/asm/csr.h | 6 ++++++ > 3 files changed, 22 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 85e15ebffa..87c40f6c4c 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -222,6 +222,16 @@ config XIP > from a NOR flash memory without copying the code to ram. > Say yes here if U-Boot boots from flash directly. > > +config RISCV_PRIV_1_9_1 > + bool "Use version 1.9.1 of the RISC-V priviledged specification" typo: privileged > + help > + Older versions of the RISC-V priviledged specification had typo: privileged > + separate counter enable CSRs for each privilege mode. Writing > + to the unified mcounteren CSR on a processor implementing the > + old specification will result in an illegal instruction > + exception. In addition to counter CSR changes, the way virtual > + memory is configured was also changed. > + > config STACK_SIZE_SHIFT > int > default 14 > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > index e457f6acbf..83cb6557cd 100644 > --- a/arch/riscv/cpu/cpu.c > +++ b/arch/riscv/cpu/cpu.c > @@ -89,7 +89,13 @@ int arch_cpu_init_dm(void) > * Enable perf counters for cycle, time, > * and instret counters only > */ > +#ifdef CONFIG_RISCV_PRIV_1_9_1 > + /* FIXME: Can't use the macro for some reason... */ This is weird ... > + csr_write(mscounteren, GENMASK(2, 0)); > + csr_write(mucounteren, GENMASK(2, 0)); > +#else > csr_write(CSR_MCOUNTEREN, GENMASK(2, 0)); > +#endif > > /* Disable paging */ > if (supports_extension('s')) > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h > index d1520743a2..c16b65d3f3 100644 > --- a/arch/riscv/include/asm/csr.h > +++ b/arch/riscv/include/asm/csr.h > @@ -93,7 +93,13 @@ > #define CSR_MISA 0x301 > #define CSR_MIE 0x304 > #define CSR_MTVEC 0x305 > +#ifdef RISCV_PRIV_1_9_1 > +#define CSR_MUCOUNTEREN 0x320 > +#define CSR_MSCOUNTEREN 0x321 > +#define CSR_MHCOUNTEREN 0x322 > +#else > #define CSR_MCOUNTEREN 0x306 > +#endif > #define CSR_MSCRATCH 0x340 > #define CSR_MEPC 0x341 > #define CSR_MCAUSE 0x342 > -- Other than above, Reviewed-by: Bin Meng <bmeng.cn at gmail.com> Regards, Bin
On 2/4/20 6:21 AM, Bin Meng wrote: > Hi Sean, > > On Mon, Feb 3, 2020 at 4:05 AM Sean Anderson <seanga2 at gmail.com> wrote: >> >> Some older processors (notably the Kendryte K210) use an older version of the >> RISC-V privileged specification. The primary changes between the old and new are >> in virtual memory, and in the merging of three separate counter enable CSRs. >> Using the new CSR on an old processor causes an illegal instruction exception. >> This patch adds an option to use the old CSRs instead of the new one. >> >> Signed-off-by: Sean Anderson <seanga2 at gmail.com> >> --- >> Changes for v3: >> - Renamed from "riscv: Add option to disable writes to mcounteren" >> - Added original functionality back for older priv specs. >> Changes for v2: >> - Moved forward in the patch series >> >> arch/riscv/Kconfig | 10 ++++++++++ >> arch/riscv/cpu/cpu.c | 6 ++++++ >> arch/riscv/include/asm/csr.h | 6 ++++++ >> 3 files changed, 22 insertions(+) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 85e15ebffa..87c40f6c4c 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -222,6 +222,16 @@ config XIP >> from a NOR flash memory without copying the code to ram. >> Say yes here if U-Boot boots from flash directly. >> >> +config RISCV_PRIV_1_9_1 >> + bool "Use version 1.9.1 of the RISC-V priviledged specification" > > typo: privileged > >> + help >> + Older versions of the RISC-V priviledged specification had > > typo: privileged Whoops, will fix that for v4. >> + separate counter enable CSRs for each privilege mode. Writing >> + to the unified mcounteren CSR on a processor implementing the >> + old specification will result in an illegal instruction >> + exception. In addition to counter CSR changes, the way virtual >> + memory is configured was also changed. >> + >> config STACK_SIZE_SHIFT >> int >> default 14 >> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c >> index e457f6acbf..83cb6557cd 100644 >> --- a/arch/riscv/cpu/cpu.c >> +++ b/arch/riscv/cpu/cpu.c >> @@ -89,7 +89,13 @@ int arch_cpu_init_dm(void) >> * Enable perf counters for cycle, time, >> * and instret counters only >> */ >> +#ifdef CONFIG_RISCV_PRIV_1_9_1 >> + /* FIXME: Can't use the macro for some reason... */ > > This is weird ... I believe the macro compiles to "csrs CSR_FOO". At least with my gcc/binutils (9.2.0/2.33.1) this style is not available for these older CSRs. Perhaps it would work if we switched to letting it compile with the numeric CSR as defined earlier in asm/csr.h --Sean
Hi Sean, On Tue, Feb 4, 2020 at 10:19 PM Sean Anderson <seanga2 at gmail.com> wrote: > > > On 2/4/20 6:21 AM, Bin Meng wrote: > > Hi Sean, > > > > On Mon, Feb 3, 2020 at 4:05 AM Sean Anderson <seanga2 at gmail.com> wrote: > >> > >> Some older processors (notably the Kendryte K210) use an older version of the > >> RISC-V privileged specification. The primary changes between the old and new are > >> in virtual memory, and in the merging of three separate counter enable CSRs. > >> Using the new CSR on an old processor causes an illegal instruction exception. > >> This patch adds an option to use the old CSRs instead of the new one. > >> > >> Signed-off-by: Sean Anderson <seanga2 at gmail.com> > >> --- > >> Changes for v3: > >> - Renamed from "riscv: Add option to disable writes to mcounteren" > >> - Added original functionality back for older priv specs. > >> Changes for v2: > >> - Moved forward in the patch series > >> > >> arch/riscv/Kconfig | 10 ++++++++++ > >> arch/riscv/cpu/cpu.c | 6 ++++++ > >> arch/riscv/include/asm/csr.h | 6 ++++++ > >> 3 files changed, 22 insertions(+) > >> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >> index 85e15ebffa..87c40f6c4c 100644 > >> --- a/arch/riscv/Kconfig > >> +++ b/arch/riscv/Kconfig > >> @@ -222,6 +222,16 @@ config XIP > >> from a NOR flash memory without copying the code to ram. > >> Say yes here if U-Boot boots from flash directly. > >> > >> +config RISCV_PRIV_1_9_1 > >> + bool "Use version 1.9.1 of the RISC-V priviledged specification" > > > > typo: privileged > > > >> + help > >> + Older versions of the RISC-V priviledged specification had > > > > typo: privileged > > Whoops, will fix that for v4. > > >> + separate counter enable CSRs for each privilege mode. Writing > >> + to the unified mcounteren CSR on a processor implementing the > >> + old specification will result in an illegal instruction > >> + exception. In addition to counter CSR changes, the way virtual > >> + memory is configured was also changed. > >> + > >> config STACK_SIZE_SHIFT > >> int > >> default 14 > >> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > >> index e457f6acbf..83cb6557cd 100644 > >> --- a/arch/riscv/cpu/cpu.c > >> +++ b/arch/riscv/cpu/cpu.c > >> @@ -89,7 +89,13 @@ int arch_cpu_init_dm(void) > >> * Enable perf counters for cycle, time, > >> * and instret counters only > >> */ > >> +#ifdef CONFIG_RISCV_PRIV_1_9_1 > >> + /* FIXME: Can't use the macro for some reason... */ > > > > This is weird ... > > I believe the macro compiles to "csrs CSR_FOO". At least with my > gcc/binutils (9.2.0/2.33.1) this style is not available for these older > CSRs. Perhaps it would work if we switched to letting it compile with > the numeric CSR as defined earlier in asm/csr.h It's already using the numeric CSR for csr_write(). Could you double check? Regards, Bin
On 2/4/20 9:38 AM, Bin Meng wrote: > Hi Sean, > > On Tue, Feb 4, 2020 at 10:19 PM Sean Anderson <seanga2 at gmail.com> wrote: >> I believe the macro compiles to "csrs CSR_FOO". At least with my >> gcc/binutils (9.2.0/2.33.1) this style is not available for these older >> CSRs. Perhaps it would work if we switched to letting it compile with >> the numeric CSR as defined earlier in asm/csr.h > > It's already using the numeric CSR for csr_write(). Could you double check? Well, the current definition is #define csr_write(csr, val) \ ({ \ unsigned long __v = (unsigned long)(val); \ __asm__ __volatile__ ("csrw " __ASM_STR(csr) ", %0" \ : : "rK" (__v) \ : "memory"); \ }) and _ASM_STR(csr) evaluates to #csr. I think that results in something like __asm__("csrw " "CSR_FOO" ", %0" In any case, the errors I get are arch/riscv/cpu/cpu.c: Assembler messages: arch/riscv/cpu/cpu.c:94: Error: unknown CSR `CSR_MSCOUNTEREN' arch/riscv/cpu/cpu.c:94: Error: unknown CSR `CSR_MSCOUNTEREN' which doesn't seem like a numeric CSR to me. --Sean
Hi Sean, On Tue, Feb 4, 2020 at 10:48 PM Sean Anderson <seanga2 at gmail.com> wrote: > > On 2/4/20 9:38 AM, Bin Meng wrote: > > Hi Sean, > > > > On Tue, Feb 4, 2020 at 10:19 PM Sean Anderson <seanga2 at gmail.com> wrote: > >> I believe the macro compiles to "csrs CSR_FOO". At least with my > >> gcc/binutils (9.2.0/2.33.1) this style is not available for these older > >> CSRs. Perhaps it would work if we switched to letting it compile with > >> the numeric CSR as defined earlier in asm/csr.h > > > > It's already using the numeric CSR for csr_write(). Could you double check? > > Well, the current definition is > > #define csr_write(csr, val) \ > ({ \ > unsigned long __v = (unsigned long)(val); \ > __asm__ __volatile__ ("csrw " __ASM_STR(csr) ", %0" \ > : : "rK" (__v) \ > : "memory"); \ > }) > > and _ASM_STR(csr) evaluates to #csr. I think that results in something > like > > __asm__("csrw " "CSR_FOO" ", %0" Yes, this generates numeric CSR for us. > > In any case, the errors I get are > > arch/riscv/cpu/cpu.c: Assembler messages: > arch/riscv/cpu/cpu.c:94: Error: unknown CSR `CSR_MSCOUNTEREN' > arch/riscv/cpu/cpu.c:94: Error: unknown CSR `CSR_MSCOUNTEREN' > > which doesn't seem like a numeric CSR to me. Oops, I did a careful look and found that's because 'CSR_MSCOUNTEREN' is undefined. +#ifdef RISCV_PRIV_1_9_1 This should be: CONFIG_RISCV_PRIV_1_9_1 +#define CSR_MUCOUNTEREN 0x320 +#define CSR_MSCOUNTEREN 0x321 +#define CSR_MHCOUNTEREN 0x322 +#else #define CSR_MCOUNTEREN 0x306 +#endif Regards, Bin
On 2/4/20 11:04 AM, Bin Meng wrote: > Hi Sean, > > On Tue, Feb 4, 2020 at 10:48 PM Sean Anderson <seanga2 at gmail.com> wrote: >> In any case, the errors I get are >> >> arch/riscv/cpu/cpu.c: Assembler messages: >> arch/riscv/cpu/cpu.c:94: Error: unknown CSR `CSR_MSCOUNTEREN' >> arch/riscv/cpu/cpu.c:94: Error: unknown CSR `CSR_MSCOUNTEREN' >> >> which doesn't seem like a numeric CSR to me. > > Oops, I did a careful look and found that's because 'CSR_MSCOUNTEREN' > is undefined. > > +#ifdef RISCV_PRIV_1_9_1 > > This should be: CONFIG_RISCV_PRIV_1_9_1 > > +#define CSR_MUCOUNTEREN 0x320 > +#define CSR_MSCOUNTEREN 0x321 > +#define CSR_MHCOUNTEREN 0x322 > +#else > #define CSR_MCOUNTEREN 0x306 > +#endif > > Regards, > Bin Ah, good catch, thanks. I'll be sure to fix that for v4. --Sean
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 85e15ebffa..87c40f6c4c 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -222,6 +222,16 @@ config XIP from a NOR flash memory without copying the code to ram. Say yes here if U-Boot boots from flash directly. +config RISCV_PRIV_1_9_1 + bool "Use version 1.9.1 of the RISC-V priviledged specification" + help + Older versions of the RISC-V priviledged specification had + separate counter enable CSRs for each privilege mode. Writing + to the unified mcounteren CSR on a processor implementing the + old specification will result in an illegal instruction + exception. In addition to counter CSR changes, the way virtual + memory is configured was also changed. + config STACK_SIZE_SHIFT int default 14 diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index e457f6acbf..83cb6557cd 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -89,7 +89,13 @@ int arch_cpu_init_dm(void) * Enable perf counters for cycle, time, * and instret counters only */ +#ifdef CONFIG_RISCV_PRIV_1_9_1 + /* FIXME: Can't use the macro for some reason... */ + csr_write(mscounteren, GENMASK(2, 0)); + csr_write(mucounteren, GENMASK(2, 0)); +#else csr_write(CSR_MCOUNTEREN, GENMASK(2, 0)); +#endif /* Disable paging */ if (supports_extension('s')) diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index d1520743a2..c16b65d3f3 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -93,7 +93,13 @@ #define CSR_MISA 0x301 #define CSR_MIE 0x304 #define CSR_MTVEC 0x305 +#ifdef RISCV_PRIV_1_9_1 +#define CSR_MUCOUNTEREN 0x320 +#define CSR_MSCOUNTEREN 0x321 +#define CSR_MHCOUNTEREN 0x322 +#else #define CSR_MCOUNTEREN 0x306 +#endif #define CSR_MSCRATCH 0x340 #define CSR_MEPC 0x341 #define CSR_MCAUSE 0x342
Some older processors (notably the Kendryte K210) use an older version of the RISC-V privileged specification. The primary changes between the old and new are in virtual memory, and in the merging of three separate counter enable CSRs. Using the new CSR on an old processor causes an illegal instruction exception. This patch adds an option to use the old CSRs instead of the new one. Signed-off-by: Sean Anderson <seanga2 at gmail.com> --- Changes for v3: - Renamed from "riscv: Add option to disable writes to mcounteren" - Added original functionality back for older priv specs. Changes for v2: - Moved forward in the patch series arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/cpu/cpu.c | 6 ++++++ arch/riscv/include/asm/csr.h | 6 ++++++ 3 files changed, 22 insertions(+)