Common csr_read_num() and csr_write_num() for RISC-V

PATCH Superseded Anup Patel 3 participants 6 messages Oct 14, 2025 – Nov 24, 2025

Subsystems (5)

ACPI RISC-V Arm PMU profiling and debugging RISC-V PMU drivers +1 more subsystems

lore

Share & tools

lore

Anup Patel

author

2025-10-14 13:21 UTC #0 lore raw

v3 cover Common csr_read_num() and csr_write_num() for RISC-V

Some of the RISC-V drivers (such as RISC-V PMU and ACPI CPPC) need to access CSR based on CSR number discovered from somewhere. Add common RISC-V csr_read_num() and csr_write_num() functions under arch/riscv for such drivers.

These patches can be found in the riscv_csr_read_num_v3 branch at:
https://github.com/avpatel/linux.git

Changes since v2:
- Rebase on Linux-6.18-rc1
- Added reviewed-by tags

Changes since v1:
- Make "out_err" mandatory for csr_read_num() and csr_write_num() in PATCH2 as suggested by Sunil and Drew. This also helps further simplify csr_read_num() and csr_write_num().

Anup Patel (1):
RISC-V: Add common csr_read_num() and csr_write_num() functions

 arch/riscv/include/asm/csr.h |   3 +
 arch/riscv/kernel/Makefile   |   1 +
 arch/riscv/kernel/csr.c      | 165 +++++++++++++++++++++++++++++++++++
 drivers/acpi/riscv/cppc.c    |  17 ++--
 drivers/perf/riscv_pmu.c     |  54 ++----------
 5 files changed, 184 insertions(+), 56 deletions(-)
 create mode 100644 arch/riscv/kernel/csr.c

Signature · 1 line

2.43.0

Anup Patel

author

2025-10-14 13:21 UTC #1 lore raw

PATCH v3 1/1 RISC-V: Add common csr_read_num() and csr_write_num() functions

In RISC-V, there is no CSR read/write instruction which takes CSR number via register so add common csr_read_num() and csr_write_num() functions which allow accessing certain CSRs by passing CSR number as parameter. These common functions will be first used by the ACPI CPPC driver and RISC-V PMU driver.

Signed-off-by Anup Patel <[email protected]>

Reviewed-by Sunil V L <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

Reviewed-by Atish Patra <[email protected]>

Reviewed-by Nutty Liu <[email protected]>

---
 arch/riscv/include/asm/csr.h |   3 +
 arch/riscv/kernel/Makefile   |   1 +
 arch/riscv/kernel/csr.c      | 165 +++++++++++++++++++++++++++++++++++
 drivers/acpi/riscv/cppc.c    |  17 ++--
 drivers/perf/riscv_pmu.c     |  54 ++----------
 5 files changed, 184 insertions(+), 56 deletions(-)
 create mode 100644 arch/riscv/kernel/csr.c

arch/riscv/include/asm/csr.h +3 -0

index 4a37a98398ad..46e1fcd82fd0 100644
@@ -575,6 +575,9 @@
 			      : "memory");			\
 })
 
+extern unsigned long csr_read_num(unsigned long csr_num, int *out_err);
+extern void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err);
+
 #endif /* __ASSEMBLER__ */
 
 #endif /* _ASM_RISCV_CSR_H */

arch/riscv/kernel/Makefile +1 -0

index f60fce69b725..a75e79192531 100644
@@ -50,6 +50,7 @@ obj-y	+= soc.o
 obj-$(CONFIG_RISCV_ALTERNATIVE) += alternative.o
 obj-y	+= cpu.o
 obj-y	+= cpufeature.o
+obj-y	+= csr.o
 obj-y	+= entry.o
 obj-y	+= irq.o
 obj-y	+= process.o

arch/riscv/kernel/csr.c new file +165 -0

165 lines not on this page · raw message

drivers/acpi/riscv/cppc.c +6 ~~-11~~

