Re: [PATCH v4 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] CBQRI controllers registered after system boot are not validated for mismatched capabilities when attached to resctrl domains.
- [Medium] Missing ARCH_HAS_CPU_RESCTRL selection in RISC-V Kconfig prevents RESCTRL_FS from being enabled.
--

--- Patch [5]: [PATCH v4 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask ---
commit bf65a655f2aedc980d538dde50ae87b2b9ea72e4
Author: Drew Fustini <fustini@xxxxxxxxxx>

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. Domains are created lazily in the cpuhp online
callback so cpu_mask reflects only currently online CPUs.

> diff --git a/drivers/resctrl/Kconfig b/drivers/resctrl/Kconfig
> index 92b9c82cf9f39..f8566c003d495 100644
> --- a/drivers/resctrl/Kconfig
> +++ b/drivers/resctrl/Kconfig
> @@ -42,3 +42,7 @@ menuconfig RISCV_CBQRI
>  if RISCV_CBQRI
>  
>  endif
> +
> +config RISCV_CBQRI_RESCTRL_FS
> +	bool
> +	default y if RISCV_CBQRI && RESCTRL_FS

[Severity: Medium]
Will this feature ever be enabled? Since the RISC-V architecture Kconfig does
not currently select ARCH_HAS_CPU_RESCTRL, it appears RESCTRL_FS can never be
satisfied, making this newly added support unreachable.

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

[ ... ]

> +static int cbqri_attach_cpu_to_cap_ctrl(struct cbqri_controller *ctrl,
> +					unsigned int cpu)
> +{
> +	struct cbqri_resctrl_res *hw_res;
> +	struct rdt_ctrl_domain *domain;
> +	struct rdt_resource *res;
> +	int dom_id;
> +	int rid;
> +
> +	rid = cbqri_cache_level_to_rid(ctrl->cache.cache_level);
> +	if (rid < 0)
> +		return 0;
> +	hw_res = &cbqri_resctrl_resources[rid];
> +
> +	if (!hw_res->ctrl)
> +		return 0;
> +
> +	res = &hw_res->resctrl_res;
> +	dom_id = ctrl->cache.cache_id;
> +
> +	domain = cbqri_find_ctrl_domain(&res->ctrl_domains, dom_id);
> +	if (domain) {
> +		cpumask_set_cpu(cpu, &domain->hdr.cpu_mask);
> +		return 0;
> +	}
> +
> +	domain = cbqri_create_ctrl_domain(ctrl, res, cpu, dom_id);

[Severity: High]
Can a mismatched controller be attached here if it is probed after system boot?

While cbqri_resctrl_pick_caches() verifies that all controllers agree on
capabilities like rcid_count and ncblks during boot, this function seems to
unconditionally create a new control domain for late-attached controllers
without similar validation against the baseline hw_res->ctrl capabilities.

If a deferred probe or late module load brings up a controller with fewer
capabilities (such as a smaller capacity block mask or fewer RCIDs), could
the resctrl core's use of the baseline capabilities lead to out-of-bounds
hardware writes when programming this new controller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dfustini-atl-sc-cbqri-dt-v4-0-e75c20201d64@xxxxxxxxxx?part=5