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

[Drew Fustini <fustini@kernel.org>]

On Sun, Jun 28, 2026 at 09:34:24PM +0000, sashiko-bot@kernel.org wrote:
> 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@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/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?

cbqri_resctrl_pick_caches() drops a cache level entirely if its
same-level controllers disagree on rcid_count ), so every exposed level
is backed by controllers with a uniform rcid_count.

However, it doesn't hurt to harden it by iterating to the attached
domain's own hw_dom->hw_ctrl->rcid_count.

-Drew