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 v3 8/8] riscv_cbqri: Add CBQRI capacity allocation platform driver
Date: Mon, 6 Jul 2026 10:09:54 -0700	[thread overview]
Message-ID: <akvhYsHxMsTfBEnv@x1> (raw)
In-Reply-To: <20260628212744.1AD731F000E9@smtp.kernel.org>

On Sun, Jun 28, 2026 at 09:27:43PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] cbqri_capacity_probe() incorrectly uses get_cpu_cacheinfo_level() to find the cacheinfo, which will fail for split caches (e.g., L1I/L1D).
> --
> 
> commit 1a6b06fa1768e5d101b318b8e5167a959fc10696
> Author: Drew Fustini <fustini@kernel.org>
> 
> riscv_cbqri: Add CBQRI capacity allocation platform driver
> 
> This commit adds a device-tree platform driver for the RISC-V CBQRI capacity
> controller. It reads the cache level from the device tree and matches it
> against the system's cacheinfo to get the resctrl domain ID and shared harts.
> 
> > diff --git a/drivers/resctrl/cbqri_capacity.c b/drivers/resctrl/cbqri_capacity.c
> > new file mode 100644
> > index 0000000000000..2172432eb3287
> > --- /dev/null
> > +++ b/drivers/resctrl/cbqri_capacity.c
> [ ... ]
> > +static int cbqri_capacity_probe(struct platform_device *pdev)
> > +{
> [ ... ]
> > +	/*
> > +	 * Associate the controller with its cache instance via
> > +	 * cacheinfo. The matching cache provides the cache id and the
> > +	 * set of harts that share the cache.
> > +	 */
> > +	cache_id = -1;
> > +	cpus_read_lock();
> > +	for_each_online_cpu(cpu) {
> > +		struct cacheinfo *ci = get_cpu_cacheinfo_level(cpu, cache_level);
> 
> [Severity: Medium]
> Could this fail to match split caches, like an L1 data cache?
> 
> The get_cpu_cacheinfo_level() function unconditionally returns the first
> cacheinfo leaf it finds at the requested level and stops searching. If the
> given level has multiple leaves (for example, L1 instruction and L1 data),
> and the device tree node refers to the second leaf, this loop will miss it.
> This would cause the fw_token check to fail and the probe to return -ENODEV.

riscv,cbqri-cache points at a separate unified cache node. Split L1 I/D
come from i-cache-size/d-cache-size on the CPU node which have no
phandle. Per the CBQRI spec, the capacity controller governs a unified
shared cache, so a split private L1 is not a valid allocation target.

-Drew
~                                                                                                                                        
~                        

      reply	other threads:[~2026-07-06 17:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-28 21:18 [PATCH v3 0/8] riscv: Add Ssqosid and initial CBQRI resctrl support Drew Fustini
2026-06-28 21:18 ` [PATCH v3 1/8] dt-bindings: riscv: Add Ssqosid extension description Drew Fustini
2026-06-28 21:18 ` [PATCH v3 2/8] riscv: Detect the Ssqosid extension Drew Fustini
2026-06-28 21:18 ` [PATCH v3 3/8] riscv: Add support for srmcfg CSR from " Drew Fustini
2026-06-28 21:28   ` sashiko-bot
2026-06-29 22:46     ` Drew Fustini
2026-06-28 21:18 ` [PATCH v3 4/8] riscv_cbqri: Add capacity controller probe and allocation device ops Drew Fustini
2026-06-28 21:28   ` sashiko-bot
2026-07-01  0:24     ` Drew Fustini
2026-06-28 21:18 ` [PATCH v3 5/8] riscv_cbqri: resctrl: Add cache allocation via capacity block mask Drew Fustini
2026-06-28 21:34   ` sashiko-bot
2026-07-06 16:59     ` Drew Fustini
2026-07-01 20:18   ` Fenghua Yu
2026-07-03  8:00     ` Drew Fustini
2026-06-28 21:18 ` [PATCH v3 6/8] riscv: Enable resctrl filesystem for Ssqosid Drew Fustini
2026-06-28 21:30   ` sashiko-bot
2026-07-06 17:05     ` Drew Fustini
2026-06-28 21:18 ` [PATCH v3 7/8] dt-bindings: riscv: Add binding for CBQRI controllers Drew Fustini
2026-06-29 15:32   ` Conor Dooley
2026-06-28 21:18 ` [PATCH v3 8/8] riscv_cbqri: Add CBQRI capacity allocation platform driver Drew Fustini
2026-06-28 21:27   ` sashiko-bot
2026-07-06 17:09     ` Drew Fustini [this message]

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=akvhYsHxMsTfBEnv@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