Re: [PATCH v10 2/6] riscv_cbqri: resctrl: Add cache allocation via capacity block mask

[Drew Fustini <fustini@kernel.org>]

On Wed, Nov 04, 2026 at 05:06:58PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] resctrl: cbqri: ZERO_SIZE_PTR deref on CDP mount when rcid_count is 1
> - [Medium] resctrl: cbqri: stale cpu_srmcfg_default and CSR_SRMCFG across cpuhp
> --
> 
> commit ba22d9f49019cdbd30a554bde797ded425016085
> Author: Drew Fustini <fustini@kernel.org>
> 
> riscv_cbqri: resctrl: Add cache allocation via capacity block mask
> 
> This commit wires RISC-V CBQRI capacity controllers into resctrl as L2 and
> L3 cache resources, managing control domains across CPU hotplug and exposing
> capacity block mask configuration.
> 
> > diff --git a/drivers/resctrl/cbqri_resctrl.c b/drivers/resctrl/cbqri_resctrl.c
> > new file mode 100644
> > index 0000000000000..165b0027bbe0f
> > --- /dev/null
> > +++ b/drivers/resctrl/cbqri_resctrl.c
[..]
> [Severity: High]
> What happens if a CBQRI capacity controller reports rcid_count == 1 and
> supports_alloc_at_code == true, and userspace mounts resctrl with CDP
> enabled (-o cdp, -o cdpl2, or -o cdpl3)?
> 
> In drivers/resctrl/cbqri_devices.c, riscv_cbqri_register_cc() only rejects
> rcid_count == 0, so a controller with rcid_count == 1 is accepted. Here,
> cbqri_resctrl_control_init() sets res->cdp_capable =
> ctrl->cc.supports_alloc_at_code without requiring ctrl->rcid_count >= 2, and
> resctrl_arch_set_cdp_enabled() allows enabling CDP whenever res->cdp_capable
> is true.
> 
> When mounting with CDP enabled, rdt_get_tree() -> schemata_list_create() ->
> schemata_list_add() halves num_closid:
> 
>     s->num_closid = resctrl_arch_get_num_closid(r);
>     if (resctrl_arch_get_cdp_enabled(r->rid))
>             s->num_closid /= 2;
> 
> With rcid_count == 1, s->num_closid becomes 0. Next, closid_init() computes
> rdt_min_closid == 0:
> 
>     list_for_each_entry(s, &resctrl_schema_all, list)
>             rdt_min_closid = min(rdt_min_closid, s->num_closid);
> 
>     closid_free_map = bitmap_alloc(rdt_min_closid, GFP_KERNEL);
>     if (!closid_free_map)
>             return -ENOMEM;
>     bitmap_fill(closid_free_map, rdt_min_closid);
> 
>     /* RESCTRL_RESERVED_CLOSID is always reserved for the default group */
>     __clear_bit(RESCTRL_RESERVED_CLOSID, closid_free_map);
> 
> Because bitmap_alloc(0, GFP_KERNEL) returns ZERO_SIZE_PTR, Could
> __clear_bit() immediately fault on ZERO_SIZE_PTR? Should
> cbqri_resctrl_control_init() or resctrl_arch_set_cdp_enabled() require
> rcid_count >= 2 for CDP?

A controller with a single RCID has nothing to allocate between in the
first place, so I will reject an alloc-capable controller with fewer
than two RCIDs in riscv_cbqri_register_cc().

> [Severity: Medium]
> Can CSR_SRMCFG and per_cpu(cpu_srmcfg, cpu) become out of sync with
> per_cpu(cpu_srmcfg_default, cpu) across CPU hotplug?
> 
> When a CPU assigned to a non-default resctrl group is taken offline,
> cbqri_resctrl_offline_cpu() calls resctrl_offline_cpu(cpu), which removes the
> CPU from the group's cpu_mask without resetting cpu_srmcfg_default to (0, 0).
> Because the CPU is no longer in the group's cpu_mask or cpu_online_mask, a
> subsequent rmdir of that group or unmount of resctrl will not clear
> cpu_srmcfg_default on the offline CPU either.
> 
> In addition, riscv/srmcfg:online (registered at arch_initcall in
> arch/riscv/kernel/qos.c) and cbqri:online (registered at late_initcall in
> cbqri_arch_late_init()) both use CPUHP_AP_ONLINE_DYN, so
> riscv_srmcfg_reset_cache() runs before cbqri_resctrl_online_cpu() on CPU
> online and sets per_cpu(cpu_srmcfg, cpu) = U32_MAX while cpu_srmcfg_default
> still holds the stale pre-offline RCID/MCID.
> 
> Inside cbqri_resctrl_online_cpu(), cbqri_attach_cpu_to_all_ctrls(cpu) runs
> before resctrl_arch_set_cpu_default_closid_rmid(cpu, 0, 0) and performs
> operations that can sleep (such as mutex_lock(), kzalloc_obj() with
> GFP_KERNEL, and usleep_range() in cbqri_wait_busy_flag()). If a context
> switch occurs on the onlining CPU during cbqri_attach_cpu_to_all_ctrls(),
> __switch_to_srmcfg() sees cpu_srmcfg == U32_MAX, writes the stale
> cpu_srmcfg_default into CSR_SRMCFG, and caches that stale value in
> cpu_srmcfg.
> 
> When cbqri_resctrl_online_cpu() then calls
> resctrl_arch_set_cpu_default_closid_rmid(cpu, 0, 0), it only updates
> cpu_srmcfg_default in memory and never calls resctrl_arch_sched_in(current)
> to refresh CSR_SRMCFG and cpu_srmcfg on the running CPU.
> 
> Should cbqri_resctrl_offline_cpu() reset the CPU's default CLOSID/RMID and
> refresh CSR_SRMCFG, and should cbqri_resctrl_online_cpu() set the default to
> (0, 0) and call resctrl_arch_sched_in(current) so CSR_SRMCFG and cpu_srmcfg
> stay synchronized?

I will reset the CPU's default RCID and MCID in
cbqri_resctrl_offline_cpu() as x86 does in clear_closid_rmid().

Drew