index 42c1a9052470..fe491937ed25 100644
@@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
 static void cppc_ffh_csr_read(void *read_data)
 {
 	struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
+	int err;
 
-	switch (data->reg) {
-	/* Support only TIME CSR for now */
-	case CSR_TIME:
-		data->ret.value = csr_read(CSR_TIME);
-		data->ret.error = 0;
-		break;
-	default:
-		data->ret.error = -EINVAL;
-		break;
-	}
+	data->ret.value = csr_read_num(data->reg, &err);
+	data->ret.error = err;
 }
 
 static void cppc_ffh_csr_write(void *write_data)
 {
 	struct sbi_cppc_data *data = (struct sbi_cppc_data *)write_data;
+	int err;
 
-	data->ret.error = -EINVAL;
+	csr_write_num(data->reg, data->val, &err);
+	data->ret.error = err;
 }
 
 /*

drivers/perf/riscv_pmu.c +9 ~~-45~~

73 lines not on this page · raw message

Signature · 1 line

2.43.0

Paul Walmsley

maintainer

2025-10-29 19:05 UTC #2 lore raw

Re: [PATCH v3 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions

Hi Anup,

Quoted 11 lines · Anup Patel

In RISC-V, there is no CSR read/write instruction which takes CSR number via register so add common csr_read_num() and csr_write_num() functions which allow accessing certain CSRs by passing CSR number as parameter. These common functions will be first used by the ACPI CPPC driver and RISC-V PMU driver.
Signed-off-by Anup Patel <[email protected]>
Reviewed-by Sunil V L <[email protected]>
Reviewed-by Andrew Jones <[email protected]>
Reviewed-by Atish Patra <[email protected]>
Reviewed-by Nutty Liu <[email protected]>

This patch also (silently) removes the CSR number filtering, e.g.

Quoted 10 lines

drivers/acpi/riscv/cppc.c +1 -0

index 42c1a9052470..fe491937ed25 100644
@@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
 static void cppc_ffh_csr_read(void *read_data)
 {
 	struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
+	int err;

drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 78

Quoted 10 lines

drivers/acpi/riscv/cppc.c +0 ~~-10~~

-	switch (data->reg) {
-	/* Support only TIME CSR for now */
-	case CSR_TIME:
-		data->ret.value = csr_read(CSR_TIME);
-		data->ret.error = 0;
-		break;
-	default:
-		data->ret.error = -EINVAL;
-		break;
-	}

... the above code, and:

drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 87

Quoted 9 lines

