From: Drew Fustini <fustini@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask
Date: Mon, 6 Jul 2026 17:45:30 -0700 [thread overview]
Message-ID: <akxMKn6HVbCfvAPE@x1> (raw)
In-Reply-To: <20260706232031.D95C21F000E9@smtp.kernel.org>
On Mon, Jul 06, 2026 at 11:20:31PM +0000, sashiko-bot@kernel.org wrote:
> 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@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. 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.
RISCV_CBQRI selects ARCH_HAS_CPU_RESCTRL in drivers/resctrl/Kconfig, so
RESCTRL_FS becomes selectable once CBQRI is enabled. The select is in
the CBQRI menuconfig rather than arch/riscv/Kconfig.
> > 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?
That is not possible. This driver is always built-in when enabled and
the probe never returns deferred. The resctrl setup that calls
cbqri_resctrl_pick_caches() runs later at late_initcall(), so every
controller is already on cbqri_controllers by the time pick_caches()
validates the set.
Drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Drew Fustini <fustini@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask
Date: Mon, 6 Jul 2026 17:45:30 -0700 [thread overview]
Message-ID: <akxMKn6HVbCfvAPE@x1> (raw)
In-Reply-To: <20260706232031.D95C21F000E9@smtp.kernel.org>
On Mon, Jul 06, 2026 at 11:20:31PM +0000, sashiko-bot@kernel.org wrote:
> 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@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. 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.
RISCV_CBQRI selects ARCH_HAS_CPU_RESCTRL in drivers/resctrl/Kconfig, so
RESCTRL_FS becomes selectable once CBQRI is enabled. The select is in
the CBQRI menuconfig rather than arch/riscv/Kconfig.
> > 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?
That is not possible. This driver is always built-in when enabled and
the probe never returns deferred. The resctrl setup that calls
cbqri_resctrl_pick_caches() runs later at late_initcall(), so every
controller is already on cbqri_controllers by the time pick_caches()
validates the set.
Drew
next prev parent reply other threads:[~2026-07-07 0:45 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-06 23:06 [PATCH v4 0/8] riscv: Add Ssqosid and initial CBQRI resctrl support Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 1/8] dt-bindings: riscv: Add Ssqosid extension description Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 2/8] riscv: Detect the Ssqosid extension Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 3/8] riscv: Add support for srmcfg CSR from " Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:17 ` sashiko-bot
2026-07-07 0:25 ` Drew Fustini
2026-07-07 0:25 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 4/8] riscv_cbqri: Add capacity controller probe and allocation device ops Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:20 ` sashiko-bot
2026-07-07 0:45 ` Drew Fustini [this message]
2026-07-07 0:45 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 6/8] riscv: Enable resctrl filesystem for Ssqosid Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 7/8] dt-bindings: riscv: Add binding for CBQRI controllers Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-06 23:06 ` [PATCH v4 8/8] riscv_cbqri: Add CBQRI capacity allocation platform driver Drew Fustini
2026-07-06 23:06 ` Drew Fustini
2026-07-07 8:48 ` Uwe Kleine-König
2026-07-07 8:48 ` Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=akxMKn6HVbCfvAPE@x1 \
--to=fustini@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.