[v2] lib: sbi: Improve csr read and write

Message ID	20220526141809.303161-1-wxjstz@126.com
State	Deferred
Headers	show Return-Path: <opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org> From: Xiang W <wxjstz@126.com> To: opensbi@lists.infradead.org Cc: anup@brainfault.org, Xiang W <wxjstz@126.com> Subject: [PATCH v2] lib: sbi: Improve csr read and write Date: Thu, 26 May 2022 22:18:09 +0800 Message-Id: <20220526141809.303161-1-wxjstz@126.com> MIME-Version: 1.0 preview: Reduce the overhead of csr read and write code by producing a small piece of code to perform csr read and write. Signed-off-by: Xiang W --- Changes in v2: - Fix thread safety related bugs as suggested by Anup Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [123.126.96.4 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider [wxjstz[at]126.com] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "opensbi" <opensbi-bounces@lists.infradead.org> Errors-To: opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org
Series	[v2] lib: sbi: Improve csr read and write \| expand [v2] lib: sbi: Improve csr read and write

Message ID

20220526141809.303161-1-wxjstz@126.com

State

Deferred

Headers

From: Xiang W <wxjstz@126.com>
To: opensbi@lists.infradead.org
Cc: anup@brainfault.org,
	Xiang W <wxjstz@126.com>
Subject: [PATCH v2] lib: sbi: Improve csr read and write
Date: Thu, 26 May 2022 22:18:09 +0800
Message-Id: <20220526141809.303161-1-wxjstz@126.com>
MIME-Version: 1.0
Precedence: list
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: "opensbi" <opensbi-bounces@lists.infradead.org>
Errors-To: opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org

Series

[v2] lib: sbi: Improve csr read and write | expand

Commit Message

Xiang W May 26, 2022, 2:18 p.m. UTC

Reduce the overhead of csr read and write code by producing a small
piece of code to perform csr read and write.

Signed-off-by: Xiang W <wxjstz@126.com>
---
Changes in v2:
- Fix thread safety related bugs as suggested by Anup

 lib/sbi/riscv_asm.c | 142 ++++----------------------------------------
 1 file changed, 10 insertions(+), 132 deletions(-)

Comments

Jan Remes May 26, 2022, 2:31 p.m. UTC | #1

On Thu, May 26, 2022 at 4:19 PM Xiang W <wxjstz@126.com> wrote:
>
> +       uint32_t opcode_buff[2];
> +       typedef unsigned long (*read_f)(void);
> +       opcode_buff[0] = (csr_num << 20) | 0x00002573; /* csrr a0, csr */
> +       opcode_buff[1] = 0x00008067; /* ret */
> +       return ((read_f)opcode_buff)();
I am not completely sure here, but doesn't this need a FENCE.I before
the opcode is executed?

Best regards,
  Jan

Xiang W May 26, 2022, 2:37 p.m. UTC | #2

在 2022-05-26星期四的 16:31 +0200，Jan Remeš写道：
> On Thu, May 26, 2022 at 4:19 PM Xiang W <wxjstz@126.com> wrote:
> > 
> > +       uint32_t opcode_buff[2];
> > +       typedef unsigned long (*read_f)(void);
> > +       opcode_buff[0] = (csr_num << 20) | 0x00002573; /* csrr a0, csr */
> > +       opcode_buff[1] = 0x00008067; /* ret */
> > +       return ((read_f)opcode_buff)();
> I am not completely sure here, but doesn't this need a FENCE.I before
> the opcode is executed?
Yes, fence.i is need. 

Thx
 Xiang W
> 
> Best regards,
>   Jan

diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
index a09cf78..7966401 100644
--- a/lib/sbi/riscv_asm.c
+++ b/lib/sbi/riscv_asm.c
@@ -93,142 +93,20 @@  void misa_string(int xlen, char *out, unsigned int out_sz)
 
 unsigned long csr_read_num(int csr_num)
 {
-#define switchcase_csr_read(__csr_num, __val)		\
-	case __csr_num:					\
-		__val = csr_read(__csr_num);		\
-		break;
-#define switchcase_csr_read_2(__csr_num, __val)	\
-	switchcase_csr_read(__csr_num + 0, __val)	\
-	switchcase_csr_read(__csr_num + 1, __val)
-#define switchcase_csr_read_4(__csr_num, __val)	\
-	switchcase_csr_read_2(__csr_num + 0, __val)	\
-	switchcase_csr_read_2(__csr_num + 2, __val)
-#define switchcase_csr_read_8(__csr_num, __val)	\
-	switchcase_csr_read_4(__csr_num + 0, __val)	\
-	switchcase_csr_read_4(__csr_num + 4, __val)
-#define switchcase_csr_read_16(__csr_num, __val)	\
-	switchcase_csr_read_8(__csr_num + 0, __val)	\
-	switchcase_csr_read_8(__csr_num + 8, __val)
-#define switchcase_csr_read_32(__csr_num, __val)	\
-	switchcase_csr_read_16(__csr_num + 0, __val)	\
-	switchcase_csr_read_16(__csr_num + 16, __val)
-#define switchcase_csr_read_64(__csr_num, __val)	\
-	switchcase_csr_read_32(__csr_num + 0, __val)	\
-	switchcase_csr_read_32(__csr_num + 32, __val)
-
-	unsigned long ret = 0;
-
-	switch (csr_num) {
-	switchcase_csr_read_16(CSR_PMPCFG0, ret)
-	switchcase_csr_read_64(CSR_PMPADDR0, ret)
-	switchcase_csr_read(CSR_MCYCLE, ret)
-	switchcase_csr_read(CSR_MINSTRET, ret)
-	switchcase_csr_read(CSR_MHPMCOUNTER3, ret)
-	switchcase_csr_read_4(CSR_MHPMCOUNTER4, ret)
-	switchcase_csr_read_8(CSR_MHPMCOUNTER8, ret)
-	switchcase_csr_read_16(CSR_MHPMCOUNTER16, ret)
-	switchcase_csr_read(CSR_MCOUNTINHIBIT, ret)
-	switchcase_csr_read(CSR_MHPMEVENT3, ret)
-	switchcase_csr_read_4(CSR_MHPMEVENT4, ret)
-	switchcase_csr_read_8(CSR_MHPMEVENT8, ret)
-	switchcase_csr_read_16(CSR_MHPMEVENT16, ret)
-#if __riscv_xlen == 32
-	switchcase_csr_read(CSR_MCYCLEH, ret)
-	switchcase_csr_read(CSR_MINSTRETH, ret)
-	switchcase_csr_read(CSR_MHPMCOUNTER3H, ret)
-	switchcase_csr_read_4(CSR_MHPMCOUNTER4H, ret)
-	switchcase_csr_read_8(CSR_MHPMCOUNTER8H, ret)
-	switchcase_csr_read_16(CSR_MHPMCOUNTER16H, ret)
-	/**
-	 * The CSR range MHPMEVENT[3-16]H are available only if sscofpmf
-	 * extension is present. The caller must ensure that.
-	 */
-	switchcase_csr_read(CSR_MHPMEVENT3H, ret)
-	switchcase_csr_read_4(CSR_MHPMEVENT4H, ret)
-	switchcase_csr_read_8(CSR_MHPMEVENT8H, ret)
-	switchcase_csr_read_16(CSR_MHPMEVENT16H, ret)
-#endif
-
-	default:
-		sbi_panic("%s: Unknown CSR %#x", __func__, csr_num);
-		break;
-	};
-
-	return ret;
-
-#undef switchcase_csr_read_64
-#undef switchcase_csr_read_32
-#undef switchcase_csr_read_16
-#undef switchcase_csr_read_8
-#undef switchcase_csr_read_4
-#undef switchcase_csr_read_2
-#undef switchcase_csr_read
+	uint32_t opcode_buff[2];
+	typedef unsigned long (*read_f)(void);
+	opcode_buff[0] = (csr_num << 20) | 0x00002573; /* csrr a0, csr */
+	opcode_buff[1] = 0x00008067; /* ret */
+	return ((read_f)opcode_buff)();
 }
 
 void csr_write_num(int csr_num, unsigned long val)
 {
-#define switchcase_csr_write(__csr_num, __val)		\
-	case __csr_num:					\
-		csr_write(__csr_num, __val);		\
-		break;
-#define switchcase_csr_write_2(__csr_num, __val)	\
-	switchcase_csr_write(__csr_num + 0, __val)	\
-	switchcase_csr_write(__csr_num + 1, __val)
-#define switchcase_csr_write_4(__csr_num, __val)	\
-	switchcase_csr_write_2(__csr_num + 0, __val)	\
-	switchcase_csr_write_2(__csr_num + 2, __val)
-#define switchcase_csr_write_8(__csr_num, __val)	\
-	switchcase_csr_write_4(__csr_num + 0, __val)	\
-	switchcase_csr_write_4(__csr_num + 4, __val)
-#define switchcase_csr_write_16(__csr_num, __val)	\
-	switchcase_csr_write_8(__csr_num + 0, __val)	\
-	switchcase_csr_write_8(__csr_num + 8, __val)
-#define switchcase_csr_write_32(__csr_num, __val)	\
-	switchcase_csr_write_16(__csr_num + 0, __val)	\
-	switchcase_csr_write_16(__csr_num + 16, __val)
-#define switchcase_csr_write_64(__csr_num, __val)	\
-	switchcase_csr_write_32(__csr_num + 0, __val)	\
-	switchcase_csr_write_32(__csr_num + 32, __val)
-
-	switch (csr_num) {
-	switchcase_csr_write_16(CSR_PMPCFG0, val)
-	switchcase_csr_write_64(CSR_PMPADDR0, val)
-	switchcase_csr_write(CSR_MCYCLE, val)
-	switchcase_csr_write(CSR_MINSTRET, val)
-	switchcase_csr_write(CSR_MHPMCOUNTER3, val)
-	switchcase_csr_write_4(CSR_MHPMCOUNTER4, val)
-	switchcase_csr_write_8(CSR_MHPMCOUNTER8, val)
-	switchcase_csr_write_16(CSR_MHPMCOUNTER16, val)
-#if __riscv_xlen == 32
-	switchcase_csr_write(CSR_MCYCLEH, val)
-	switchcase_csr_write(CSR_MINSTRETH, val)
-	switchcase_csr_write(CSR_MHPMCOUNTER3H, val)
-	switchcase_csr_write_4(CSR_MHPMCOUNTER4H, val)
-	switchcase_csr_write_8(CSR_MHPMCOUNTER8H, val)
-	switchcase_csr_write_16(CSR_MHPMCOUNTER16H, val)
-	switchcase_csr_write(CSR_MHPMEVENT3H, val)
-	switchcase_csr_write_4(CSR_MHPMEVENT4H, val)
-	switchcase_csr_write_8(CSR_MHPMEVENT8H, val)
-	switchcase_csr_write_16(CSR_MHPMEVENT16H, val)
-#endif
-	switchcase_csr_write(CSR_MCOUNTINHIBIT, val)
-	switchcase_csr_write(CSR_MHPMEVENT3, val)
-	switchcase_csr_write_4(CSR_MHPMEVENT4, val)
-	switchcase_csr_write_8(CSR_MHPMEVENT8, val)
-	switchcase_csr_write_16(CSR_MHPMEVENT16, val)
-
-	default:
-		sbi_panic("%s: Unknown CSR %#x", __func__, csr_num);
-		break;
-	};
-
-#undef switchcase_csr_write_64
-#undef switchcase_csr_write_32
-#undef switchcase_csr_write_16
-#undef switchcase_csr_write_8
-#undef switchcase_csr_write_4
-#undef switchcase_csr_write_2
-#undef switchcase_csr_write
+	uint32_t opcode_buff[2];
+	typedef void (*write_f)(unsigned long val);
+	opcode_buff[0] = (csr_num << 20) | 0x00051073; /* csrw csr, a0 */
+	opcode_buff[1] = 0x00008067; /* ret */
+	((write_f)opcode_buff)(val);
 }
 
 static unsigned long ctz(unsigned long x)

[v2] lib: sbi: Improve csr read and write

Commit Message

Comments

Patch