drivers/acpi/riscv/cppc.c +0 -4

 /*
  * Read the CSR of a corresponding counter.
  */
 unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
 {
-	if (csr < CSR_CYCLE || csr > CSR_HPMCOUNTER31H ||
-	   (csr > CSR_HPMCOUNTER31 && csr < CSR_CYCLEH)) {
-		pr_err("Invalid performance counter csr %lx\n", csr);
-		return -EINVAL;

... the above code.

I'm thinking that we probably want to keep the CSR number filtering code in; at least, I can't think of a good reason to remove it. Care to add it back in?

- Paul

Anup Patel

reviewer

2025-10-30 06:07 UTC #3 lore raw

Re: [PATCH v3 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions

Quoted 29 lines · Paul Walmsley

Hi Anup,
Quoted 11 lines · Anup Patel
In RISC-V, there is no CSR read/write instruction which takes CSR number via register so add common csr_read_num() and csr_write_num() functions which allow accessing certain CSRs by passing CSR number as parameter. These common functions will be first used by the ACPI CPPC driver and RISC-V PMU driver.
Signed-off-by Anup Patel <[email protected]>
Reviewed-by Sunil V L <[email protected]>
Reviewed-by Andrew Jones <[email protected]>
Reviewed-by Atish Patra <[email protected]>
Reviewed-by Nutty Liu <[email protected]>
This patch also (silently) removes the CSR number filtering, e.g.
Quoted 10 lines
drivers/acpi/riscv/cppc.c +1 -0
index 42c1a9052470..fe491937ed25 100644
@@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
 static void cppc_ffh_csr_read(void *read_data)
 {
      struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
+     int err;
 

Quoted 12 lines

drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 78

Quoted 10 lines

drivers/acpi/riscv/cppc.c +0 ~~-10~~

-     switch (data->reg) {
-     /* Support only TIME CSR for now */
-     case CSR_TIME:
-             data->ret.value = csr_read(CSR_TIME);
-             data->ret.error = 0;
-             break;
-     default:
-             data->ret.error = -EINVAL;
-             break;
-     }

... the above code, and:

The switch case is incomplete for cppc_ffh_csr_read(). Also, csr_read_num() already does appropriate filtering so the switch case over here is now redundant.

Quoted 11 lines

drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 87

Quoted 10 lines

drivers/acpi/riscv/cppc.c +0 -4

 /*
  * Read the CSR of a corresponding counter.
  */
 unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
 {
-     if (csr < CSR_CYCLE || csr > CSR_HPMCOUNTER31H ||
-        (csr > CSR_HPMCOUNTER31 && csr < CSR_CYCLEH)) {
-             pr_err("Invalid performance counter csr %lx\n", csr);
-             return -EINVAL;

Quoted 5 lines

... the above code.
I'm thinking that we probably want to keep the CSR number filtering code in; at least, I can't think of a good reason to remove it. Care to add it back in?

We can potentially have custom CSRs as hardware counters hence the CSR filtering over here is already incomplete. Plus, csr_read_num() already does the CSR filtering and returns failure for inappropriate CSR number.

Regards,
Anup

Paul Walmsley

maintainer

2025-11-22 01:14 UTC #4 lore raw

Re: [PATCH v3 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions

Quoted 51 lines · Anup Patel

Quoted 20 lines · Paul Walmsley
Quoted 5 lines · Anup Patel
In RISC-V, there is no CSR read/write instruction which takes CSR number via register so add common csr_read_num() and csr_write_num() functions which allow accessing certain CSRs by passing CSR number as parameter. These common functions will be first used by the ACPI CPPC driver and RISC-V PMU driver.
This patch also (silently) removes the CSR number filtering, e.g.
Quoted 10 lines
drivers/acpi/riscv/cppc.c +1 -0
index 42c1a9052470..fe491937ed25 100644
@@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
 static void cppc_ffh_csr_read(void *read_data)
 {
      struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
+     int err;
 
Quoted 12 lines
drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 78
Quoted 10 lines
drivers/acpi/riscv/cppc.c +0 ~~-10~~
-     switch (data->reg) {
-     /* Support only TIME CSR for now */
-     case CSR_TIME:
-             data->ret.value = csr_read(CSR_TIME);
-             data->ret.error = 0;
-             break;
-     default:
-             data->ret.error = -EINVAL;
-             break;
-     }
... the above code, and:
The switch case is incomplete for cppc_ffh_csr_read(). Also, csr_read_num() already does appropriate filtering so the switch case over here is now redundant.
Quoted 13 lines
drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 87
Quoted 10 lines
drivers/acpi/riscv/cppc.c +0 -4
 /*
  * Read the CSR of a corresponding counter.
  */
 unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
 {
-     if (csr < CSR_CYCLE || csr > CSR_HPMCOUNTER31H ||
-        (csr > CSR_HPMCOUNTER31 && csr < CSR_CYCLEH)) {
-             pr_err("Invalid performance counter csr %lx\n", csr);
-             return -EINVAL;
... the above code.

Quoted 8 lines

Quoted 3 lines
I'm thinking that we probably want to keep the CSR number filtering code in; at least, I can't think of a good reason to remove it. Care to add it back in?
We can potentially have custom CSRs as hardware counters hence the CSR filtering over here is already incomplete. Plus, csr_read_num() already does the CSR filtering and returns failure for inappropriate CSR number.

OK. To me, the most notable changes in this patch are the changes in the filters for both reads and writes for the two different call sites, ACPI FFH and PMU. It would be good to document these filter changes directly in the patch description, along with the rationale.

I'm also not yet completely convinced that we should allow both of these call sites to read and write any custom CSR. But maybe the updated patch description might be convincing...

- Paul

Anup Patel

author

2025-11-24 04:49 UTC #5 lore raw

Re: [PATCH v3 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions

Quoted 63 lines · Paul Walmsley

Quoted 51 lines · Anup Patel
Quoted 20 lines · Paul Walmsley
Quoted 5 lines · Anup Patel
In RISC-V, there is no CSR read/write instruction which takes CSR number via register so add common csr_read_num() and csr_write_num() functions which allow accessing certain CSRs by passing CSR number as parameter. These common functions will be first used by the ACPI CPPC driver and RISC-V PMU driver.
This patch also (silently) removes the CSR number filtering, e.g.
Quoted 10 lines
drivers/acpi/riscv/cppc.c +1 -0
index 42c1a9052470..fe491937ed25 100644
@@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
 static void cppc_ffh_csr_read(void *read_data)
 {
      struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
+     int err;
 
Quoted 12 lines
drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 78
Quoted 10 lines
drivers/acpi/riscv/cppc.c +0 ~~-10~~
-     switch (data->reg) {
-     /* Support only TIME CSR for now */
-     case CSR_TIME:
-             data->ret.value = csr_read(CSR_TIME);
-             data->ret.error = 0;
-             break;
-     default:
-             data->ret.error = -EINVAL;
-             break;
-     }
... the above code, and:
The switch case is incomplete for cppc_ffh_csr_read(). Also, csr_read_num() already does appropriate filtering so the switch case over here is now redundant.
Quoted 13 lines
drivers/acpi/riscv/cppc.c static void sbi_cppc_write(void *write_data) · line 87
Quoted 10 lines
drivers/acpi/riscv/cppc.c +0 -4
 /*
  * Read the CSR of a corresponding counter.
  */
 unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
 {
-     if (csr < CSR_CYCLE || csr > CSR_HPMCOUNTER31H ||
-        (csr > CSR_HPMCOUNTER31 && csr < CSR_CYCLEH)) {
-             pr_err("Invalid performance counter csr %lx\n", csr);
-             return -EINVAL;
... the above code.
Quoted 8 lines
Quoted 3 lines
I'm thinking that we probably want to keep the CSR number filtering code in; at least, I can't think of a good reason to remove it. Care to add it back in?
We can potentially have custom CSRs as hardware counters hence the CSR filtering over here is already incomplete. Plus, csr_read_num() already does the CSR filtering and returns failure for inappropriate CSR number.

Quoted 8 lines

OK. To me, the most notable changes in this patch are the changes in the filters for both reads and writes for the two different call sites, ACPI FFH and PMU. It would be good to document these filter changes directly in the patch description, along with the rationale.
I'm also not yet completely convinced that we should allow both of these call sites to read and write any custom CSR. But maybe the updated patch description might be convincing...

The RISC-V ACPI FFH specification allows a 12-bit CSR number in _CPC object encoding so any CSR can be mapped as a CPPC register (including custom CSRs).

Similarly, the RISC-V SBI PMU specification return 12-bit CSR number in sbi_pmu_counter_get_info() so a platform can use custom CSRs for additional HW counters.

I will update the commit description accordingly.

Regards,
Anup