Re: [PATCH v3 3/8] riscv: Add support for srmcfg CSR from Ssqosid extension

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Failing cpuhp_setup_state skips PM notifier registration, silently breaking QoS IDs across system suspend/resume.
--

commit e3ff463a672a85eb328bf3048323a464af88e408
Author: Drew Fustini <fustini@kernel.org>

riscv: Add support for srmcfg CSR from Ssqosid extension

This commit adds support for the srmcfg CSR defined in the Ssqosid ISA
extension. It introduces a per-task thread field and a per-CPU cache to track
Resource Control ID (RCID) and Monitoring Counter ID (MCID) fields, updating
the CSR during context switches.

Link: https://github.com/riscv/riscv-ssqosid/releases/tag/v1.0

[ ... ]

> diff --git a/arch/riscv/kernel/qos.c b/arch/riscv/kernel/qos.c
> new file mode 100644
> index 0000000000000..c8900d91996f2
> --- /dev/null
> +++ b/arch/riscv/kernel/qos.c

[ ... ]

> +static int __init riscv_srmcfg_init(void)
> +{
> +	int err;
> +
> +	if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SSQOSID))
> +		return 0;
> +
> +	/*
> +	 * cpuhp_setup_state() invokes the startup callback locally on every
> +	 * already-online CPU, so no separate seed loop is needed here.
> +	 */
> +	err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/srmcfg:online",
> +				riscv_srmcfg_reset_cache, riscv_srmcfg_reset_cache);
> +	if (err < 0) {
> +		pr_warn("srmcfg cpuhp registration failed (%d), cpus brought online after boot will not invalidate the CSR_SRMCFG cache\n",
> +			err);
> +		return err;

[Severity: Medium]
If cpuhp_setup_state() fails and returns early here, the
cpu_pm_register_notifier() registration is skipped, but the hardware
capability RISCV_ISA_EXT_SSQOSID remains active.

Because returning an error from an arch_initcall does not halt the boot
process, the QoS feature will remain active in the kernel.

If the PM notifier is not registered to invalidate the software cache upon
exit from low-power states, will subsequent context switches find a cache
match, mistakenly bypass the csr_write(), and execute tasks with the wrong
hardware QoS ID?

> +	}
> +
> +	cpu_pm_register_notifier(&riscv_srmcfg_pm_nb);
> +	return 0;
> +}
> +arch_initcall(riscv_srmcfg_init);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-dfustini-atl-sc-cbqri-dt-v3-0-c9c1342fe3cf@kernel.org?part=3