| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEEQ3wm-TGcRFjmb7cw5K-M13CicwgJSLZrgY1KMZA5SgUjziw@mail.gmail.com>
Date: Tue, 23 Sep 2025 14:12:03 +0800
From: yunhui cui <cuiyunhui@...edance.com>
To: Atish Patra <atishp@...osinc.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Anup Patel <anup@...infault.org>, Atish Patra <atishp@...shpatra.org>,
Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, weilin.wang@...el.com,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
Conor Dooley <conor@...nel.org>, devicetree@...r.kernel.org, kvm@...r.kernel.org,
kvm-riscv@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org
Subject: Re: [External] [PATCH v5 12/21] RISC-V: perf: Modify the counter
discovery mechanism
Hi Atish,
On Fri, Mar 28, 2025 at 3:42 AM Atish Patra <atishp@...osinc.com> wrote:
>
> If both counter delegation and SBI PMU is present, the counter
> delegation will be used for hardware pmu counters while the SBI PMU
> will be used for firmware counters. Thus, the driver has to probe
> the counters info via SBI PMU to distinguish the firmware counters.
>
> The hybrid scheme also requires improvements of the informational
> logging messages to indicate the user about underlying interface
> used for each use case.
>
> Signed-off-by: Atish Patra <atishp@...osinc.com>
> ---
> drivers/perf/riscv_pmu_dev.c | 130 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 96 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/perf/riscv_pmu_dev.c b/drivers/perf/riscv_pmu_dev.c
> index 6cebbc16bfe4..c0397bd68b91 100644
> --- a/drivers/perf/riscv_pmu_dev.c
> +++ b/drivers/perf/riscv_pmu_dev.c
> @@ -66,6 +66,20 @@ static bool sbi_v2_available;
> static DEFINE_STATIC_KEY_FALSE(sbi_pmu_snapshot_available);
> #define sbi_pmu_snapshot_available() \
> static_branch_unlikely(&sbi_pmu_snapshot_available)
> +static DEFINE_STATIC_KEY_FALSE(riscv_pmu_sbi_available);
> +static DEFINE_STATIC_KEY_FALSE(riscv_pmu_cdeleg_available);
> +
> +/* Avoid unnecessary code patching in the one time booting path*/
> +#define riscv_pmu_cdeleg_available_boot() \
> + static_key_enabled(&riscv_pmu_cdeleg_available)
> +#define riscv_pmu_sbi_available_boot() \
> + static_key_enabled(&riscv_pmu_sbi_available)
> +
> +/* Perform a runtime code patching with static key */
> +#define riscv_pmu_cdeleg_available() \
> + static_branch_unlikely(&riscv_pmu_cdeleg_available)
> +#define riscv_pmu_sbi_available() \
> + static_branch_likely(&riscv_pmu_sbi_available)
>
> static struct attribute *riscv_arch_formats_attr[] = {
> &format_attr_event.attr,
> @@ -88,7 +102,8 @@ static int sysctl_perf_user_access __read_mostly = SYSCTL_USER_ACCESS;
>
> /*
> * This structure is SBI specific but counter delegation also require counter
> - * width, csr mapping. Reuse it for now.
> + * width, csr mapping. Reuse it for now we can have firmware counters for
> + * platfroms with counter delegation support.
> * RISC-V doesn't have heterogeneous harts yet. This need to be part of
> * per_cpu in case of harts with different pmu counters
> */
> @@ -100,6 +115,8 @@ static unsigned int riscv_pmu_irq;
>
> /* Cache the available counters in a bitmask */
> static unsigned long cmask;
> +/* Cache the available firmware counters in another bitmask */
> +static unsigned long firmware_cmask;
>
> struct sbi_pmu_event_data {
> union {
> @@ -780,34 +797,38 @@ static int rvpmu_sbi_find_num_ctrs(void)
> return sbi_err_map_linux_errno(ret.error);
> }
>
> -static int rvpmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
> +static u32 rvpmu_deleg_find_ctrs(void)
> +{
> + /* TODO */
> + return 0;
> +}
> +
> +static int rvpmu_sbi_get_ctrinfo(u32 nsbi_ctr, u32 *num_fw_ctr, u32 *num_hw_ctr)
> {
> struct sbiret ret;
> - int i, num_hw_ctr = 0, num_fw_ctr = 0;
> + int i;
> union sbi_pmu_ctr_info cinfo;
>
> - pmu_ctr_list = kcalloc(nctr, sizeof(*pmu_ctr_list), GFP_KERNEL);
> - if (!pmu_ctr_list)
> - return -ENOMEM;
> -
> - for (i = 0; i < nctr; i++) {
> + for (i = 0; i < nsbi_ctr; i++) {
> ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_GET_INFO, i, 0, 0, 0, 0, 0);
> if (ret.error)
> /* The logical counter ids are not expected to be contiguous */
> continue;
>
> - *mask |= BIT(i);
> -
> cinfo.value = ret.value;
> - if (cinfo.type == SBI_PMU_CTR_TYPE_FW)
> - num_fw_ctr++;
> - else
> - num_hw_ctr++;
> - pmu_ctr_list[i].value = cinfo.value;
> + if (cinfo.type == SBI_PMU_CTR_TYPE_FW) {
> + /* Track firmware counters in a different mask */
> + firmware_cmask |= BIT(i);
> + pmu_ctr_list[i].value = cinfo.value;
> + *num_fw_ctr = *num_fw_ctr + 1;
> + } else if (cinfo.type == SBI_PMU_CTR_TYPE_HW &&
> + !riscv_pmu_cdeleg_available_boot()) {
> + *num_hw_ctr = *num_hw_ctr + 1;
> + cmask |= BIT(i);
> + pmu_ctr_list[i].value = cinfo.value;
> + }
> }
>
> - pr_info("%d firmware and %d hardware counters\n", num_fw_ctr, num_hw_ctr);
> -
> return 0;
> }
>
> @@ -1069,16 +1090,41 @@ static void rvpmu_ctr_stop(struct perf_event *event, unsigned long flag)
> /* TODO: Counter delegation implementation */
> }
>
> -static int rvpmu_find_num_ctrs(void)
> +static int rvpmu_find_ctrs(void)
> {
> - return rvpmu_sbi_find_num_ctrs();
> - /* TODO: Counter delegation implementation */
> -}
> + u32 num_sbi_counters = 0, num_deleg_counters = 0;
> + u32 num_hw_ctr = 0, num_fw_ctr = 0, num_ctr = 0;
> + /*
> + * We don't know how many firmware counters are available. Just allocate
> + * for maximum counters the driver can support. The default is 64 anyways.
> + */
> + pmu_ctr_list = kcalloc(RISCV_MAX_COUNTERS, sizeof(*pmu_ctr_list),
> + GFP_KERNEL);
> + if (!pmu_ctr_list)
> + return -ENOMEM;
>
> -static int rvpmu_get_ctrinfo(int nctr, unsigned long *mask)
> -{
> - return rvpmu_sbi_get_ctrinfo(nctr, mask);
> - /* TODO: Counter delegation implementation */
> + if (riscv_pmu_cdeleg_available_boot())
> + num_deleg_counters = rvpmu_deleg_find_ctrs();
> +
> + /* This is required for firmware counters even if the above is true */
> + if (riscv_pmu_sbi_available_boot()) {
> + num_sbi_counters = rvpmu_sbi_find_num_ctrs();
> + /* cache all the information about counters now */
> + rvpmu_sbi_get_ctrinfo(num_sbi_counters, &num_hw_ctr, &num_fw_ctr);
> + }
> +
> + if (num_sbi_counters > RISCV_MAX_COUNTERS || num_deleg_counters > RISCV_MAX_COUNTERS)
> + return -ENOSPC;
> +
> + if (riscv_pmu_cdeleg_available_boot()) {
> + pr_info("%u firmware and %u hardware counters\n", num_fw_ctr, num_deleg_counters);
> + num_ctr = num_fw_ctr + num_deleg_counters;
> + } else {
> + pr_info("%u firmware and %u hardware counters\n", num_fw_ctr, num_hw_ctr);
> + num_ctr = num_sbi_counters;
> + }
> +
> + return num_ctr;
> }
>
> static int rvpmu_event_map(struct perf_event *event, u64 *econfig)
> @@ -1379,12 +1425,21 @@ static int rvpmu_device_probe(struct platform_device *pdev)
> int ret = -ENODEV;
> int num_counters;
>
> - pr_info("SBI PMU extension is available\n");
> + if (riscv_pmu_cdeleg_available_boot()) {
> + pr_info("hpmcounters will use the counter delegation ISA extension\n");
> + if (riscv_pmu_sbi_available_boot())
> + pr_info("Firmware counters will use SBI PMU extension\n");
> + else
> + pr_info("Firmware counters will not be available as SBI PMU extension is not present\n");
> + } else if (riscv_pmu_sbi_available_boot()) {
> + pr_info("Both hpmcounters and firmware counters will use SBI PMU extension\n");
> + }
> +
> pmu = riscv_pmu_alloc();
> if (!pmu)
> return -ENOMEM;
>
> - num_counters = rvpmu_find_num_ctrs();
> + num_counters = rvpmu_find_ctrs();
> if (num_counters < 0) {
> pr_err("SBI PMU extension doesn't provide any counters\n");
> goto out_free;
> @@ -1396,9 +1451,6 @@ static int rvpmu_device_probe(struct platform_device *pdev)
> pr_info("SBI returned more than maximum number of counters. Limiting the number of counters to %d\n", num_counters);
> }
>
> - /* cache all the information about counters now */
> - if (rvpmu_get_ctrinfo(num_counters, &cmask))
> - goto out_free;
>
> ret = rvpmu_setup_irqs(pmu, pdev);
> if (ret < 0) {
> @@ -1488,13 +1540,23 @@ static int __init rvpmu_devinit(void)
> int ret;
> struct platform_device *pdev;
>
> - if (sbi_spec_version < sbi_mk_version(0, 3) ||
> - !sbi_probe_extension(SBI_EXT_PMU)) {
> - return 0;
> - }
> + if (sbi_spec_version >= sbi_mk_version(0, 3) &&
> + sbi_probe_extension(SBI_EXT_PMU))
> + static_branch_enable(&riscv_pmu_sbi_available);
>
> if (sbi_spec_version >= sbi_mk_version(2, 0))
> sbi_v2_available = true;
> + /*
> + * We need all three extensions to be present to access the counters
> + * in S-mode via Supervisor Counter delegation.
> + */
> + if (riscv_isa_extension_available(NULL, SSCCFG) &&
> + riscv_isa_extension_available(NULL, SMCDELEG) &&
Is there no need to check SMCDELEG (Machine-level) in the kernel, and
can it be done directly via SSCCFG or sbi_probe_extension?
The #define RISCV_ISA_EXT_SMCDELEG 98 also doesn't need to be defined
in the kernel.
> + riscv_isa_extension_available(NULL, SSCSRIND))
> + static_branch_enable(&riscv_pmu_cdeleg_available);
> +
> + if (!(riscv_pmu_sbi_available_boot() || riscv_pmu_cdeleg_available_boot()))
> + return 0;
>
> ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_RISCV_STARTING,
> "perf/riscv/pmu:starting",
>
> --
> 2.43.0
>
>
Thanks,
Yunhui
Powered by blists - more mailing lists