* [PATCH v5 0/1] Common csr_read_num() and csr_write_num() for RISC-V @ 2026-02-04 15:53 Anup Patel 2026-02-04 15:53 ` [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions Anup Patel 0 siblings, 1 reply; 6+ messages in thread From: Anup Patel @ 2026-02-04 15:53 UTC (permalink / raw) To: Sunil V L, Rafael J . Wysocki Cc: Palmer Dabbelt, Paul Walmsley, Alexandre Ghiti, Len Brown, Atish Patra, Andrew Jones, Anup Patel, Will Deacon, Mark Rutland, linux-acpi, linux-riscv, linux-kernel, Anup Patel 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_v5 branch at: https://github.com/avpatel/linux.git Changes since v4: - Rebased on Linux-6.19-rc6 Changes since v3: - Rebased on Linux-6.18-rc7 - Updated commit decription of PATCH1 to reflect the fact that we are removing sanity checks on CSR number which are already taken care by csr_read_num() and csr_write_num(). Changes since v2: - Rebased 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 -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions 2026-02-04 15:53 [PATCH v5 0/1] Common csr_read_num() and csr_write_num() for RISC-V Anup Patel @ 2026-02-04 15:53 ` Anup Patel 2026-04-03 8:25 ` Paul Walmsley 0 siblings, 1 reply; 6+ messages in thread From: Anup Patel @ 2026-02-04 15:53 UTC (permalink / raw) To: Sunil V L, Rafael J . Wysocki Cc: Palmer Dabbelt, Paul Walmsley, Alexandre Ghiti, Len Brown, Atish Patra, Andrew Jones, Anup Patel, Will Deacon, Mark Rutland, linux-acpi, linux-riscv, linux-kernel, Anup Patel, Sunil V L, Andrew Jones, Atish Patra, Nutty Liu From: Anup Patel <apatel@ventanamicro.com> 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. Also, the RISC-V ACPI FFH specification allows arbitrary CSR number as CPPC register and the RISC-V SBI specification allows arbitrary CSR number as PMU hardware counter. This means ACPI CPPC driver and RISC-V PMU driver no longer need to do sanity checks on CSR number which are now done by the common csr_read_num() and csr_write_num() functions. Signed-off-by: Anup Patel <apatel@ventanamicro.com> Reviewed-by: Sunil V L <sunilvl@ventanamicro.com> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Reviewed-by: Atish Patra <atishp@rivosinc.com> Reviewed-by: Nutty Liu <nutty.liu@hotmail.com> --- 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 diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index 4a37a98398ad..543a3364f11d 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -575,6 +575,9 @@ : "memory"); \ }) +unsigned long csr_read_num(unsigned long csr_num, int *out_err); +void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err); + #endif /* __ASSEMBLER__ */ #endif /* _ASM_RISCV_CSR_H */ diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index a01f6439d62b..0b9008a6f4e0 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -51,6 +51,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 diff --git a/arch/riscv/kernel/csr.c b/arch/riscv/kernel/csr.c new file mode 100644 index 000000000000..e96b129c1a99 --- /dev/null +++ b/arch/riscv/kernel/csr.c @@ -0,0 +1,165 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2025 Ventana Micro Systems Inc. + */ + +#include <linux/err.h> +#include <linux/export.h> +#include <linux/types.h> +#include <asm/csr.h> + +#define CSR_CUSTOM0_U_RW_BASE 0x800 +#define CSR_CUSTOM0_U_RW_COUNT 0x100 + +#define CSR_CUSTOM1_U_RO_BASE 0xCC0 +#define CSR_CUSTOM1_U_RO_COUNT 0x040 + +#define CSR_CUSTOM2_S_RW_BASE 0x5C0 +#define CSR_CUSTOM2_S_RW_COUNT 0x040 + +#define CSR_CUSTOM3_S_RW_BASE 0x9C0 +#define CSR_CUSTOM3_S_RW_COUNT 0x040 + +#define CSR_CUSTOM4_S_RO_BASE 0xDC0 +#define CSR_CUSTOM4_S_RO_COUNT 0x040 + +#define CSR_CUSTOM5_HS_RW_BASE 0x6C0 +#define CSR_CUSTOM5_HS_RW_COUNT 0x040 + +#define CSR_CUSTOM6_HS_RW_BASE 0xAC0 +#define CSR_CUSTOM6_HS_RW_COUNT 0x040 + +#define CSR_CUSTOM7_HS_RO_BASE 0xEC0 +#define CSR_CUSTOM7_HS_RO_COUNT 0x040 + +#define CSR_CUSTOM8_M_RW_BASE 0x7C0 +#define CSR_CUSTOM8_M_RW_COUNT 0x040 + +#define CSR_CUSTOM9_M_RW_BASE 0xBC0 +#define CSR_CUSTOM9_M_RW_COUNT 0x040 + +#define CSR_CUSTOM10_M_RO_BASE 0xFC0 +#define CSR_CUSTOM10_M_RO_COUNT 0x040 + +unsigned long csr_read_num(unsigned long csr_num, int *out_err) +{ +#define switchcase_csr_read(__csr_num) \ + case (__csr_num): \ + return csr_read(__csr_num) +#define switchcase_csr_read_2(__csr_num) \ + switchcase_csr_read(__csr_num + 0); \ + switchcase_csr_read(__csr_num + 1) +#define switchcase_csr_read_4(__csr_num) \ + switchcase_csr_read_2(__csr_num + 0); \ + switchcase_csr_read_2(__csr_num + 2) +#define switchcase_csr_read_8(__csr_num) \ + switchcase_csr_read_4(__csr_num + 0); \ + switchcase_csr_read_4(__csr_num + 4) +#define switchcase_csr_read_16(__csr_num) \ + switchcase_csr_read_8(__csr_num + 0); \ + switchcase_csr_read_8(__csr_num + 8) +#define switchcase_csr_read_32(__csr_num) \ + switchcase_csr_read_16(__csr_num + 0); \ + switchcase_csr_read_16(__csr_num + 16) +#define switchcase_csr_read_64(__csr_num) \ + switchcase_csr_read_32(__csr_num + 0); \ + switchcase_csr_read_32(__csr_num + 32) +#define switchcase_csr_read_128(__csr_num) \ + switchcase_csr_read_64(__csr_num + 0); \ + switchcase_csr_read_64(__csr_num + 64) +#define switchcase_csr_read_256(__csr_num) \ + switchcase_csr_read_128(__csr_num + 0); \ + switchcase_csr_read_128(__csr_num + 128) + + *out_err = 0; + switch (csr_num) { + switchcase_csr_read_32(CSR_CYCLE); + switchcase_csr_read_32(CSR_CYCLEH); + switchcase_csr_read_256(CSR_CUSTOM0_U_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM1_U_RO_BASE); + switchcase_csr_read_64(CSR_CUSTOM2_S_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM3_S_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM4_S_RO_BASE); + switchcase_csr_read_64(CSR_CUSTOM5_HS_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM6_HS_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM7_HS_RO_BASE); +#ifdef CONFIG_RISCV_M_MODE + switchcase_csr_read_64(CSR_CUSTOM8_M_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM9_M_RW_BASE); + switchcase_csr_read_64(CSR_CUSTOM10_M_RO_BASE); +#endif + default: + *out_err = -EINVAL; + break; + } + + return 0; +#undef switchcase_csr_read_256 +#undef switchcase_csr_read_128 +#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 +} +EXPORT_SYMBOL_GPL(csr_read_num); + +void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err) +{ +#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) +#define switchcase_csr_write_128(__csr_num, __val) \ + switchcase_csr_write_64(__csr_num + 0, __val); \ + switchcase_csr_write_64(__csr_num + 64, __val) +#define switchcase_csr_write_256(__csr_num, __val) \ + switchcase_csr_write_128(__csr_num + 0, __val); \ + switchcase_csr_write_128(__csr_num + 128, __val) + + *out_err = 0; + switch (csr_num) { + switchcase_csr_write_256(CSR_CUSTOM0_U_RW_BASE, val); + switchcase_csr_write_64(CSR_CUSTOM2_S_RW_BASE, val); + switchcase_csr_write_64(CSR_CUSTOM3_S_RW_BASE, val); + switchcase_csr_write_64(CSR_CUSTOM5_HS_RW_BASE, val); + switchcase_csr_write_64(CSR_CUSTOM6_HS_RW_BASE, val); +#ifdef CONFIG_RISCV_M_MODE + switchcase_csr_write_64(CSR_CUSTOM8_M_RW_BASE, val); + switchcase_csr_write_64(CSR_CUSTOM9_M_RW_BASE, val); +#endif + default: + *out_err = -EINVAL; + break; + } +#undef switchcase_csr_write_256 +#undef switchcase_csr_write_128 +#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 +} +EXPORT_SYMBOL_GPL(csr_write_num); diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c index 42c1a9052470..fe491937ed25 100644 --- a/drivers/acpi/riscv/cppc.c +++ b/drivers/acpi/riscv/cppc.c @@ -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; } /* diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c index 7644147d50b4..b41f353ba964 100644 --- a/drivers/perf/riscv_pmu.c +++ b/drivers/perf/riscv_pmu.c @@ -16,6 +16,7 @@ #include <linux/smp.h> #include <linux/sched_clock.h> +#include <asm/csr.h> #include <asm/sbi.h> static bool riscv_perf_user_access(struct perf_event *event) @@ -88,58 +89,21 @@ void arch_perf_update_userpage(struct perf_event *event, userpg->cap_user_time_short = 1; } -static 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)} - - unsigned long ret = 0; - - switch (csr_num) { - switchcase_csr_read_32(CSR_CYCLE, ret) - switchcase_csr_read_32(CSR_CYCLEH, ret) - default : - break; - } - - return ret; -#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 -} - /* * 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; + unsigned long val; + int rc; + + val = csr_read_num(csr, &rc); + if (rc) { + pr_err("Failed to read performance counter csr %lx (error %d)\n", csr, rc); + return rc; } - return csr_read_num(csr); + return val; } u64 riscv_pmu_ctr_get_width_mask(struct perf_event *event) -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions 2026-02-04 15:53 ` [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions Anup Patel @ 2026-04-03 8:25 ` Paul Walmsley 2026-04-03 8:43 ` Anup Patel 0 siblings, 1 reply; 6+ messages in thread From: Paul Walmsley @ 2026-04-03 8:25 UTC (permalink / raw) To: Anup Patel Cc: Sunil V L, Rafael J . Wysocki, Mark Rutland, Anup Patel, Alexandre Ghiti, Atish Patra, Anup Patel, Atish Patra, linux-kernel, Andrew Jones, linux-acpi, Palmer Dabbelt, Paul Walmsley, Nutty Liu, linux-riscv, Andrew Jones, Will Deacon, Len Brown Hi Anup, On Wed, 4 Feb 2026, Anup Patel wrote: > From: Anup Patel <apatel@ventanamicro.com> > > 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. > > Also, the RISC-V ACPI FFH specification allows arbitrary CSR number > as CPPC register and the RISC-V SBI specification allows arbitrary > CSR number as PMU hardware counter. This means ACPI CPPC driver and > RISC-V PMU driver no longer need to do sanity checks on CSR number > which are now done by the common csr_read_num() and csr_write_num() > functions. I recall that when we discussed this patch on a call a few months ago, it was brought up that it seemed useful to restrict which CSRs could be read or written at runtime, on a per-vendor basis. That way, the FFH interface couldn't be used to read or write custom CSRs that had nothing to do with CPPC or PMU functions. Are you still planning to implement that change? - Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions 2026-04-03 8:25 ` Paul Walmsley @ 2026-04-03 8:43 ` Anup Patel 2026-04-03 15:56 ` Paul Walmsley 0 siblings, 1 reply; 6+ messages in thread From: Anup Patel @ 2026-04-03 8:43 UTC (permalink / raw) To: Paul Walmsley Cc: Anup Patel, Sunil V L, Rafael J . Wysocki, Mark Rutland, Anup Patel, Alexandre Ghiti, Atish Patra, Atish Patra, linux-kernel, Andrew Jones, linux-acpi, Palmer Dabbelt, Paul Walmsley, Nutty Liu, linux-riscv, Andrew Jones, Will Deacon, Len Brown On Fri, Apr 3, 2026 at 1:55 PM Paul Walmsley <pjw@kernel.org> wrote: > > Hi Anup, > > On Wed, 4 Feb 2026, Anup Patel wrote: > > > From: Anup Patel <apatel@ventanamicro.com> > > > > 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. > > > > Also, the RISC-V ACPI FFH specification allows arbitrary CSR number > > as CPPC register and the RISC-V SBI specification allows arbitrary > > CSR number as PMU hardware counter. This means ACPI CPPC driver and > > RISC-V PMU driver no longer need to do sanity checks on CSR number > > which are now done by the common csr_read_num() and csr_write_num() > > functions. > > I recall that when we discussed this patch on a call a few months ago, it > was brought up that it seemed useful to restrict which CSRs could be read > or written at runtime, on a per-vendor basis. That way, the FFH interface > couldn't be used to read or write custom CSRs that had nothing to do with > CPPC or PMU functions. > > Are you still planning to implement that change? > The current csr_read_num() and csr_write_num() only allows HPM counters and custom CSR space (as defined by the Priv spec) to be accessible via ACPI FFH. The ACPI FFH interface is very flexible and allows vendors to specify the CSR number in the ACPI table itself so we don't need to compile kernel on per-vendor basis. In other words, the current approach allows same Linux kernel image for multiple vendor even if they use custom CSRs in their ACPI FFH interface. Regards, Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions 2026-04-03 8:43 ` Anup Patel @ 2026-04-03 15:56 ` Paul Walmsley 2026-04-03 16:47 ` Anup Patel 0 siblings, 1 reply; 6+ messages in thread From: Paul Walmsley @ 2026-04-03 15:56 UTC (permalink / raw) To: Anup Patel Cc: Paul Walmsley, Anup Patel, Sunil V L, Rafael J . Wysocki, Mark Rutland, Anup Patel, Alexandre Ghiti, Atish Patra, Atish Patra, linux-kernel, Andrew Jones, linux-acpi, Palmer Dabbelt, Paul Walmsley, Nutty Liu, linux-riscv, Andrew Jones, Will Deacon, Len Brown [-- Attachment #1: Type: text/plain, Size: 2276 bytes --] On Fri, 3 Apr 2026, Anup Patel wrote: > On Fri, Apr 3, 2026 at 1:55 PM Paul Walmsley <pjw@kernel.org> wrote: > > > > On Wed, 4 Feb 2026, Anup Patel wrote: > > > > > From: Anup Patel <apatel@ventanamicro.com> > > > > > > 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. > > > > > > Also, the RISC-V ACPI FFH specification allows arbitrary CSR number > > > as CPPC register and the RISC-V SBI specification allows arbitrary > > > CSR number as PMU hardware counter. This means ACPI CPPC driver and > > > RISC-V PMU driver no longer need to do sanity checks on CSR number > > > which are now done by the common csr_read_num() and csr_write_num() > > > functions. > > > > I recall that when we discussed this patch on a call a few months ago, it > > was brought up that it seemed useful to restrict which CSRs could be read > > or written at runtime, on a per-vendor basis. That way, the FFH interface > > couldn't be used to read or write custom CSRs that had nothing to do with > > CPPC or PMU functions. > > > > Are you still planning to implement that change? > > The current csr_read_num() and csr_write_num() only allows > HPM counters and custom CSR space (as defined by the Priv > spec) to be accessible via ACPI FFH. The ACPI FFH interface > is very flexible and allows vendors to specify the CSR number > in the ACPI table itself so we don't need to compile kernel on > per-vendor basis. > > In other words, the current approach allows same Linux kernel > image for multiple vendor even if they use custom CSRs in their > ACPI FFH interface. I understand that part. The part that I don't like is that with this patch, the FFH interface could also be used to read and write custom CSRs that have nothing to do with CPPC or PMU functions. That doesn't seem like a good idea. What I had in mind was to add a way to restrict the custom CSRs that the kernel would allow FFH to use, via a mechanism other than the ACPI table - e.g., sysfs, or the kernel command line, etc. - Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions 2026-04-03 15:56 ` Paul Walmsley @ 2026-04-03 16:47 ` Anup Patel 0 siblings, 0 replies; 6+ messages in thread From: Anup Patel @ 2026-04-03 16:47 UTC (permalink / raw) To: Paul Walmsley Cc: Anup Patel, Sunil V L, Rafael J . Wysocki, Mark Rutland, Anup Patel, Alexandre Ghiti, Atish Patra, Atish Patra, linux-kernel, Andrew Jones, linux-acpi, Palmer Dabbelt, Paul Walmsley, Nutty Liu, linux-riscv, Andrew Jones, Will Deacon, Len Brown On Fri, Apr 3, 2026 at 9:26 PM Paul Walmsley <pjw@kernel.org> wrote: > > On Fri, 3 Apr 2026, Anup Patel wrote: > > > On Fri, Apr 3, 2026 at 1:55 PM Paul Walmsley <pjw@kernel.org> wrote: > > > > > > On Wed, 4 Feb 2026, Anup Patel wrote: > > > > > > > From: Anup Patel <apatel@ventanamicro.com> > > > > > > > > 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. > > > > > > > > Also, the RISC-V ACPI FFH specification allows arbitrary CSR number > > > > as CPPC register and the RISC-V SBI specification allows arbitrary > > > > CSR number as PMU hardware counter. This means ACPI CPPC driver and > > > > RISC-V PMU driver no longer need to do sanity checks on CSR number > > > > which are now done by the common csr_read_num() and csr_write_num() > > > > functions. > > > > > > I recall that when we discussed this patch on a call a few months ago, it > > > was brought up that it seemed useful to restrict which CSRs could be read > > > or written at runtime, on a per-vendor basis. That way, the FFH interface > > > couldn't be used to read or write custom CSRs that had nothing to do with > > > CPPC or PMU functions. > > > > > > Are you still planning to implement that change? > > > > The current csr_read_num() and csr_write_num() only allows > > HPM counters and custom CSR space (as defined by the Priv > > spec) to be accessible via ACPI FFH. The ACPI FFH interface > > is very flexible and allows vendors to specify the CSR number > > in the ACPI table itself so we don't need to compile kernel on > > per-vendor basis. > > > > In other words, the current approach allows same Linux kernel > > image for multiple vendor even if they use custom CSRs in their > > ACPI FFH interface. > > I understand that part. The part that I don't like is that with this > patch, the FFH interface could also be used to read and write custom CSRs > that have nothing to do with CPPC or PMU functions. That doesn't seem > like a good idea. What I had in mind was to add a way to restrict the > custom CSRs that the kernel would allow FFH to use, via a mechanism other > than the ACPI table - e.g., sysfs, or the kernel command line, etc. > We can't use FFH interface to read and write custom CSRs from any part of the kernel. This is because ACPI FFH encoding is specific to a subsystem. Currently, RISC-V ACPI FFH encoding is only defined for LPI states and CPPC registers. Out of these, only CPPC FFH encoding allows specifying custom CSRs. Regards, Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-03 16:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-04 15:53 [PATCH v5 0/1] Common csr_read_num() and csr_write_num() for RISC-V Anup Patel 2026-02-04 15:53 ` [PATCH v5 1/1] RISC-V: Add common csr_read_num() and csr_write_num() functions Anup Patel 2026-04-03 8:25 ` Paul Walmsley 2026-04-03 8:43 ` Anup Patel 2026-04-03 15:56 ` Paul Walmsley 2026-04-03 16:47 ` Anup Patel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox