Re: [PATCH RFC v6 11/18] riscv_cbqri: resctrl: Add cache allocation via capacity block mask

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mismatched alloc_capable capabilities between capacity controllers at the same cache level are silently ignored, violating the intended fatal configuration error check.
- [Low] Kconfig allows enabling RESCTRL_FS without building the RISC-V resctrl architecture backend, resulting in a linker error.
--

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

riscv_cbqri: resctrl: Add cache allocation via capacity block mask

Wire CBQRI capacity controllers into resctrl as RDT_RESOURCE_L2 and
RDT_RESOURCE_L3 schemata. Mismatched CC caps at the same cache level
are treated as a fatal configuration error since fs/resctrl exposes
a single per-rid cap set. Domains are created lazily in the cpuhp
online callback so cpu_mask reflects only currently online CPUs.

> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 6abbb21f3a0dc..390353a6153a2 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -595,6 +595,7 @@ config RISCV_ISA_SSQOSID
>  	bool "Ssqosid extension support for supervisor mode Quality of Service ID"
>  	depends on 64BIT
>  	default n
> +	select ARCH_HAS_CPU_RESCTRL

[Severity: Low]
Does selecting ARCH_HAS_CPU_RESCTRL here unconditionally allow RESCTRL_FS
to be enabled, even if the RISC-V backend driver isn't built?

Since the backend implementation in cbqri_resctrl.c is gated by
RISCV_CBQRI_DRIVER:

config RISCV_CBQRI_RESCTRL_FS
    bool
    default y if RISCV_CBQRI_DRIVER && RESCTRL_FS

Could this result in a linker error for missing resctrl_arch_* symbols if
RISCV_ISA_SSQOSID is enabled but RISCV_CBQRI_DRIVER is disabled?

[ ... ]

> diff --git a/drivers/resctrl/cbqri_resctrl.c b/drivers/resctrl/cbqri_resctrl.c
> new file mode 100644
> index 0000000000000..fb6d82aa3ffc2
> --- /dev/null
> +++ b/drivers/resctrl/cbqri_resctrl.c

[ ... ]

> +static int cbqri_resctrl_pick_caches(void)
> +{
> +	struct cbqri_controller *ctrl;
> +
> +	list_for_each_entry(ctrl, &cbqri_controllers, list) {
> +		struct cbqri_resctrl_res *cbqri_res;
> +		enum resctrl_res_level rid;
> +
> +		if (ctrl->type != CBQRI_CONTROLLER_TYPE_CAPACITY)
> +			continue;
> +		if (!ctrl->alloc_capable) {
> +			if (ctrl->mon_capable)
> +				pr_warn_once("CC @%pa: monitor-only controllers aren't supported\n",
> +					     &ctrl->addr);
> +			continue;
> +		}

[ ... ]

> +		cbqri_res = &cbqri_resctrl_resources[rid];
> +		if (cbqri_res->ctrl) {
> +			/*
> +			 * CCs at the same cache level must agree on every cap
> +			 * resctrl exposes globally. Reject mismatches at pick
> +			 * time so the inconsistency is visible at boot.
> +			 */
> +			if (cbqri_res->ctrl->rcid_count != ctrl->rcid_count ||
> +			    cbqri_res->ctrl->cc.ncblks != ctrl->cc.ncblks ||
> +			    cbqri_res->ctrl->cc.supports_alloc_at_code !=
> +				    ctrl->cc.supports_alloc_at_code ||
> +			    cbqri_res->ctrl->alloc_capable != ctrl->alloc_capable) {

[Severity: Medium]
Is this alloc_capable mismatch check dead code?

Because the earlier loop condition explicitly skips controllers where
alloc_capable is false:

    if (!ctrl->alloc_capable) {
        ...
        continue;
    }

Will both cbqri_res->ctrl and the current ctrl always have alloc_capable
set to true at this point in cbqri_resctrl_pick_caches? This seems to
cause non-alloc-capable controllers to be silently ignored rather than
triggering the fatal configuration error described in the commit message.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-ssqosid-cbqri-rqsc-v7-0-v6-0-baf00f50028a@kernel.org?part=11