Handoff document for working on fixes to v4 of the Ssqosid/CBQRI/RQSC patch series. Self-contained, no external memory required.
Fixes folded into b4/ssqosid-cbqri-rqsc (the v5-prep branch in the
main worktree). Cover commit carries the v4-to-v5 changelog; b4 sees
revision 5 with base ef5f46b63023. Series stays at 18 patches,
builds clean under ARCH=riscv LLVM=1 W=1, every modified commit
passes checkpatch.pl --strict (cover-letter commit trips the known
b4-tracking-marker false positive, which is excluded per repo
convention).
Reviews covered 10 of 18 patches: P03, P07, P08, P09, P11, P12, P13,
P15, P16, P18. Reply drafts written for all 10 in
/home/pdp7/dev/linux/v5-replies/. P16 and P18 arrived later than
the initial batch; their findings are folded into v5 the same way.
sashiko.dev exposes reviews for all 18 patches (the mailing list only saw the 10 with findings). The other 8: P01/P02/P17 are clean; P04/ P05/P06/P10 had intermediate concerns dismissed in the multi-stage pipeline; P14 failed all four review attempts with a tool timeout and has no LLM coverage at all. The P10/P14 manual follow-up below covers the gap.
All real-and-meaningful findings have been addressed except the two documented limitations:
- P12 #4 / P15 #3 monitor-only CCs and BW gated on L3 OCCUP,
blocked on the same
cbqri_l3_mon_domdecoupling refactor. - P13 #1 group-removal MB_MIN reset, blocked on a new
resctrl_arch_*callback in fs/resctrl.
The mailing list mbox is missing reviews on P01/P02/P04/P05/P06/P10/ P14/P17 (sashiko.dev marked their email as Skipped or Failed). Manual review of the two interesting patches:
-
P10 (BC mon device ops) sashiko raised three internal concerns dismissed in stage gating: (a) UAF on
bc->mbm_total_statesearly publish, already closed by the P15 #1 ordering fix; (b) lockless list traversal incbqri_find_only_mon_bc(), already closed by P15 #2 plus the function being attach-time only and called undercbqri_domain_list_lock; (c) "counter snapshot during RMID reset causes massive bandwidth spike" — false positive. Per CBQRI Table 10,CONFIG_EVENTwith a non-zeroEVT_IDresets the hardware counter to 0, which matches the driver'sprev_ctr=0on the software side. The reset_rmid path atcbqri_resctrl.c:281-287already documents this. -
P14 (MB_WGHT) real bug found: the P09 #2 cache-staging refactor kcalloc-zeroes
mweight_cache, andcbqri_apply_bc_field()reads both halves ofbc_bw_allocon every CONFIG_LIMIT. The first MB_MIN domain init therefore commits Mweight=0 to every RCID. Per CBQRI 4.5 a weight of 0 is a hard cap, no opportunistic bandwidth allowed — so every RCID is starved at its Rbwb reservation (typically 1 bwblk for non-zero RCIDs) for the ~32ms window until MB_WGHT domain init catches up. Fix: pre-seedmweight_cache[*] = FIELD_MAX(MWEIGHT_MASK)incbqri_probe_bc(). Folded into the P09 commit ("riscv_cbqri: Add bandwidth controller probe and allocation device ops"),cbqri_devices.c:774.
- Cover msg-id (v4 send):
20260510-ssqosid-cbqri-rqsc-v7-0-v4-0-eb53831ef683@kernel.org - Local mbox (v4 send):
v4-mbox/20260510-...mbx - Branch (v4 send, archived):
tt-fustini/ssqosid-cbqri-rqsc-v4 - Branch (v5 prep, current):
b4/ssqosid-cbqri-rqsc - HEAD as of update:
f3ba981889e6(P18 "riscv: enable resctrl filesystem for Ssqosid") - Pre-fixes backup:
refs/backup/b4-ssqosid-cbqri-rqsc-pre-fixes(bb7fce761c9d) - Patch count: 18 (cover + 18 patches)
Series structure:
- dt-bindings: riscv: Add Ssqosid extension
- riscv: detect the Ssqosid extension
- riscv: add support for srmcfg CSR from Ssqosid
- fs/resctrl: Add resctrl_is_membw() helper
- fs/resctrl: Add RDT_RESOURCE_MB_MIN and RDT_RESOURCE_MB_WGHT
- fs/resctrl: Let bandwidth resources default to min_bw at reset
- riscv_cbqri: Add capacity controller probe and allocation device ops
- riscv_cbqri: Add capacity controller monitoring device ops
- riscv_cbqri: Add bandwidth controller probe and allocation device ops
- riscv_cbqri: Add bandwidth controller monitoring device ops
- riscv_cbqri: resctrl: Add cache allocation via capacity block mask
- riscv_cbqri: resctrl: Add L3 cache occupancy monitoring
- riscv_cbqri: resctrl: Add MB_MIN bandwidth allocation via Rbwb
- riscv_cbqri: resctrl: Add MB_WGHT bandwidth allocation via Mweight
- riscv_cbqri: resctrl: Add mbm_total_bytes bandwidth monitoring
- ACPI: RISC-V: Parse RISC-V Quality of Service Controller (RQSC) table
- ACPI: RISC-V: Add support for RISC-V Quality of Service Controller (RQSC)
- riscv: enable resctrl filesystem for Ssqosid
Sashiko reviewed 8 of 18 patches: P03, P07, P08, P09, P11, P12, P13,
P15. P15 review arrived later (2026-05-12 21:26 UTC) than the initial
batch and is captured separately in the local mbox file sash15.
No coverage on P01, P02, P04, P05, P06, P10 (BC mon), P14 (MB_WGHT), P16, P17, P18.
Coverage gap to note:
- P10 shares
cbqri_probe_feature/EVT_IDissue surface with P09, and the FIELD_MODIFY/GENMASK hazard with P08. - P14 shares cache-vs-HW divergence shape with P09/P13.
Worth eyeballing P10/P14 manually before responding to the list.
Format: [Severity per sashiko] short title -> verdict | file:line references | suggested fix.
-
[High] cpu_srmcfg early-boot bypass -> REAL.
arch/riscv/kernel/qos.c:12zero-initscpu_srmcfgin .bss.riscv_srmcfg_init()at line 30 (arch_initcall) seeds U32_MAX, but any context switches before that run with cached 0 matching thread.srmcfg=0, skipping the CSR write atarch/riscv/include/asm/qos.h:32. Hardware CSR retains implementation-defined boot value. Fix:DEFINE_PER_CPU(u32, cpu_srmcfg) = U32_MAX;atqos.c:12. -
[High] RCID/MCID conflation -> REAL.
arch/riscv/include/asm/qos.h:29tests the full thread.srmcfg word for zero. A task in default RCID with specific MCID has thread.srmcfg != 0 and bypasses cpu_srmcfg_default override. x86 evaluatestsk->closidandtsk->rmidindependently against per-CPU defaults atarch/x86/include/asm/resctrl.h:100-128. Fix: extract RCID and MCID fields viaFIELD_GET(SRMCFG_RCID_MASK)andFIELD_GET(SRMCFG_MCID_MASK), compare each againstRESCTRL_RESERVED_CLOSID = 0/RESCTRL_RESERVED_RMID = 0, and substitute from cpu_srmcfg_default per field. -
[High] Boot CPU stale across suspend -> REAL.
riscv_srmcfg_online()cpuhp callback atqos.c:24only fires on CPU online. Boot CPU never offlines during suspend, so itscpu_srmcfgcache retains pre-suspend value. Hardware CSR may have reset on resume (Ssqosid v1.0 leaves it implementation-defined). Next __switch_to_srmcfg sees cache match, skips write. Fix: register a CPU PM notifier to invalidatecpu_srmcfgto U32_MAX onCPU_PM_ENTER/ resume. -
[Medium] Redundant seed loop -> REAL minor.
qos.c:39for_each_online_cpuseeds U32_MAX, then line 42cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, ...)invokes the same callback on every already-online CPU. Race claim is stretched (both writes set the same sentinel) but the loop is dead code. Fix: drop the loop. -
[Medium] Initcall returns positive hotplug state ID -> FALSE POSITIVE.
do_one_initcall()ininit/main.cignores positive returns. No warning is logged. Cleanup: explicitreturn 0is nicer.
-
[High] controller_destroy() leaks I/O mapping -> REAL.
drivers/resctrl/cbqri_devices.c:897-902: onlykfree(ctrl->...), missingiounmap(ctrl->base)andrelease_mem_region(ctrl->addr, ctrl->size).riscv_cbqri_unregister_last()at line 914 invokes destroy on a successfully-probed controller. Fix: add iounmap + release_mem_region in destroy. -
[High] Stale RCID in AT=CODE secondary probe -> REAL.
cbqri_probe_feature()atcbqri_devices.c:501: first probe at line 527 clears RCID, but the AT secondary at line 548 restoressaved_regwithout re-clearing RCID before writing. Fix: addreg &= ~CBQRI_CONTROL_REGISTERS_RCID_MASK;after the FIELD_MODIFY of AT (line 551). -
[High] BLOCK_MASK no pre-clear before READ_LIMIT capture -> REAL.
cbqri_apply_cache_config()atcbqri_devices.c:249-255: capturessaved_cbmvia READ_LIMIT without pre-clearing the staging register. Silent firmware no-op leaves stale data in saved_cbm, later revert (line 287-290) writes garbage.cbqri_read_cache_config()at line 335 does the pre-clear dance; the writer path doesn't. Fix:cbqri_set_cbm(ctrl, 0);before line 250. -
[High] cache_size from offline CPUs during early boot -> REAL mild.
cbqri_probe_cc()atcbqri_devices.c:618-630resolves cache_size viacpu_online_mask. ACPI parse runs atsubsys_initcallso SMP is up by then; defensive code at line 647-651 declines mon_capable when cache_size=0. Real edge case ismaxcpus=boot param restricting initial bringup; cache_size stays 0 even after CPUs come online later. Fix: add CPU online callback to retry cache_size resolution and late-enable mon_capable if it succeeds. -
[Medium] rcid_count/mcid_count bounds check -> FALSE POSITIVE. Already capped at
drivers/acpi/riscv/rqsc.c:71-79. Future DT path would need its own check. Defensive add atregister_controller()would prevent regression from future discovery layers.
-
[High] RV32 GENMASK 64-bit corruption -> FALSE POSITIVE.
RISCV_ISA_SSQOSIDrequires64BITperarch/riscv/Kconfig:596, so RV32 is unreachable. Macro hygiene cleanup worth doing for portability. Optional fix: changeCBQRI_MON_CTL_OP_MASKetc. atdrivers/resctrl/cbqri_internal.h:79-82toGENMASK_ULL. -
[High] cbqri_probe_feature misapplied to MON_CTL -> REAL (partial). The EVT_ID concern is real: probe writes operation without clearing EVT_ID at
cbqri_devices.c:526-528. Bootloader junk in EVT_ID can cause false "monitoring not supported" detection. The reserved-bits concern (AT field write into a no-AT register) is cosmetic WPRI per RISC-V conventions. Fix: clear EVT_ID in addition to RCID at line 527 (cross-cuts to P09 #4). -
[Medium] Secondary AT=CODE doesn't re-clear RCID -> REAL. Same root cause as P07 #2, applied to MON_CTL (which uses the field-position as MCID). Fix is the same.
-
*[High] verify-fail leaves committed=true -> REAL (defensible).
cbqri_apply_bc_field()atcbqri_devices.c:384-385sets*committed = trueBEFORE verify; if verify fails at line 400-404 returning -EIO, caller at line 451-452 still caches the rejected value. Comment at line 380-383 defends this as "hardware probably has val even if our verify failed". Pessimistic case: cache out of sync. Fix: move*committed = trueto after verify succeeds, or change the comment to reflect the optimistic assumption explicitly. -
[High] Initial READ_LIMIT no sentinel pre-clear -> REAL.
cbqri_apply_bc_field()at line 370: READ_LIMIT loads staging from closid's state. Silent firmware no-op leaves stale RCID fields. set() only modifies one field; CONFIG_LIMIT commits the stale unmodified field, cross-RCID corruption. Verify-side at line 393 usesset(ctrl, ~val)sentinel; read-side should do equivalent before line 370. Fix: pre-write a sentinel for the other field before READ_LIMIT. Tricky because we don't know which field; might be easier to pin the other field to its known cached value via set_other(cached). -
[High] Latched faulted permanently bricks controller -> REAL.
cbqri_wait_busy_flag()atcbqri_devices.c:85latchesctrl->faulted = true. Line 93 clears on success but runtime ops at line 112/160/199 early-return BEFORE the wait, so the wait that could clear the fault is never reached. Fix: drop the early-return onctrl->faulted. Or, give it an escape valve: allow one retry on faulted controllers, clearing faulted only if the retry succeeds. -
[Medium] BC monitor probe missing EVT_ID clear -> REAL. Same as P08 #2 against BC. Fixed by the same
cbqri_probe_featurerefactor. -
[Medium] mweight bounds check -> REAL.
cbqri_apply_rbwb()at line 421-422 rejectsrbwb > U16_MAX.cbqri_apply_mweight_config()at line 458-468 has no bound. The 8-bitMWEIGHT_MASK = GENMASK(27, 20)silently truncates values255, then verify mismatches and returns -EIO instead of the expected -EINVAL. Fix:
if (mweight > 0xFF) return -EINVAL;at line 461.
-
[High] CDP_NUM_TYPES iteration on non-CDP -> DOWNGRADE (Low).
cbqri_resctrl.c:738-746: comment claims "CDP_NUM_TYPES is 1 on non-CDP controllers" butCDP_NUM_TYPESis a compile-time constant = 3 (perinclude/linux/resctrl.h:76). Loop writes 3x per RCID.cbqri_cc_alloc_op()atcbqri_devices.c:127-129masks AT to 0 on non-AT controllers, so no hardware fault. The waste is real (~3x MMIO per RCID at domain init) and the comment is wrong. Fix: rewrite the loop to iterate exactly the number of distinct slots needed (1 for non-CDP, 2 for CDP), or drop the iteration and let the AT-mirror in cbqri_apply_cache_config handle CODE/DATA. Update or delete the misleading comment. -
[High] rcid_count vs 4096 truncation -> FALSE POSITIVE.
CBQRI_MAX_RCID = 1024perinclude/linux/riscv_cbqri.h:25is capped atdrivers/acpi/riscv/rqsc.c:71-73, well belowSRMCFG_RCID_MASK = GENMASK(11,0)(4095). -
[Medium] exposed_alloc_capable cleanup on init failure -> REAL minor.
cbqri_resctrl_setup()atcbqri_resctrl.c:1253setscbqri_resctrl_inited = trueonly AFTERresctrl_init()succeeds. On failure path,cbqri_resctrl_teardown()early-returns at line 1234 becauseinited=false, leavingexposed_alloc_capable = true. Practically unobservable (resctrl FS not mounted on failure). Fix: set inited=true earlier, or reset exposed_alloc_capable directly in the failure paths. -
[Medium] wait_event between initcalls -> FALSE POSITIVE.
device_initcall_sync(__cacheinfo_ready)runs at level 6 strictly beforelate_initcall(cbqri_arch_late_init)at level 7. By the timewait_event(wait_cacheinfo_ready, cacheinfo_ready)runs atcbqri_resctrl.c:1259,cacheinfo_readyis already true. Cleanup: drop the wait_event and the producer initcall entirely. -
[Medium] synchronize_rcu inside hotplug callback -> FALSE POSITIVE.
list_del_rcu + synchronize_rcuis the same pattern used inarch/x86/kernel/cpu/resctrl/core.cfor domain teardown. Upstream-blessed. -
[Medium] cbqri_domain_list_lock redundant -> REAL but defensible. cpuhp serializes online/offline, but the mutex provides
lockdep_assert_heldin helpers at line 1032 and 1193. Keep for the lockdep value. -
[Low] Kconfig linker hazard -> FALSE POSITIVE.
arch/riscv/Kconfig:599select RISCV_CBQRI_DRIVER if RESCTRL_FSunderRISCV_ISA_SSQOSIDmakes (SSQOSID=y, RESCTRL_FS=y, CBQRI_DRIVER=n) unreachable. -
[Low] kzalloc_obj doesn't exist -> FALSE POSITIVE. Macro defined at
include/linux/slab.h:1039, widely used in kunit/iucv/net.
-
[High] AB-BA deadlock on offline -> REAL High.
cbqri_detach_cpu_from_l3_mon()atcbqri_resctrl.c:1199runs undercpus_write_lock(cpuhp) and callscancel_delayed_work_sync(&mon_dom->cqm_limbo).cqm_handle_limbo()worker atfs/resctrl/monitor.c:797starts withcpus_read_lock(). Worker blocks waiting for hotplug, hotplug blocks waiting for worker. fs/resctrl itself uses non-_synccancel_delayed_workatrdtgroup.c:4353to dodge this. Fix: switch tocancel_delayed_work(non-_sync). -
[High] cancel_delayed_work_sync on zeroed mbm_over -> REAL High.
INIT_DELAYED_WORK(&d->mbm_over, ...)atrdtgroup.c:4454-4458only fires whenresctrl_is_mbm_enabled(). CBQRI L3 OCCUP without a paired BC leaves mbm_over zero-initialized. Sync-cancel on a zeroed timer/work struct can panic indel_timer_sync. Fix: guard the cancel onresctrl_is_mbm_enabled()or checkmon_dom->mbm_over.work.funcis non-NULL. -
[High] Hotplug rollback leak -> REAL High.
cbqri_attach_cpu_to_cap_ctrl()atcbqri_resctrl.c:1135doescpumask_set_cpuBEFOREcbqri_attach_cpu_to_l3_mon()at line 1143. If l3_mon attach fails, error returns at line 1145 without removing the cpu from the mask or freeing the newly-allocated ctrl_domain. cpuhp doesn't invoke this state's offline on a failed online. Fix: undo the cpumask_set (and possibly destroy a freshly-created domain) on the error path. -
[High] Monitoring-only controllers ignored -> REAL High.
cbqri_resctrl_online_cpu()atcbqri_resctrl.c:1307-1308skips on!ctrl->alloc_capable. Mon-only controllers never get attached and no mon domain is created. Fix: also attempt attach whenctrl->mon_capableis set, even if alloc_capable is false. Split or extend the conditional. -
[High] resctrl_arch_reset_rmid never called for L3 OCCUP -> REAL mild.
mon_event_read(..., first=true)atfs/resctrl/rdtgroup.c:3335is gated onresctrl_is_mbm_event(). L3 OCCUP always getsfirst=false. The commit message claim thatresctrl_arch_reset_rmid()re-arms on recycle is wrong, but functional impact is mild becausecbqri_init_mon_counters()pre-configures the slot and OCCUP is a current-state read (not accumulator). Fix: rewrite the commit message to be accurate; optionally removeresctrl_arch_reset_rmid()impl for OCCUP path since it's dead. -
[Medium] resctrl_arch_rmid_read -EIO on nohz_full -> REAL High.
fs/resctrl/ctrlmondata.cdispatches viasmp_call_function_any(..., 1)in hardirq context for nohz_full CPUs. CBQRI'sif (irqs_disabled()) return -EIO;atcbqri_resctrl.c:312rejects unconditionally. Reason CBQRI rejects:cbqri_mon_op()takes a sleeping mutex. Fix options: (a) document that CBQRI doesn't support nohz_full monitoring; (b) provide a non-sleeping path with bounded busy-wait (CBQRI ops complete in ~1ms); (c) defer to a workqueue and return -EAGAIN with a hint. -
[Medium] Duplicate CONFIG_EVENT MMIO -> REAL minor.
resctrl_online_mon_domain()internally callsresctrl_arch_reset_rmid_all()(atcbqri_resctrl.c:284-294) which writes CONFIG_EVENT for OCCUP+MBM_TOTAL for every MCID.cbqri_init_mon_counters()atcbqri_devices.c:811-820writes the same CONFIG_EVENT/OCCUP for every MCID immediately after. Each op is ~1ms; ~2x the latency at attach. Fix: dropcbqri_init_mon_counters()(the reset_rmid_all path covers it), OR drop reset_rmid_all's per-MCID work and rely on init_mon_counters. The former is simpler. -
[Low] cbqri_resctrl_counters[] dead -> FALSE POSITIVE. Array IS read at
cbqri_resctrl.c:849to decide whether to enableQOS_L3_MBM_TOTAL_EVENT_ID.
-
[High] Group-removal leaks MB_MIN allocation -> REAL High.
closid_free()atfs/resctrl/rdtgroup.c:209just frees the bitmap bit. No arch hook to reset state.rbwb_cache[freed_closid]retains its old value and contaminates the sum-check incbqri_apply_rbwb()atcbqri_devices.c:426-440(note: that loop skips its OWN closid but includes all others, so the stale value inflates the sum for others). Fix: hook into group destroy. Either:- Add a
resctrl_arch_reset_default(closid, rid)hook that core calls on group destroy, and implement it for CBQRI. - Or, walk-and-reset
rbwb_cachefor unallocated closids before each sum check. - Or, on group destroy, have fs/resctrl call resctrl_arch_update_one with the default before freeing.
- Add a
-
[High] Multi-BC per NUMA silently ignored -> REAL.
struct cbqri_resctrl_domatcbqri_resctrl.c:carries a singlehw_ctrlpointer.cbqri_attach_cpu_to_one_bw_res()at line 1151 finds the existing domain byprox_domand just adds the cpu; the second BC sharing the same prox_dom is dropped. Unusual hardware, but silent. Fix options: (a) reject the second BC at register time with a pr_warn; (b) extend struct cbqri_resctrl_dom to track multiple hw_ctrl pointers and broadcast applies. -
[High] MRBWB underflow to 0 -> FALSE POSITIVE.
cbqri_probe_bc()atcbqri_devices.c:700-704rejectsmrbwb < rcid_count, somrbwb - (rcid_count - 1) >= 1is guaranteed. -
[Low] Reset-loop comment -> REAL minor. Comment at
cbqri_resctrl.c:631-635says "intermediate sum may exceed MRBWB". Actually the hardware sum strictly decreases during the walk. The reasoncheck_sum=falseis needed is that the cache-based sum check atcbqri_devices.c:426-440may transiently fail when neighbor cache values are still inflated. Fix: reword the comment.
(Late review, arrived 2026-05-12 21:26 UTC. Local mbox at sash15.)
-
[High] Use-After-Free / NULL pointer dereference due to premature domain exposure and TOCTOU race -> REAL High.
cbqri_attach_cpu_to_l3_mon()callsresctrl_online_mon_domain()beforecbqri_init_bc_mon_counters(). If BC init fails partway through, it freesbc->mbm_total_statesand NULLs it while a concurrent sysfs reader has already passed theWARN_ON_ONCE(!bc->mbm_total_states)check and is waiting onbc->lock. When the reader gets the lock it dereferences NULL. Fix: reorder so BC init runs before domain online. -
[High] List traversal race leading to Use-After-Free during CPU hotplug -> REAL High.
cbqri_find_ctrl_domain()walksctrl_domainswith plainlist_for_each_entry. The offline path useslist_del_rcu + synchronize_rcubut the readers don't holdrcu_read_lock, so synchronize_rcu doesn't actually wait for them. fs/resctrl protects both sides withrdtgroup_mutex; CBQRI's offline runs from cpuhp without it. Fix: takecbqri_domain_list_lockaround the reader walks, drop the RCU on the writer side. -
[Medium] Bandwidth monitoring is silently disabled if the L3 capacity controller lacks occupancy monitoring -> REAL but coupled with P12 #4. The
resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID)block sits insideif (ctrl->mon_capable)where ctrl is the L3 CC. A system with a mon-capable BC but mon-incapable L3 CC losesmbm_total_bytesentirely. Same underlying coupling as P12 #4 (mon-only-CC requires decouplingmon_domainfrom L3 CC'shw_ctrl). Deferred to the same follow-up. -
[Medium] Severe bandwidth overcounting on platforms with multiple L3 domains due to lack of topology matching -> REAL. The single paired BC pairs with every L3 domain, so userspace tools summing across L3 domains see Nx the real bandwidth. Cover letter acknowledges the assumption but doesn't gate on actual L3 domain count. Fix: count distinct L3 cache_ids in
cbqri_resctrl_pick_counters()and refuse to exposembm_total_byteswhen > 1.
(Late review, arrived 2026-05-12 21:46 UTC. Local mbox at sash16.
Spec at riscv-specs/riscv-rqsc.pdf, RQSC v0.9.2 stable.)
-
[High] Parser ignores
rqsc->numfield, may fail fatally on trailing padding -> Defensive / discard. Loop bound bynode < end, notrqsc->num. Spec does not document trailing padding allowance. Current strict-abort behaviour is defensible; no current firmware ships padded tables. -
[High] Unvalidated resource descriptor types -> REAL, addressed in v5. RQSC v0.9.2 Tables 4-6 define Resource Type (Cache/Memory) and Resource ID Type (Processor Cache / Memory Range / etc.) fields. v4 consumed
res0->id1based only on controller type, blindly trustingres[0]was the matching primary resource. v5 validates res0->type and res0->id_type per controller type, warning and skipping mismatches. -
[High]
node->flagsignored -> FALSE POSITIVE. Sashiko assumed standard ACPI bit 0 = enabled convention. RQSC v0.9.2 Table 3 defines bit 0 as "supports allocating zero capacity blocks or zero bandwidth blocks to an RCID" — a capability hint, not an enabled bit. RQSC defines no enabled flag at all. Current parser correctly ignores flags. -
[High] Hardcoded
sizeof(*node)resource offset / no revision check -> REAL, addressed in v5. v4 had no revision check; future RQSC revisions extendingacpi_rqsc_nodewould invalidate hardcodedsizeof(*node)offsets. v5 rejects tables with revision != 1 (the value from RQSC v0.9.2 Table 1) with -ENOTSUPP at function entry. -
[Medium] Missing validation of
node->lengthagainst table boundary -> Defensive / discard. Malformednode->lengthovershootingendis silently terminated by the loop. Every byte the parser reads is bound-checked, so no UAF or OOB read. ACPI tables come from signed firmware. Per the false-positive guide, defensive against trusted firmware does not warrant a check.
(Late review, arrived 2026-05-12 22:56 UTC. Local mbox at sash18.)
- [Medium] Commit message inaccuracy and unnecessary conditional
select -> REAL, addressed in v5.
The v4 commit message said "RISCV_CBQRI_DRIVER ... depends on
RESCTRL_FS" which was wrong (the driver depends only on SSQOSID).
The
select RISCV_CBQRI_DRIVER if RESCTRL_FSconditional did not match ARM64 MPAM's unconditional pattern. v5 drops theif RESCTRL_FSand rewrites the commit message to describe the new wiring. The resctrl filesystem integration stays gated byRISCV_CBQRI_RESCTRL_FS(silent, default y when both prereqs are enabled).
- False positive (12): P03 #5, P07 #5, P08 #1, P11 #2, P11 #4, P11 #5, P11 #7, P11 #8, P12 #8, P13 #3, P16 #1, P16 #3, P16 #5.
- Real but mild / defensible / cleanup (10): P03 #4, P07 #4, P09 #1, P11 #1, P11 #3, P11 #6, P12 #5, P12 #7, P13 #4, P08 #2 (reserved-bits part only).
- Real meaningful (24): P03 #1, P03 #2, P03 #3, P07 #1, P07 #2, P07 #3, P08 #2 (EVT_ID part), P08 #3, P09 #2, P09 #3, P09 #4, P09 #5, P12 #1, P12 #2, P12 #3, P12 #4, P12 #6, P13 #1, P13 #2, P15 #1, P15 #2, P15 #3, P15 #4, P16 #2, P16 #4, P18 #1.
(P16 #1 and P16 #5 are defensive against trusted firmware and classified as false positives by the strict false-positive guide.)
P07 #2, P08 #2 (EVT_ID), P08 #3, P09 #4 all stem from one function:
cbqri_probe_feature() at cbqri_devices.c:501. The probe was designed
for ALLOC_CTL (which has an AT field) and reused for MON_CTL (which
doesn't). It also fails to clear EVT_ID on every iteration and fails
to re-clear RCID on the AT=CODE secondary iteration.
Refactor goal: make cbqri_probe_feature defensively clear both RCID
and EVT_ID on every iteration. Optionally, split the AT-detection into
a separate function that's only called for ALLOC_CTL, leaving MON_CTL
probes as a simple "is the operation supported?" query.
This single refactor closes 4 findings.
P12 #1 and P12 #2 both stem from sync-cancel on delayed_work structs
that may be uninitialized or may deadlock against worker functions
taking cpus_read_lock.
Fix: switch to cancel_delayed_work (non-sync) following fs/resctrl's
pattern at rdtgroup.c:4353. Adds explicit handling for the
"work may still be running after cancel" case (the worker takes
domain_list_lock in CBQRI's case, so the post-cancel race is
bounded by our own lock).
All fixes are folded into their originating patches on branch
b4/ssqosid-cbqri-rqsc (the v5-prep branch) via git commit --fixup= plus git rebase --autosquash. Tip: 18 patches, same
shape as v4 send.
| ID | Fix |
|---|---|
| P03 #1 | DEFINE_PER_CPU(u32, cpu_srmcfg) = U32_MAX; in arch/riscv/kernel/qos.c |
| P03 #2 | __switch_to_srmcfg() evaluates RCID and MCID independently against cpu_srmcfg_default |
| P03 #3 | CPU PM notifier in riscv_srmcfg_init() invalidates cpu_srmcfg on CPU_PM_EXIT and CPU_PM_ENTER_FAILED |
| P03 #4 | Dropped redundant for_each_online_cpu seed loop |
| P07 #1 | cbqri_controller_destroy() now does iounmap() and release_mem_region() |
| P07 #2 | cbqri_probe_feature() refactor clears OP, AT, RCID, and EVT_ID on every iteration |
| P07 #3 | cbqri_apply_cache_config() pre-clears cc_block_mask before READ_LIMIT capture |
| P08 #2 | EVT_ID part: same cbqri_probe_feature() refactor |
| P08 #3 | Same cbqri_probe_feature() refactor |
| P09 #1 | *committed=true moved after verify in cbqri_apply_bc_field() |
| P09 #2 | mweight_cache added; cbqri_apply_bc_field() now seeds both fields of bc_bw_alloc from authoritative software caches, eliminating the silent-READ_LIMIT-noop corruption window |
| P09 #3 | Removed early-return on ctrl->faulted from all three runtime ops; flag is now informational only |
| P09 #4 | Same cbqri_probe_feature() refactor |
| P09 #5 | if (mweight > FIELD_MAX(...)) return -EINVAL; in cbqri_apply_mweight_config() |
| P12 #1 | cancel_delayed_work_sync -> cancel_delayed_work to break AB-BA against cqm_handle_limbo |
| P12 #2 | cancel_delayed_work(&mon_dom->mbm_over) guarded on resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID) |
| P12 #3 | Hotplug rollback in cbqri_attach_cpu_to_cap_ctrl(): undo cpumask_set_cpu and tear down a freshly-created ctrl_domain when L3 mon attach fails |
| P12 #6 | Comments at the irqs_disabled() rejects in resctrl_arch_rmid_read() and resctrl_arch_reset_rmid() reference MPAM parity |
| P13 #2 | BC register-time rejection of duplicate prox_dom with pr_warn |
| P15 #1 | Pair the BC and initialise its per-MCID accumulators before resctrl_online_mon_domain() exposes the L3 mon_domain to userspace |
| P15 #2 | Take cbqri_domain_list_lock around the list walks in resctrl_arch_rmid_read() and resctrl_arch_reset_rmid(); drop list_del_rcu/synchronize_rcu from the writer-side detach paths |
| P15 #4 | cbqri_resctrl_pick_counters() counts distinct L3 cache_ids and disables mbm_total_bytes when more than one L3 domain is present |
| P16 #2 | acpi_parse_rqsc() validates res0->type and res0->id_type per controller type before consuming res0->id1 (CACHE/PROCESSOR_CACHE for capacity, MEMORY/MEMORY_RANGE for bandwidth) |
| P16 #4 | acpi_parse_rqsc() rejects tables whose revision is not 1 (RQSC v0.9.2 Table 1) with -ENOTSUPP at function entry |
| P18 #1 | arch/riscv/Kconfig: select RISCV_CBQRI_DRIVER unconditionally (drop if RESCTRL_FS), matching ARM64 MPAM pattern; commit message rewritten to describe the new wiring |
| ID | Fix |
|---|---|
| P11 #1 | Reworded the misleading CDP_NUM_TYPES is 1 on non-CDP comment in cbqri_init_domain_ctrlval() |
| P12 #5 | Rewrote P12 commit message: dropped the inaccurate paragraph claiming resctrl_arch_reset_rmid() re-arms CONFIG_EVENT on RMID recycle. The OCCUP branch is reached only via resctrl_arch_reset_rmid_all() on mon-event config writes, not from a core RMID-recycle path |
| P13 #4 | Reworded the reset-loop comment to describe cache-based sum check |
P12 #4 monitor-only CCs / P15 #3 BW gated on L3 OCCUP. Both stem
from the same coupling: mon paths read hw_ctrl via the ctrl_domain
wrapper, so a CC that advertises mon_capable without alloc_capable
cannot back a mon_domain, and the L3 MBM_TOTAL hookup is gated on the
L3 CC being mon_capable. Decoupling needs a separate
cbqri_l3_mon_dom wrapper holding hw_ctrl and paired_bc
directly. Estimated ~115 lines net, confined to drivers/resctrl/.
CBQRI spec permits these configurations but no shipping RISC-V
silicon today exposes a mon-only CC. Decision: leave a
pr_warn_once in cbqri_resctrl_pick_caches() to surface the
limitation, address in a follow-up if/when hardware demands it.
P13 #1 group-removal MB_MIN reset. The leak (rbwb_cache[freed_closid]
stays inflated and contaminates the sum check) cannot be closed in
this series because fs/resctrl/rdtgroup.c::closid_free() has no
arch-side hook. Needs a new resctrl_arch_* callback in fs/resctrl/
that core invokes on group destroy, plus a CBQRI implementation that
resets rbwb_cache[closid] = 0. That's a core-side change, out of
scope for an arch-driver series.
P03 #5, P07 #5, P08 #1, P11 #2, P11 #4, P11 #5, P11 #7, P11 #8, P12 #8, P13 #3, P16 #1, P16 #3, P16 #5 are all false positives or already-prevented by existing code. The reply to sashiko-bot (sashiko@lists.linux.dev) explains each briefly. Notable:
- P16 #3 (node->flags) was disproved by reading RQSC v0.9.2 Table 3: bit 0 is a "supports zero allocation" capability, not an enabled bit. RQSC defines no enabled flag.
- P16 #1 and P16 #5 are defensive against malformed firmware tables; the false-positive guide treats trusted-firmware defence as non-reportable.
P03 #4 (delete redundant loop), P07 #4 (defensive comment kept), P09 #1 (timing fix done as part of cache work), P11 #3 (kept; rare failure path), P11 #6 (kept for lockdep value), P12 #5 (commit-msg fix at v5 send), P12 #7 (left as-is; ~1ms duplicate MMIO at attach is acceptable), P13 #4 (comment fix landed).
cbqri_probe_feature()hygiene (P07 #2, P08 #2 EVT_ID, P08 #3, P09 #4): the function now builds the OP write fromsaved_regmasked clean ofOP | AT | RCID | EVT_ID, then ORs in only the intended fields. Both the initial probe and the AT-CODE secondary probe go through the same builder. Single refactor, four findings closed.cbqri_apply_bc_field()cache-driven staging (P09 #2): replaces the function-pointer set/get API with a(field, val)enum API, reads bothrbwb_cache[closid]andmweight_cache[closid], stages both viacbqri_set_bc_bw_alloc(), then CONFIG_LIMIT. No initial READ_LIMIT round trip needed.cancel_delayed_work_sync->cancel_delayed_work(P12 #1, P12 #2): followsfs/resctrl/rdtgroup.c:4353pattern.
sashiko@lists.linux.dev. The reply should:
- Acknowledge each landed finding with a short note on the fix.
- For false positives, cite the file:line that already prevents the scenario sashiko worried about.
- For P12 #4 and P13 #1, explain the deferral rationale.
- For real-but-mild items, either note the fix or explain why it stays as-is.
- Use a worktree (per repo convention): branch off
b4/ssqosid-cbqri-rqscatf3ba981889e6. - Build with
ARCH=riscv LLVM=1 W=1(via./llvm.sh). - Per-commit verification:
checkpatch.pl --strict,./build-matrix.sh. - Don't bundle fixes across logical patches; each fix lands in the patch that introduced the affected code, via interactive rebase.
- v3 (#45) findings history in
~/.claude/projects/-home-pdp7-dev-linux/memory/project_sashiko_45_pending_findings.mdfor reference but most items there are already closed.
Uh oh!
There was an error while loading. Please reload this page.