| Message ID | 20240926065422.226518-2-nick.hu@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support SSTC while PM operations | expand |
It seems like my last mail didn't send out successfully. On Wed, Oct 2, 2024 at 9:52 AM Nick Hu <nick.hu@sifive.com> wrote: > > Hi Andrew > > On Thu, Sep 26, 2024 at 3:59 PM Andrew Jones <ajones@ventanamicro.com> wrote: >> >> On Thu, Sep 26, 2024 at 02:54:16PM GMT, Nick Hu wrote: >> > In RV32, we may have the need to read both low 32 bit and high 32 bit of >> > the CSR. Therefore adding the csr_read_hi_lo() and csr_write_hi_lo() to >> > support such case. >> > >> > Suggested-by: Andrew Jones <ajones@ventanamicro.com> >> > Suggested-by: Zong Li <zong.li@sifive.com> >> > Signed-off-by: Nick Hu <nick.hu@sifive.com> >> > --- >> > arch/riscv/include/asm/csr.h | 22 ++++++++++++++++++++++ >> > 1 file changed, 22 insertions(+) >> > >> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h >> > index 25966995da04..54198284eb22 100644 >> > --- a/arch/riscv/include/asm/csr.h >> > +++ b/arch/riscv/include/asm/csr.h >> > @@ -501,6 +501,17 @@ >> > __v; \ >> > }) >> > >> > +#if __riscv_xlen < 64 >> > +#define csr_read_hi_lo(csr) \ >> > +({ \ >> > + u32 hi = csr_read(csr##H); \ >> > + u32 lo = csr_read(csr); \ >> > + lo | ((u64)hi << 32); \ >> > +}) >> > +#else >> > +#define csr_read_hi_lo csr_read >> > +#endif >> > + >> > #define csr_write(csr, val) \ >> > ({ \ >> > unsigned long __v = (unsigned long)(val); \ >> > @@ -509,6 +520,17 @@ >> > : "memory"); \ >> > }) >> > >> > +#if __riscv_xlen < 64 >> > +#define csr_write_hi_lo(csr, val) \ >> > +({ \ >> > + u64 _v = (u64)(val); \ >> > + csr_write(csr##H, (_v) >> 32); \ >> > + csr_write(csr, (_v)); \ >> > +}) >> > +#else >> > +#define csr_write_hi_lo csr_write >> > +#endif >> > + >> > #define csr_read_set(csr, val) \ >> > ({ \ >> > unsigned long __v = (unsigned long)(val); \ >> > -- >> > 2.34.1 >> > >> >> I know I suggested this, but I'm having second thoughts. The nice thing >> about the >> >> csr_write(CSR, ...); >> if (__riscv_xlen < 64) >> csr_write(CSRH, ...); >> >> pattern is that it matches the spec. With this helper we'll have >> >> csr_write_hi_lo(CSR, ...); >> >> for both rv32 and rv64. That looks odd for rv64 and hides the hi register >> access for rv32. >> >> We could avoid the oddness of the helper's name for rv64 if we instead >> added csr_write32 and csr_write64 which do the right things, but that >> still hides the hi register access for rv32. Hiding the hi register >> access is probably fine, though, since we can be pretty certain that >> the spec will rarely, if ever, deviate from naming high registers with >> the H suffix and/or not keep the upper bits compatible. >> >> In summary, I think I'm in favor of just dropping this patch, keeping >> the noisy, but explicit, pattern. Or, if the consensus is to add >> helpers, then I'd rather have all csr_<op>32/64 helpers. Then, code >> would match the spec by choosing the right helper based on the width >> of the CSR being accessed, when the CSR has an explicit width, or still >> use the current helpers for xlen-wide CSRs. >> >> Thanks, >> drew > Sure I'll drop the patch in the next version Regards, Nick
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index 25966995da04..54198284eb22 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -501,6 +501,17 @@ __v; \ }) +#if __riscv_xlen < 64 +#define csr_read_hi_lo(csr) \ +({ \ + u32 hi = csr_read(csr##H); \ + u32 lo = csr_read(csr); \ + lo | ((u64)hi << 32); \ +}) +#else +#define csr_read_hi_lo csr_read +#endif + #define csr_write(csr, val) \ ({ \ unsigned long __v = (unsigned long)(val); \ @@ -509,6 +520,17 @@ : "memory"); \ }) +#if __riscv_xlen < 64 +#define csr_write_hi_lo(csr, val) \ +({ \ + u64 _v = (u64)(val); \ + csr_write(csr##H, (_v) >> 32); \ + csr_write(csr, (_v)); \ +}) +#else +#define csr_write_hi_lo csr_write +#endif + #define csr_read_set(csr, val) \ ({ \ unsigned long __v = (unsigned long)(val); \
In RV32, we may have the need to read both low 32 bit and high 32 bit of the CSR. Therefore adding the csr_read_hi_lo() and csr_write_hi_lo() to support such case. Suggested-by: Andrew Jones <ajones@ventanamicro.com> Suggested-by: Zong Li <zong.li@sifive.com> Signed-off-by: Nick Hu <nick.hu@sifive.com> --- arch/riscv/include/asm/csr.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)