[U-Boot] [PATCH 2/2] riscv: Access CSRs using CSR numbers

Auer, Lukas lukas.auer at aisec.fraunhofer.de
Thu Jul 18 09:28:18 UTC 2019


Hi Bin,

On Thu, 2019-07-18 at 17:01 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Thu, Jul 18, 2019 at 4:54 PM Auer, Lukas
> <lukas.auer at aisec.fraunhofer.de> wrote:
> > Hi Bin,
> > 
> > On Wed, 2019-07-10 at 23:43 -0700, Bin Meng wrote:
> > > We should prefer accessing CSRs using their CSR numbers
> > > because:
> > > 1. It compiles fine with older toolchains.
> > > 2. We can use latest CSR names in #define macro names of CSR
> > >    numbers as-per RISC-V spec.
> > > 3. We can access newly added CSRs even if toolchain does not
> > >    recognize newly added CSRs by name.
> > > 
> > > This commit is inspired from Linux kernel commit a3182c91ef4e
> > > ("RISC-V: Access CSRs using CSR numbers").
> > > 
> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > ---
> > > 
> > >  arch/riscv/cpu/cpu.c              |   9 +-
> > >  arch/riscv/cpu/start.S            |   3 +-
> > >  arch/riscv/include/asm/csr.h      |  12 ++
> > >  arch/riscv/include/asm/encoding.h | 238 +-------------------------------------
> > >  4 files changed, 19 insertions(+), 243 deletions(-)
> > > 
> > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > > index e9a8b43..5ca1857 100644
> > > --- a/arch/riscv/cpu/cpu.c
> > > +++ b/arch/riscv/cpu/cpu.c
> > > @@ -7,7 +7,6 @@
> > >  #include <cpu.h>
> > >  #include <dm.h>
> > >  #include <log.h>
> > > -#include <asm/csr.h>
> > >  #include <asm/encoding.h>
> > >  #include <dm/uclass-internal.h>
> > > 
> > > @@ -48,7 +47,7 @@ static inline bool supports_extension(char ext)
> > >       return false;
> > >  #else  /* !CONFIG_CPU */
> > >  #ifdef CONFIG_RISCV_MMODE
> > > -     return csr_read(misa) & (1 << (ext - 'a'));
> > > +     return csr_read(CSR_MISA) & (1 << (ext - 'a'));
> > >  #else  /* !CONFIG_RISCV_MMODE */
> > >  #warning "There is no way to determine the available extensions in S-mode."
> > >  #warning "Please convert your board to use the RISC-V CPU driver."
> > > @@ -82,7 +81,7 @@ int arch_cpu_init_dm(void)
> > >       /* Enable FPU */
> > >       if (supports_extension('d') || supports_extension('f')) {
> > >               csr_set(MODE_PREFIX(status), MSTATUS_FS);
> > > -             csr_write(fcsr, 0);
> > > +             csr_write(CSR_FCSR, 0);
> > >       }
> > > 
> > >       if (CONFIG_IS_ENABLED(RISCV_MMODE)) {
> > > @@ -90,11 +89,11 @@ int arch_cpu_init_dm(void)
> > >                * Enable perf counters for cycle, time,
> > >                * and instret counters only
> > >                */
> > > -             csr_write(mcounteren, GENMASK(2, 0));
> > > +             csr_write(CSR_MCOUNTEREN, GENMASK(2, 0));
> > > 
> > >               /* Disable paging */
> > >               if (supports_extension('s'))
> > > -                     csr_write(satp, 0);
> > > +                     csr_write(CSR_SATP, 0);
> > >       }
> > > 
> > >       return 0;
> > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > > index 60ac8c6..e06db40 100644
> > > --- a/arch/riscv/cpu/start.S
> > > +++ b/arch/riscv/cpu/start.S
> > > @@ -13,7 +13,6 @@
> > >  #include <config.h>
> > >  #include <common.h>
> > >  #include <elf.h>
> > > -#include <asm/csr.h>
> > >  #include <asm/encoding.h>
> > >  #include <generated/asm-offsets.h>
> > > 
> > > @@ -41,7 +40,7 @@ secondary_harts_relocation_error:
> > >  .globl _start
> > >  _start:
> > >  #ifdef CONFIG_RISCV_MMODE
> > > -     csrr    a0, mhartid
> > > +     csrr    a0, CSR_MHARTID
> > >  #endif
> > > 
> > >       /* save hart id and dtb pointer */
> > > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > > index 1a6bcfc..d152074 100644
> > > --- a/arch/riscv/include/asm/csr.h
> > > +++ b/arch/riscv/include/asm/csr.h
> > > @@ -75,6 +75,7 @@
> > >  #define SIE_STIE             (_AC(0x1, UL) << IRQ_S_TIMER)
> > >  #define SIE_SEIE             (_AC(0x1, UL) << IRQ_S_EXT)
> > > 
> > > +#define CSR_FCSR             0x003
> > >  #define CSR_CYCLE            0xc00
> > >  #define CSR_TIME             0xc01
> > >  #define CSR_INSTRET          0xc02
> > > @@ -88,9 +89,20 @@
> > >  #define CSR_STVAL            0x143
> > >  #define CSR_SIP                      0x144
> > >  #define CSR_SATP             0x180
> > > +#define CSR_MSTATUS          0x300
> > > +#define CSR_MISA             0x301
> > > +#define CSR_MIE                      0x304
> > > +#define CSR_MTVEC            0x305
> > > +#define CSR_MCOUNTEREN               0x306
> > > +#define CSR_MSCRATCH         0x340
> > > +#define CSR_MEPC             0x341
> > > +#define CSR_MCAUSE           0x342
> > > +#define CSR_MTVAL            0x343
> > > +#define CSR_MIP                      0x344
> > >  #define CSR_CYCLEH           0xc80
> > >  #define CSR_TIMEH            0xc81
> > >  #define CSR_INSTRETH         0xc82
> > > +#define CSR_MHARTID          0xf14
> > > 
> > >  #ifndef __ASSEMBLY__
> > > 
> > > diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h
> > > index 772668c..c450eb9 100644
> > > --- a/arch/riscv/include/asm/encoding.h
> > > +++ b/arch/riscv/include/asm/encoding.h
> > > @@ -7,6 +7,8 @@
> > >  #ifndef RISCV_CSR_ENCODING_H
> > >  #define RISCV_CSR_ENCODING_H
> > > 
> > > +#include <asm/csr.h>
> > > +
> > >  #ifdef CONFIG_RISCV_SMODE
> > >  #define MODE_PREFIX(__suffix)        s##__suffix
> > >  #else
> > > @@ -49,13 +51,10 @@
> > >  #define SSTATUS64_SD 0x8000000000000000
> > > 
> > >  #define MIP_SSIP     BIT(IRQ_S_SOFT)
> > > -#define MIP_HSIP     BIT(IRQ_H_SOFT)
> > >  #define MIP_MSIP     BIT(IRQ_M_SOFT)
> > >  #define MIP_STIP     BIT(IRQ_S_TIMER)
> > > -#define MIP_HTIP     BIT(IRQ_H_TIMER)
> > >  #define MIP_MTIP     BIT(IRQ_M_TIMER)
> > >  #define MIP_SEIP     BIT(IRQ_S_EXT)
> > > -#define MIP_HEIP     BIT(IRQ_H_EXT)
> > >  #define MIP_MEIP     BIT(IRQ_M_EXT)
> > > 
> > >  #define SIP_SSIP     MIP_SSIP
> > > @@ -73,18 +72,6 @@
> > >  #define VM_SV39              9
> > >  #define VM_SV48              10
> > > 
> > > -#define IRQ_S_SOFT   1
> > > -#define IRQ_H_SOFT   2
> > > -#define IRQ_M_SOFT   3
> > > -#define IRQ_S_TIMER  5
> > > -#define IRQ_H_TIMER  6
> > > -#define IRQ_M_TIMER  7
> > > -#define IRQ_S_EXT    9
> > > -#define IRQ_H_EXT    10
> > > -#define IRQ_M_EXT    11
> > > -#define IRQ_COP              12
> > > -#define IRQ_HOST     13
> > > -
> > >  #define CAUSE_MISALIGNED_FETCH               0
> > >  #define CAUSE_FETCH_ACCESS           1
> > >  #define CAUSE_ILLEGAL_INSTRUCTION    2
> > > @@ -167,227 +154,6 @@
> > >  #define RISCV_PGSHIFT 12
> > >  #define RISCV_PGSIZE BIT(RISCV_PGSHIFT)
> > > 
> > > -/* CSR numbers */
> > > -#define CSR_FFLAGS           0x1
> > > -#define CSR_FRM                      0x2
> > > -#define CSR_FCSR             0x3
> > > -
> > > -#define CSR_SSTATUS          0x100
> > > -#define CSR_SEDELEG          0x102
> > > -#define CSR_SIDELEG          0x103
> > > -#define CSR_SIE                      0x104
> > > -#define CSR_STVEC            0x105
> > > -#define CSR_SCOUNTEREN               0x106
> > > -#define CSR_SSCRATCH         0x140
> > > -#define CSR_SEPC             0x141
> > > -#define CSR_SCAUSE           0x142
> > > -#define CSR_STVAL            0x143
> > > -#define CSR_SIP                      0x144
> > > -#define CSR_SATP             0x180
> > > -
> > > -#define CSR_MSTATUS          0x300
> > > -#define CSR_MISA             0x301
> > > -#define CSR_MEDELEG          0x302
> > > -#define CSR_MIDELEG          0x303
> > > -#define CSR_MIE                      0x304
> > > -#define CSR_MTVEC            0x305
> > > -#define CSR_MCOUNTEREN               0x306
> > > -#define CSR_MHPMEVENT3               0x323
> > > -#define CSR_MHPMEVENT4               0x324
> > > -#define CSR_MHPMEVENT5               0x325
> > > -#define CSR_MHPMEVENT6               0x326
> > > -#define CSR_MHPMEVENT7               0x327
> > > -#define CSR_MHPMEVENT8               0x328
> > > -#define CSR_MHPMEVENT9               0x329
> > > -#define CSR_MHPMEVENT10              0x32a
> > > -#define CSR_MHPMEVENT11              0x32b
> > > -#define CSR_MHPMEVENT12              0x32c
> > > -#define CSR_MHPMEVENT13              0x32d
> > > -#define CSR_MHPMEVENT14              0x32e
> > > -#define CSR_MHPMEVENT15              0x32f
> > > -#define CSR_MHPMEVENT16              0x330
> > > -#define CSR_MHPMEVENT17              0x331
> > > -#define CSR_MHPMEVENT18              0x332
> > > -#define CSR_MHPMEVENT19              0x333
> > > -#define CSR_MHPMEVENT20              0x334
> > > -#define CSR_MHPMEVENT21              0x335
> > > -#define CSR_MHPMEVENT22              0x336
> > > -#define CSR_MHPMEVENT23              0x337
> > > -#define CSR_MHPMEVENT24              0x338
> > > -#define CSR_MHPMEVENT25              0x339
> > > -#define CSR_MHPMEVENT26              0x33a
> > > -#define CSR_MHPMEVENT27              0x33b
> > > -#define CSR_MHPMEVENT28              0x33c
> > > -#define CSR_MHPMEVENT29              0x33d
> > > -#define CSR_MHPMEVENT30              0x33e
> > > -#define CSR_MHPMEVENT31              0x33f
> > > -#define CSR_MSCRATCH         0x340
> > > -#define CSR_MEPC             0x341
> > > -#define CSR_MCAUSE           0x342
> > > -#define CSR_MTVAL            0x343
> > > -#define CSR_MIP                      0x344
> > > -#define CSR_PMPCFG0          0x3a0
> > > -#define CSR_PMPCFG1          0x3a1
> > > -#define CSR_PMPCFG2          0x3a2
> > > -#define CSR_PMPCFG3          0x3a3
> > > -#define CSR_PMPADDR0         0x3b0
> > > -#define CSR_PMPADDR1         0x3b1
> > > -#define CSR_PMPADDR2         0x3b2
> > > -#define CSR_PMPADDR3         0x3b3
> > > -#define CSR_PMPADDR4         0x3b4
> > > -#define CSR_PMPADDR5         0x3b5
> > > -#define CSR_PMPADDR6         0x3b6
> > > -#define CSR_PMPADDR7         0x3b7
> > > -#define CSR_PMPADDR8         0x3b8
> > > -#define CSR_PMPADDR9         0x3b9
> > > -#define CSR_PMPADDR10                0x3ba
> > > -#define CSR_PMPADDR11                0x3bb
> > > -#define CSR_PMPADDR12                0x3bc
> > > -#define CSR_PMPADDR13                0x3bd
> > > -#define CSR_PMPADDR14                0x3be
> > > -#define CSR_PMPADDR15                0x3bf
> > > -
> > > -#define CSR_TSELECT          0x7a0
> > > -#define CSR_TDATA1           0x7a1
> > > -#define CSR_TDATA2           0x7a2
> > > -#define CSR_TDATA3           0x7a3
> > > -#define CSR_DCSR             0x7b0
> > > -#define CSR_DPC                      0x7b1
> > > -#define CSR_DSCRATCH         0x7b2
> > > -
> > > -#define CSR_MCYCLE           0xb00
> > > -#define CSR_MINSTRET         0xb02
> > > -#define CSR_MHPMCOUNTER3     0xb03
> > > -#define CSR_MHPMCOUNTER4     0xb04
> > > -#define CSR_MHPMCOUNTER5     0xb05
> > > -#define CSR_MHPMCOUNTER6     0xb06
> > > -#define CSR_MHPMCOUNTER7     0xb07
> > > -#define CSR_MHPMCOUNTER8     0xb08
> > > -#define CSR_MHPMCOUNTER9     0xb09
> > > -#define CSR_MHPMCOUNTER10    0xb0a
> > > -#define CSR_MHPMCOUNTER11    0xb0b
> > > -#define CSR_MHPMCOUNTER12    0xb0c
> > > -#define CSR_MHPMCOUNTER13    0xb0d
> > > -#define CSR_MHPMCOUNTER14    0xb0e
> > > -#define CSR_MHPMCOUNTER15    0xb0f
> > > -#define CSR_MHPMCOUNTER16    0xb10
> > > -#define CSR_MHPMCOUNTER17    0xb11
> > > -#define CSR_MHPMCOUNTER18    0xb12
> > > -#define CSR_MHPMCOUNTER19    0xb13
> > > -#define CSR_MHPMCOUNTER20    0xb14
> > > -#define CSR_MHPMCOUNTER21    0xb15
> > > -#define CSR_MHPMCOUNTER22    0xb16
> > > -#define CSR_MHPMCOUNTER23    0xb17
> > > -#define CSR_MHPMCOUNTER24    0xb18
> > > -#define CSR_MHPMCOUNTER25    0xb19
> > > -#define CSR_MHPMCOUNTER26    0xb1a
> > > -#define CSR_MHPMCOUNTER27    0xb1b
> > > -#define CSR_MHPMCOUNTER28    0xb1c
> > > -#define CSR_MHPMCOUNTER29    0xb1d
> > > -#define CSR_MHPMCOUNTER30    0xb1e
> > > -#define CSR_MHPMCOUNTER31    0xb1f
> > > -#define CSR_MCYCLEH          0xb80
> > > -#define CSR_MINSTRETH                0xb82
> > > -#define CSR_MHPMCOUNTER3H    0xb83
> > > -#define CSR_MHPMCOUNTER4H    0xb84
> > > -#define CSR_MHPMCOUNTER5H    0xb85
> > > -#define CSR_MHPMCOUNTER6H    0xb86
> > > -#define CSR_MHPMCOUNTER7H    0xb87
> > > -#define CSR_MHPMCOUNTER8H    0xb88
> > > -#define CSR_MHPMCOUNTER9H    0xb89
> > > -#define CSR_MHPMCOUNTER10H   0xb8a
> > > -#define CSR_MHPMCOUNTER11H   0xb8b
> > > -#define CSR_MHPMCOUNTER12H   0xb8c
> > > -#define CSR_MHPMCOUNTER13H   0xb8d
> > > -#define CSR_MHPMCOUNTER14H   0xb8e
> > > -#define CSR_MHPMCOUNTER15H   0xb8f
> > > -#define CSR_MHPMCOUNTER16H   0xb90
> > > -#define CSR_MHPMCOUNTER17H   0xb91
> > > -#define CSR_MHPMCOUNTER18H   0xb92
> > > -#define CSR_MHPMCOUNTER19H   0xb93
> > > -#define CSR_MHPMCOUNTER20H   0xb94
> > > -#define CSR_MHPMCOUNTER21H   0xb95
> > > -#define CSR_MHPMCOUNTER22H   0xb96
> > > -#define CSR_MHPMCOUNTER23H   0xb97
> > > -#define CSR_MHPMCOUNTER24H   0xb98
> > > -#define CSR_MHPMCOUNTER25H   0xb99
> > > -#define CSR_MHPMCOUNTER26H   0xb9a
> > > -#define CSR_MHPMCOUNTER27H   0xb9b
> > > -#define CSR_MHPMCOUNTER28H   0xb9c
> > > -#define CSR_MHPMCOUNTER29H   0xb9d
> > > -#define CSR_MHPMCOUNTER30H   0xb9e
> > > -#define CSR_MHPMCOUNTER31H   0xb9f
> > > -
> > > -#define CSR_CYCLE            0xc00
> > > -#define CSR_TIME             0xc01
> > > -#define CSR_INSTRET          0xc02
> > > -#define CSR_HPMCOUNTER3              0xc03
> > > -#define CSR_HPMCOUNTER4              0xc04
> > > -#define CSR_HPMCOUNTER5              0xc05
> > > -#define CSR_HPMCOUNTER6              0xc06
> > > -#define CSR_HPMCOUNTER7              0xc07
> > > -#define CSR_HPMCOUNTER8              0xc08
> > > -#define CSR_HPMCOUNTER9              0xc09
> > > -#define CSR_HPMCOUNTER10     0xc0a
> > > -#define CSR_HPMCOUNTER11     0xc0b
> > > -#define CSR_HPMCOUNTER12     0xc0c
> > > -#define CSR_HPMCOUNTER13     0xc0d
> > > -#define CSR_HPMCOUNTER14     0xc0e
> > > -#define CSR_HPMCOUNTER15     0xc0f
> > > -#define CSR_HPMCOUNTER16     0xc10
> > > -#define CSR_HPMCOUNTER17     0xc11
> > > -#define CSR_HPMCOUNTER18     0xc12
> > > -#define CSR_HPMCOUNTER19     0xc13
> > > -#define CSR_HPMCOUNTER20     0xc14
> > > -#define CSR_HPMCOUNTER21     0xc15
> > > -#define CSR_HPMCOUNTER22     0xc16
> > > -#define CSR_HPMCOUNTER23     0xc17
> > > -#define CSR_HPMCOUNTER24     0xc18
> > > -#define CSR_HPMCOUNTER25     0xc19
> > > -#define CSR_HPMCOUNTER26     0xc1a
> > > -#define CSR_HPMCOUNTER27     0xc1b
> > > -#define CSR_HPMCOUNTER28     0xc1c
> > > -#define CSR_HPMCOUNTER29     0xc1d
> > > -#define CSR_HPMCOUNTER30     0xc1e
> > > -#define CSR_HPMCOUNTER31     0xc1f
> > > -#define CSR_CYCLEH           0xc80
> > > -#define CSR_TIMEH            0xc81
> > > -#define CSR_INSTRETH         0xc82
> > > -#define CSR_HPMCOUNTER3H     0xc83
> > > -#define CSR_HPMCOUNTER4H     0xc84
> > > -#define CSR_HPMCOUNTER5H     0xc85
> > > -#define CSR_HPMCOUNTER6H     0xc86
> > > -#define CSR_HPMCOUNTER7H     0xc87
> > > -#define CSR_HPMCOUNTER8H     0xc88
> > > -#define CSR_HPMCOUNTER9H     0xc89
> > > -#define CSR_HPMCOUNTER10H    0xc8a
> > > -#define CSR_HPMCOUNTER11H    0xc8b
> > > -#define CSR_HPMCOUNTER12H    0xc8c
> > > -#define CSR_HPMCOUNTER13H    0xc8d
> > > -#define CSR_HPMCOUNTER14H    0xc8e
> > > -#define CSR_HPMCOUNTER15H    0xc8f
> > > -#define CSR_HPMCOUNTER16H    0xc90
> > > -#define CSR_HPMCOUNTER17H    0xc91
> > > -#define CSR_HPMCOUNTER18H    0xc92
> > > -#define CSR_HPMCOUNTER19H    0xc93
> > > -#define CSR_HPMCOUNTER20H    0xc94
> > > -#define CSR_HPMCOUNTER21H    0xc95
> > > -#define CSR_HPMCOUNTER22H    0xc96
> > > -#define CSR_HPMCOUNTER23H    0xc97
> > > -#define CSR_HPMCOUNTER24H    0xc98
> > > -#define CSR_HPMCOUNTER25H    0xc99
> > > -#define CSR_HPMCOUNTER26H    0xc9a
> > > -#define CSR_HPMCOUNTER27H    0xc9b
> > > -#define CSR_HPMCOUNTER28H    0xc9c
> > > -#define CSR_HPMCOUNTER29H    0xc9d
> > > -#define CSR_HPMCOUNTER30H    0xc9e
> > > -#define CSR_HPMCOUNTER31H    0xc9f
> > > -
> > > -#define CSR_MVENDORID                0xf11
> > > -#define CSR_MARCHID          0xf12
> > > -#define CSR_MIMPID           0xf13
> > > -#define CSR_MHARTID          0xf14
> > > -
> > 
> > Why are you removing all CSR_* definitions here?
> > 
> 
> Some of the CSRs here are mainly for M-mode so I just deleted. Another
> reason is to keep in sync with Linux upstream.
> 

Ok, that makes sense.

> > The patch looks good and it is a very good idea to only use CSR
> > numbers. There are still two places left, where we are using CSR names.
> > 
> > The first is in the MODE_PREFIX macro. That should only be a matter of
> > changing it to CSR_M/S##__suffix and capitalizing the suffix in all
> > uses of the macro.
> 
> This one can't be simply replaced because we also have mret/sret. So I
> did not touch that.
> 

Ah right, I did not think about that. Since we are only using pretty
standard CSRs with that macro that should be fine.

Reviewed-by: Lukas Auer <lukas.auer at aisec.fraunhofer.de>

Thanks,
Lukas

> > The second one is in cpu/ax25/cache.c. Rick, can you maybe add a patch
> > to your cache series to convert that file to using CSR numbers instead
> > of names? That would also allow us to build the board in Travis with
> > the cache enabled.
> > 
> 
> Yes, I intended to put that notes in the commit-notes, however it
> looks I missed doing that :)
> 
> Regards,
> Bin


More information about the U-Boot mailing list