Re: [PATCH v6 17/21] RISC-V: perf: Add Qemu virt machine events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch incorrectly overloads the generic 'unimplemented' or 'non-commercial' CPU IDs (`mvendorid=0`, `marchid=0`, `mimpid=0`) to uniquely identify the QEMU virt machine, applying QEMU-specific PMU events globally to any minimal or academic RISC-V core.
--

commit 61b476b524390d76a4db3dcbab4b57114ff0ebc8
Author: Atish Patra <atishp@rivosinc.com>

RISC-V: perf: Add Qemu virt machine events

This patch adds a minimal set of legacy perf events for the QEMU virt
machine to the vendor table, allowing users to use them when counter
delegation is enabled.

> diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
> index 7f5030ee1fcf8..603aa2b21c0ba 100644
> --- a/arch/riscv/include/asm/vendorid_list.h
> +++ b/arch/riscv/include/asm/vendorid_list.h
> @@ -11,4 +11,8 @@
>  #define SIFIVE_VENDOR_ID	0x489
>  #define THEAD_VENDOR_ID		0x5b7
>  
> +#define QEMU_VIRT_VENDOR_ID		0x000
> +#define QEMU_VIRT_IMPL_ID		0x000
> +#define QEMU_VIRT_ARCH_ID		0x000
> +

[Severity: Medium]
Do these definitions overload the generic "unimplemented" or "non-commercial"
CPU IDs to uniquely identify the QEMU virt machine?

According to the RISC-V Privileged Architecture specification, a value of 0
in the mvendorid, marchid, and mimpid CSRs indicates that the ID is either
unimplemented or represents a non-commercial implementation.

> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 00b84b28117a8..74acac54328e0 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c

[ ... ]

> @@ -453,7 +454,42 @@ struct riscv_vendor_pmu_events {
>  	  .hw_event_map = _hw_event_map, .cache_event_map = _cache_event_map, \
>  	  .attrs_events = _attrs },
>  

[ ... ]

>  static struct riscv_vendor_pmu_events pmu_vendor_events_table[] = {
> +	RISCV_VENDOR_PMU_EVENTS(QEMU_VIRT_VENDOR_ID, QEMU_VIRT_ARCH_ID, QEMU_VIRT_IMPL_ID,
> +				qemu_virt_hw_event_map, qemu_virt_cache_event_map,
> +				qemu_virt_event_group)
>  };

[Severity: Medium]
By binding the 0x000 ID tuple to QEMU's specific PMU event maps here, will
the kernel mistakenly assume any generic core returning 0 is a QEMU virt
machine?

If that non-QEMU core implements PMU counter delegation (Ssccfg/Smcdeleg) but
uses standard or different raw event encodings, this could unconditionally
program QEMU's arbitrary raw event values into the core's PMU configuration
registers, silently corrupting performance monitoring statistics.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-counter_delegation-v6-0-285b72ed65a9@meta.com?part=17