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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dynamically registered capacity controllers bypass capability validation, leading to hardware errors that abort CPU hotplug.
--

commit 3d57a44029d899d70e2c81681585f914c01cc700
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. 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/drivers/resctrl/cbqri_resctrl.c b/drivers/resctrl/cbqri_resctrl.c
> new file mode 100644
> index 0000000000000..1fb0fbe1b000f
> --- /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]
Does this regression cause CPU hotplug to abort if a capacity controller is
dynamically registered (e.g., via a device tree overlay) after boot?

Late-registered controllers bypass the capability validation in
cbqri_resctrl_pick_caches(). When a CPU sharing this new controller is
hotplugged, it reaches this point to create a new domain.

If the late controller supports fewer RCIDs than the global controller,
cbqri_init_domain_ctrlval() will issue out-of-bounds commands to the hardware
because it iterates up to the global hw_res->ctrl->rcid_count:

drivers/resctrl/cbqri_resctrl.c:cbqri_init_domain_ctrlval() {
...
	for (i = 0; i < hw_res->ctrl->rcid_count; i++) {
		for (t = 0; t < CDP_NUM_TYPES; t++) {
			err = resctrl_arch_update_one(r, d, i, t, ...
...
}

The hardware will reject the out-of-bounds RCID commands, and the resulting
error will bubble up to cbqri_resctrl_online_cpu(), keeping the CPU
permanently offline.

Should the capabilities of dynamically registered controllers be validated
against hw_res->ctrl before attaching them?

> +	if (IS_ERR(domain))
> +		return PTR_ERR(domain);
> +
> +	return 0;
> +}

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