The Linux Kernel Mailing List
 help / color / mirror / Atom feed
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

  reply	other threads:[~2026-07-07  0:45 UTC|newest]

Thread overview: 13+ 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 ` [PATCH v4 1/8] dt-bindings: riscv: Add Ssqosid extension description Drew Fustini
2026-07-06 23:06 ` [PATCH v4 2/8] riscv: Detect the Ssqosid extension Drew Fustini
2026-07-06 23:06 ` [PATCH v4 3/8] riscv: Add support for srmcfg CSR from " Drew Fustini
2026-07-06 23:17   ` sashiko-bot
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 ` [PATCH v4 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask Drew Fustini
2026-07-06 23:20   ` sashiko-bot
2026-07-07  0:45     ` Drew Fustini [this message]
2026-07-06 23:06 ` [PATCH v4 6/8] riscv: Enable resctrl filesystem for Ssqosid 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 ` [PATCH v4 8/8] riscv_cbqri: Add CBQRI capacity allocation platform driver Drew Fustini

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox