* [PATCH AUTOSEL 6.18-5.15] dmaengine: dw-axi-dmac: fix PM for system sleep and channel alloc
[not found] <20260831133314.4125787-1-sashal@kernel.org>
@ 2026-08-31 13:23 ` Sasha Levin
2026-08-31 14:58 ` sashiko-bot
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-6.12] dmaengine: altera-msgdma: Use memcpy_toio for descriptor FIFO writes Sasha Levin
1 sibling, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2026-08-31 13:23 UTC (permalink / raw)
To: patches, stable
Cc: Tze Yee Ng, Vinod Koul, Sasha Levin, Eugeniy.Paltsev, dmaengine,
linux-kernel
From: Tze Yee Ng <tze.yee.ng@altera.com>
[ Upstream commit df0c2dc68770cf43f15df40b184df030b850ea05 ]
The driver only had runtime PM callbacks. If a channel stayed allocated
across system suspend/resume, the runtime usage count could remain
non-zero while hardware state (DMAC_CFG, clocks) was lost, and
axi_dma_runtime_resume() would not run to restore it.
Add system-sleep PM ops that use pm_runtime_force_suspend() and
pm_runtime_force_resume() so suspend/resume reuses the existing
axi_dma_suspend() and axi_dma_resume() paths.
Replace pm_runtime_get() with pm_runtime_resume_and_get() in
dma_chan_alloc_chan_resources() so clocks are enabled before a client
can immediately submit a transfer and touch MMIO.
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
Link: https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508d8ac6371896.1779688569.git.tze.yee.ng@altera.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable Backport Analysis: `dmaengine: dw-axi-dmac: fix PM for system
sleep and channel alloc`
**Local tree:** `v6.18.44` (`stable/linux-6.18.y`, detached HEAD)
**Mainline commit:** `df0c2dc68770c` (merged by Vinod Koul, 2026-06-11)
**Status in this tree:** Buggy code is present; fix is **not** yet
applied (`NOT_IN_TREE`)
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Parse the subject line
**Record:** `[dmaengine: dw-axi-dmac]` `[fix]` — Correct power-
management handling for system sleep and DMA channel allocation.
### Step 1.2: Parse all commit message tags
**Record:**
- **Fixes:** — absent (expected for manual review)
- **Reported-by:** — absent
- **Tested-by:** — absent
- **Reviewed-by / Acked-by:** — absent in commit message
- **Link:** https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508d8ac63
71896.1779688569.git.tze.yee.ng@altera.com
- **Cc: stable:** — absent (not a negative signal)
- **Signed-off-by:** Tze Yee Ng (author), Vinod Koul (subsystem
maintainer, committer)
- **Notable:** Merged by dmaengine maintainer; patch 2/2 in a reviewed
series
### Step 1.3: Analyze commit body
**Record:**
- **Bug:** Driver registered only runtime PM callbacks. With a channel
allocated across system suspend/resume, runtime usage count can stay
non-zero while hardware state (DMAC_CFG, clocks) is lost;
`axi_dma_runtime_resume()` is then skipped.
- **Symptom:** DMA controller left with clocks off and/or DMAC_CFG not
restored after resume; subsequent DMA/MMIO can fail or hang.
- **Second bug:** `pm_runtime_get()` in
`dma_chan_alloc_chan_resources()` bumps the usage counter without
resuming; a client can submit a transfer immediately and touch MMIO
before clocks are enabled.
- **Root cause:** Missing system-sleep PM ops; incorrect runtime PM API
usage on channel allocation.
- **Version info:** None stated; driver has had this pattern since 2018.
### Step 1.4: Detect hidden bug fixes
**Record:** Not disguised — explicitly described as a PM bug fix. The
`pm_runtime_resume_and_get()` change also adds missing
`pm_runtime_put()` on error paths (refcount balance), which is proper
error-path cleanup tied to the fix.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory the changes
**Record:**
- **File:** `drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c` (+9 / -2)
- **Functions modified:** `dma_chan_alloc_chan_resources()`,
`dw_axi_dma_pm_ops`
- **Scope:** Single-file, surgical fix
### Step 2.2: Code flow change per hunk
**Hunk 1 — `dma_chan_alloc_chan_resources()`:**
- **Before:** Check idle → allocate descriptor pool → `pm_runtime_get()`
(counter only, no resume) → return 0. Error paths did not balance
runtime PM.
- **After:** `pm_runtime_resume_and_get()` first (resume + increment);
on `-EBUSY` / `-ENOMEM`, `pm_runtime_put()` before return.
- **Path affected:** Normal DMA client channel allocation (common
client-driver path).
**Hunk 2 — `dw_axi_dma_pm_ops`:**
- **Before:** Only `SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend,
axi_dma_runtime_resume, NULL)`.
- **After:** Adds `SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)`.
- **Path affected:** System suspend/resume (S3/hibernate on affected
SoCs).
### Step 2.3: Bug mechanism
**Record:**
- **Category (a):** Error-path refcount fix — `pm_runtime_put()` on
allocation failure after `resume_and_get`.
- **Category (b):** PM / suspend-resume correctness — system sleep now
forces runtime suspend/resume regardless of usage count.
- **Category (c):** Reference-counting / PM API misuse —
`pm_runtime_get()` does not resume; `pm_runtime_resume_and_get()`
does.
- **Specific mechanism:** After system sleep, hardware is reset but
software refcount says device is "active," so runtime resume is
skipped and `axi_dma_resume()` (clocks + DMAC enable) never runs.
### Step 2.4: Fix quality
**Record:**
- **Quality:** High. Uses the standard kernel pattern documented in
`DEFINE_RUNTIME_DEV_PM_OPS()` / `pm_runtime.h` comments.
- **Minimal:** 9 lines, no API changes.
- **Regression risk:** Very low. `pm_runtime_force_suspend/resume` are
well-tested core PM helpers; error-path `pm_runtime_put()` is correct
pairing.
- **Red flags:** None.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame changed lines
**Record:**
- `dma_chan_alloc_chan_resources()` and `pm_runtime_get()`: introduced
in `1fe20f1b84548` (2018-03-06, "Introduce DW AXI DMAC driver").
- `dw_axi_dma_pm_ops` with runtime-only ops: same commit, 2018.
- Bug has been present since driver introduction in this tree.
### Step 3.2: Follow Fixes: tag
**Record:** No `Fixes:` tag. N/A.
### Step 3.3: File history for related changes
**Record:**
- Recent stable-tree changes: StarFive JH8100/JH7110 support, per-
channel IRQ, array overrun fix.
- Patch 1 of series (`dc6d681e1571c` — "drop redundant DMAC enable in
block start") is **not** in this tree (`PATCH1_NOT_IN_TREE`).
- This commit (patch 2) is **standalone**; it does not depend on patch
1. Patch 1 without patch 2 would expose the PM gap more; patch 2 alone
is sufficient and correct for 6.18.y.
### Step 3.4: Author's other commits
**Record:** Tze Yee Ng — Altera/Intel contributor; author of
stratix10-svc fixes. Vinod Koul committed and is dmaengine maintainer.
### Step 3.5: Prerequisites
**Record:** No prerequisites. `pm_runtime_force_suspend`,
`pm_runtime_force_resume`, and `pm_runtime_resume_and_get` all exist in
this tree's `include/linux/pm_runtime.h`. Patch applies cleanly (`git
apply --check` exit 0).
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: Original patch discussion
**Record:**
- **b4 dig URL:** https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508
d8ac6371896.1779688569.git.tze.yee.ng@altera.com
- **Series:** v2 0/2 "clean up DMAC enable and PM" (2026-05-25)
- **Revisions:** v2 only found by b4 dig `-a`
- **Key feedback:** Patch 2 added per review feedback from Sashiko
Watanabe (AI review bot flagged issues; patch 2 addresses PM gap
identified in review)
- **Maintainer:** Vinod Koul replied "Applied, thanks!" applying both
patches
- **Stable nomination in thread:** None found
- **NAKs:** None found
### Step 4.2: Reviewers from b4 dig -w
**Record:** CC'd: Eugeniy Paltsev (Synopsys, original driver author),
Vinod Koul, Frank Li, dmaengine@vger.kernel.org, linux-
kernel@vger.kernel.org.
### Step 4.3: Bug report
**Record:** No external bug report or syzbot link. Bug identified
through code review / PM analysis during series review.
### Step 4.4: Related patches / series
**Record:** 2-patch series. Only patch 2 is needed for this backport
decision. Patch 1 is optional cleanup not present in 6.18.y.
### Step 4.5: Stable mailing list
**Record:** Not searched separately; no stable discussion found in
downloaded thread.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key functions
**Record:** `dma_chan_alloc_chan_resources()`,
`dma_chan_free_chan_resources()` (unchanged, has matching
`pm_runtime_put`), `axi_dma_suspend()`, `axi_dma_resume()`,
`axi_dma_runtime_suspend/resume()`, `dw_axi_dma_pm_ops`.
### Step 5.2: Callers
**Record:** `dma_chan_alloc_chan_resources` is registered as
`device_alloc_chan_resources` in the dmaengine device ops (line 1565).
Called by any DMA client requesting a channel — SDHCI, SPI, audio, etc.
on affected SoCs.
### Step 5.3: Callees
**Record:** `pm_runtime_resume_and_get()` → `pm_runtime_get_active()` →
`__pm_runtime_resume()`; system sleep uses
`pm_runtime_force_suspend/resume` → existing `axi_dma_suspend/resume`
(clock disable/enable, `axi_dma_disable/enable`).
### Step 5.4: Call chain / reachability
**Record:**
1. **Suspend/resume:** Platform system sleep → driver
`.suspend`/`.resume` → force runtime suspend/resume → restore clocks
and DMAC.
2. **Channel alloc:** Userspace/driver → `dma_request_channel()` →
`alloc_chan_resources()` → must have clocks before any transfer.
- **Userspace reachable:** Yes, indirectly via drivers using DMA on
StarFive, Intel KMB, Altera/Intel FPGA platforms.
### Step 5.5: Similar patterns
**Record:** Other DMA drivers in this tree already use
`pm_runtime_resume_and_get()` in alloc paths (e.g. `zynqmp_dma.c`,
`tegra20-apb-dma.c`, `stm32-dma.c`) and/or
`SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)` (e.g. `dw_mmc-pltfm.c`, `idma64.c`). This fix
aligns dw-axi-dmac with established practice.
---
## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE
### Step 6.1: Does buggy code exist?
**Record:** **Yes.** Current tree at `v6.18.44` has:
- `pm_runtime_get()` at line 538 in `dma_chan_alloc_chan_resources()`
- Runtime-only `dw_axi_dma_pm_ops` at lines 1654–1656
- Affected platforms in OF table: `snps,axi-dma-1.01a`, `intel,kmb-axi-
dma`, `starfive,jh7110-axi-dma`, `starfive,jh8100-axi-dma`
### Step 6.2: Backport complications
**Record:** Clean apply expected. `git apply --check` on mainline patch
succeeded. No structural divergence in the changed regions.
### Step 6.3: Related fixes already present?
**Record:** No equivalent fix found. `git merge-base --is-ancestor
df0c2dc68770c HEAD` → `NOT_IN_TREE`.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: Subsystem criticality
**Record:** `drivers/dma/dw-axi-dmac` — **IMPORTANT** (DMA engine for
multiple embedded SoC platforms; suspend/resume and DMA are core to I/O
on those systems).
### Step 7.2: Subsystem activity
**Record:** Actively maintained in 6.18.y (StarFive JH8100, per-channel
IRQ, overrun fix in recent history).
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Who is affected
**Record:** Users of dw-axi-dmac on Intel KMB, StarFive JH7110/JH8100,
and Synopsys/Altera AXI DMA platforms — embedded boards, FPGA SoCs.
Config: `CONFIG_DW_AXI_DMAC` (or built-in on those platforms).
### Step 8.2: Trigger conditions
**Record:**
1. DMA channel allocated, system enters suspend (S3/hibernate), then
resumes — **common** on laptops/embedded devices.
2. Device runtime-suspended, client allocates channel and immediately
submits transfer — **plausible** under autosuspend.
- **Unprivileged trigger:** Indirectly yes (e.g., triggering suspend or
I/O that uses DMA).
### Step 8.3: Failure mode severity
**Record:**
- DMA failures after resume (broken I/O: storage, network, audio)
- MMIO with clocks disabled → bus hang, timeout, or oops
- **Severity: HIGH** (system-level I/O breakage; potential hang)
### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** HIGH — fixes real suspend/resume breakage on shipping
hardware
- **Risk:** LOW — 9-line, standard PM pattern, maintainer-merged
- **Ratio:** Strongly favors backport
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence summary
**FOR backport:**
- Fixes real PM bug present since driver introduction (2018)
- Affects multiple platforms in this stable tree
- Can break DMA and cause hangs after suspend/resume
- Small, obviously correct, maintainer-merged fix
- Applies cleanly to v6.18.44
- Uses established kernel PM APIs/patterns
- Standalone — does not require patch 1 of the series
**AGAINST backport:**
- No syzbot/user crash report (review-found bug)
- Driver-specific, not core kernel (but suspend/resume is critical for
affected users)
**Unresolved:** No explicit `Tested-by` on hardware; no `Cc: stable`
nomination in thread.
### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — standard PM pattern; merged
by maintainer after review
2. Fixes a real bug? **PASS** — suspend/resume state desync and alloc-
without-resume
3. Important issue? **PASS** — HIGH: post-resume DMA failure / potential
hang
4. Small and contained? **PASS** — 9 lines, one file
5. No new features/APIs? **PASS** — only PM ops wiring and correct API
usage
6. Can apply to local tree? **PASS** — verified clean apply
### Step 9.3: Exception categories
**Record:** None (not a quirk/ID/DT/docs fix) — qualifies on straight
bug-fix merits.
### Step 9.4: Decision rationale
For **linux-6.18.y (v6.18.44)**, the buggy code is present and the fix
is absent. The commit addresses a longstanding power-management defect:
without system-sleep PM ops, suspend/resume can leave the DMAC with
clocks off and hardware unconfigured while the runtime PM counter
indicates the device is still active. The `pm_runtime_resume_and_get()`
change fixes a second, independently valid bug where channel allocation
does not ensure the device is resumed before clients can use it. The
change is minimal, follows patterns already used elsewhere in
`drivers/dma/`, and applies cleanly. This is appropriate stable
material.
---
## Verification
- **[Phase 1]** `git describe HEAD` → `v6.18.44`; parsed commit message
from user query and `git show df0c2dc68770c`
- **[Phase 2]** Read current `dw-axi-dmac-platform.c` lines 516–564,
1315–1356, 1654–1656; confirmed diff matches missing fix
- **[Phase 3]** `git blame` lines 516–540, 1654–1656 → `1fe20f1b84548`
(2018); `git merge-base --is-ancestor df0c2dc68770c HEAD` →
`NOT_IN_TREE`; patch 1 also `NOT_IN_TREE`
- **[Phase 3]** `git apply --check` on `df0c2dc68770c` patch → exit 0
(clean apply)
- **[Phase 4]** `b4 dig -c df0c2dc68770c` → lore URL; `b4 dig -a` → v2
series; `b4 dig -w` → maintainers CC'd; mbox → Vinod "Applied,
thanks!"
- **[Phase 4]** WebFetch lkml.iu.edu cover letter → patch 2 addresses
Sashiko Watanabe review feedback
- **[Phase 5]** `grep pm_runtime_resume_and_get drivers/dma/` → pattern
used in peer drivers; read `axi_dma_enable/suspend/resume` code
- **[Phase 6]** Confirmed buggy `pm_runtime_get` and runtime-only PM ops
in tree; OF compatibles present
- **[Phase 7]** `git log --oneline -20 -- drivers/dma/dw-axi-dmac/` →
active maintenance
- **[Phase 8]** Failure mode derived from verified code paths
(`axi_dma_suspend` disables clocks; `pm_runtime_get` does not resume
per `pm_runtime.h` docs)
**YES**
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index b0e689f48bb67..ea4e47ee0d710 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -516,11 +516,17 @@ static void dw_axi_dma_synchronize(struct dma_chan *dchan)
static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
{
struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
+ int ret;
+
+ ret = pm_runtime_resume_and_get(chan->chip->dev);
+ if (ret < 0)
+ return ret;
/* ASSERT: channel is idle */
if (axi_chan_is_hw_enable(chan)) {
dev_err(chan2dev(chan), "%s is non-idle!\n",
axi_chan_name(chan));
+ pm_runtime_put(chan->chip->dev);
return -EBUSY;
}
@@ -531,12 +537,11 @@ static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
64, 0);
if (!chan->desc_pool) {
dev_err(chan2dev(chan), "No memory for descriptors\n");
+ pm_runtime_put(chan->chip->dev);
return -ENOMEM;
}
dev_vdbg(dchan2dev(dchan), "%s: allocating\n", axi_chan_name(chan));
- pm_runtime_get(chan->chip->dev);
-
return 0;
}
@@ -1652,6 +1657,8 @@ static void dw_remove(struct platform_device *pdev)
}
static const struct dev_pm_ops dw_axi_dma_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
};
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.18-6.12] dmaengine: altera-msgdma: Use memcpy_toio for descriptor FIFO writes
[not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.15] dmaengine: dw-axi-dmac: fix PM for system sleep and channel alloc Sasha Levin
@ 2026-08-31 13:25 ` Sasha Levin
2026-08-31 15:36 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2026-08-31 13:25 UTC (permalink / raw)
To: patches, stable
Cc: Adrian Ng Ho Yin, Tze Yee Ng, Vinod Koul, Sasha Levin,
adrian.ho.yin.ng, dmaengine, linux-kernel
From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
[ Upstream commit 94963138cce29f85605d76c94fa1d43a0335ead9 ]
The descriptor FIFO requires that all words of a descriptor are written
in order, with the control word written last to flush it into the DMA
engine. Using memcpy() with __force to __iomem is not the correct API
and does not guarantee appropriate MMIO access on all architectures.
Replace the descriptor body copy with memcpy_toio(), using
offsetof(struct msgdma_extended_desc, control) to exclude the control
word. This matches the previous sizeof(desc->hw_desc) - sizeof(u32)
length only when control is the last struct member; add a static_assert
to enforce that layout so a future field after control cannot silently
break FIFO ordering.
Keep writing the control word separately with write barriers, so it
remains the final word pushed into the FIFO.
Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
Link: https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab9bd76.1779697226.git.tze.yee.ng@altera.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
**Step 1.1 – Subject line**
Record: `[dmaengine: altera-msgdma] [Use] Use memcpy_toio for descriptor
FIFO writes` — subsystem is Altera mSGDMA DMAengine driver; action is
replacing incorrect MMIO copy API.
**Step 1.2 – Tags**
Record:
- **Fixes:** none
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by / Acked-by:** none
- **Link:** https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab
9bd76.1779697226.git.tze.yee.ng@altera.com
- **Signed-off-by:** Adrian Ng Ho Yin, Tze Yee Ng, Vinod Koul
(maintainer)
- **Cc: stable@vger.kernel.org:** absent (expected for manual review)
- Notable: no syzbot, no user bug report, no crash trace
**Step 1.3 – Body analysis**
Record:
- **Bug:** Descriptor FIFO hardware requires ordered 32-bit word writes;
control word must be written last to flush the descriptor. Using
`memcpy()` with `__force` cast to `__iomem` is not the correct MMIO
API and does not guarantee proper access on all architectures.
- **Symptom:** Malformed or partially written descriptors can cause DMA
to wrong addresses, transfer failures, or silent data corruption.
- **Root cause:** Body of descriptor copied via plain `memcpy()` to MMIO
instead of `memcpy_toio()`.
- **Version info:** none in message
**Step 1.4 – Hidden bug fix?**
Record: **Yes.** Although framed as an API correction, it fixes a real
hardware-interaction bug. The pre-existing comment in the driver already
acknowledged uncertainty about `memcpy()` write ordering; this commit
completes that fix by using the proper MMIO helper.
---
## Phase 2: Diff Analysis
**Step 2.1 – Inventory**
Record:
- **File:** `drivers/dma/altera-msgdma.c` (+15 / −9, 24 lines touched)
- **Function modified:** `msgdma_copy_one()` only
- **Scope:** single-file, surgical fix
**Step 2.2 – Code flow change**
Record:
- **Hunk 1 (before FIFO wait):** adds duplicate `static_assert`
verifying `control` is the last struct member.
- **Hunk 2 (before body copy):** adds second identical `static_assert`
(copy-paste error acknowledged by author on list).
- **Hunk 3 (body copy):** `memcpy((void __force *)hw_desc, ...)` →
`memcpy_toio(hw_desc, ..., offsetof(..., control))`; control word
still written separately via `iowrite32()` + `wmb()`.
- **Path affected:** every descriptor submission to hardware FIFO
(normal DMA operation path).
**Step 2.3 – Bug mechanism**
Record: **Memory safety / MMIO correctness fix.** Category: incorrect
MMIO access pattern. Plain `memcpy()` to `__iomem` may use CPU memory
copy optimizations (reordering, non-MMIO stores) that do not satisfy
hardware FIFO ordering requirements. `memcpy_toio()` uses architecture-
appropriate MMIO store sequences.
**Step 2.4 – Fix quality**
Record: Fix is logically correct and minimal. Regression risk is very
low — it replaces one copy primitive with the documented equivalent.
Minor quality blemish: duplicate `static_assert` blocks (author agreed
to fix in v3, but v2 was merged as-is). Pre-existing concerns (BE
endianness mix of `memcpy_toio` + `iowrite32`, `mdelay()` while holding
spinlock) are unchanged by this patch.
---
## Phase 3: Git History Investigation
**Step 3.1 – Blame**
Record: Buggy `memcpy()` path introduced in `6084fc2ec478d` (Aug 2017,
Stefan Roese, "Use macros instead of structs..."). Original driver added
in `a85c6f1b2921c` (Jul 2017). Bug has been present ~9 years.
**Step 3.2 – Fixes: tag**
Record: N/A — no Fixes: tag present.
**Step 3.3 – Related file history**
Record: Recent stable-tree changes to this file include descriptor
free/cleanup fixes (`54e4ada1a4206`, `d3ddfab0969b1`), spinlock IRQ
variant fix (`261d3a85d9598`). No related fix for MMIO copy already
present. Standalone patch (v2 of 1-patch series).
**Step 3.4 – Author context**
Record: Authors are Altera/Intel engineers (hardware vendor). Vinod Koul
(dmaengine maintainer) applied the patch. Authors are not regular
altera-msgdma maintainers but submitted from hardware expertise.
**Step 3.5 – Dependencies**
Record: No prerequisites. Uses `memcpy_toio()` and `static_assert`, both
available in Linux 6.18. Applies cleanly (`git apply --check` passed).
---
## Phase 4: Mailing List and External Research
**Step 4.1 – Original discussion**
Record:
- **URL:** https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab9
bd76.1779697226.git.tze.yee.ng@altera.com
- **Series:** v2 only (v1 not in thread); committed version matches v2
- **Maintainer response:** Vinod Koul — "Applied, thanks!"
- **No stable nomination** from reviewers
- **No NAKs** from human reviewers
**Step 4.2 – Reviewers**
Record: CC'd: Olivier Dautricourt, Stefan Roese (original driver
author), Vinod Koul, Frank Li, dmaengine@, linux-kernel@. Appropriate
maintainers included.
**Step 4.3 – Bug report**
Record: No external bug report. Sashiko AI review flagged duplicate
static_assert (Low) and pre-existing MMIO/endianness/spinlock+mdelay
issues (High, pre-existing). Author Tze Yee Ng agreed duplicate assert
was copy-paste error; offered v3 with single assert and optional
`iowrite32()` loop if Frank Li preferred. Frank Li asked author to
review Sashiko comments; no further human NAK before merge.
**Step 4.4 – Related patches**
Record: Standalone. Author indicated FIFO polling and stricter MMIO
access could be separate follow-ups.
**Step 4.5 – Stable list history**
Record: Not searched separately; no stable nomination found in patch
thread.
---
## Phase 5: Code Semantic Analysis
**Step 5.1 – Key functions**
Record: `msgdma_copy_one()` modified; callers unchanged.
**Step 5.2 – Callers**
Record:
- `msgdma_copy_desc_to_fifo()` → called from `msgdma_start_transfer()`
- `msgdma_start_transfer()` called from:
- `msgdma_issue_pending()` (under `spin_lock_irqsave`)
- `msgdma_irq_handler()` (under `spin_lock`)
- Reachable on every DMA transfer submission and from IRQ when
controller becomes idle.
**Step 5.3 – Callees**
Record: `ioread32()` (FIFO full check), `mdelay(1)` (wait loop),
`memcpy_toio()` (new), `wmb()`, `iowrite32()` (control word flush).
**Step 5.4 – Reachability**
Record: Triggered whenever userspace/kernel submits DMA operations
through the dmaengine API on Altera mSGDMA hardware
(`CONFIG_ALTERA_MSGDMA`). Common operational path, not init-only or
error-only.
**Step 5.5 – Similar patterns**
Record: Other dma drivers use `memcpy_toio()` for MMIO (e.g., edma). The
forced `memcpy()` to `__iomem` pattern is explicitly discouraged in
kernel MMIO documentation.
---
## Phase 6: Cross-Reference Against Local Tree
**Step 6.1 – Buggy code in this tree?**
Record: **Yes.** Local tree is **Linux 6.18.44** (`git describe HEAD` →
v6.18.44). Buggy `memcpy((void __force *)hw_desc, ...)` present at lines
518–519 of `drivers/dma/altera-msgdma.c`. Bug present since driver
introduction (2017).
**Step 6.2 – Backport complications**
Record: **Clean apply** confirmed via `git format-patch -1 94963138cce29
| git apply --check`. No conflicting recent changes to this function in
6.18.y.
**Step 6.3 – Related fixes already present?**
Record: Commit `94963138cce29` is **not** in `stable/linux-6.18.y` (`git
merge-base --is-ancestor` returned exit 1). No equivalent fix found via
grep for `memcpy_toio` in this file.
---
## Phase 7: Subsystem and Maintainer Context
**Step 7.1 – Subsystem criticality**
Record: **dmaengine / Altera mSGDMA driver** — **PERIPHERAL** (niche
FPGA/embedded hardware, `CONFIG_ALTERA_MSGDMA`). However, DMA descriptor
corruption affects memory integrity for users of that hardware.
**Step 7.2 – Subsystem activity**
Record: Driver receives occasional maintenance (descriptor lifecycle,
spinlock fixes in 2024–2025). Mature, low-churn code.
---
## Phase 8: Impact and Risk Assessment
**Step 8.1 – Who is affected**
Record: Users with `CONFIG_ALTERA_MSGDMA` on Altera/Intel FPGA platforms
(PCIe-connected Cyclone and similar, ARM and x86 per original driver
testing). Config-specific, platform-specific.
**Step 8.2 – Trigger conditions**
Record: Every DMA descriptor submission. Not timing-dependent race;
architecture-dependent MMIO behavior. Unprivileged users can trigger via
dmaengine consumers using this controller.
**Step 8.3 – Failure mode severity**
Record: Corrupted descriptor fields → DMA to/from wrong physical
addresses → **data corruption** (CRITICAL for affected users). Could
also cause transfer failure or hardware stall. No crash/oops required
for impact.
**Step 8.4 – Risk-benefit**
Record:
- **Benefit:** Prevents potential silent memory corruption on non-x86 or
strict MMIO architectures; uses correct kernel API; hardware vendor
fix.
- **Risk:** Very low — 1-line functional change plus compile-time
assert; applies cleanly.
- **Ratio:** Favorable for affected hardware users; limited broader
impact due to niche driver.
---
## Phase 9: Final Synthesis
**Step 9.1 – Evidence summary**
**FOR backport:**
- Real MMIO correctness bug present since 2017 in this tree
- Potential DMA data corruption (critical for affected users)
- Small, obviously correct, clean-apply fix
- Hardware vendor submitted with maintainer acceptance
- Original driver comment acknowledged memcpy ordering uncertainty
- Commit not yet in 6.18.y
**AGAINST backport:**
- No user bug report, crash, or syzbot finding
- Very niche driver (FPGA embedded)
- Driver has functioned with workaround since 2017 without reported
failures
- Duplicate `static_assert` in merged code (cosmetic)
- Does not address all MMIO concerns raised in AI review (pre-existing)
- No explicit stable nomination
**Unresolved:** No verified report of the bug manifesting in production;
impact inferred from API semantics and hardware requirements.
**Step 9.2 – Stable rules checklist**
1. Obviously correct and tested? **PASS** — API fix is standard; no
runtime Tested-by, but logically sound.
2. Fixes a real bug affecting users? **PASS** — incorrect MMIO access on
descriptor submission path.
3. Important issue? **PASS** — potential DMA data corruption.
4. Small and contained? **PASS** — 1 file, ~24 lines.
5. No new features/APIs? **PASS** — behavior correction only.
6. Can apply to local tree? **PASS** — clean apply verified.
**Step 9.3 – Exception categories**
Record: Not a device ID, quirk, DT, build, or docs fix. Standard driver
correctness bug fix.
**Step 9.4 – Decision rationale**
For Linux 6.18.y, the buggy code exists, the fix applies cleanly, and
the failure mode (DMA writing to wrong memory) is serious for the Altera
mSGDMA users this driver serves. While the driver is niche and lacks a
field bug report, the original code explicitly doubted `memcpy()`
ordering, the hardware vendor corrected the MMIO access pattern, and the
patch is minimal with negligible regression risk. Stable trees routinely
accept such hardware-interaction fixes when data corruption is the
consequence.
---
## Verification
- [Phase 1] Parsed commit `94963138cce29` on master; tags: Link, SOBs
only; no Reported-by/syzbot
- [Phase 2] Diff: 1 file, `msgdma_copy_one()` only; `memcpy` →
`memcpy_toio`; duplicate static_assert confirmed in `git show
94963138cce29`
- [Phase 3] `git blame`: buggy memcpy from `6084fc2ec478d` (2017);
driver from `a85c6f1b2921c` (2017)
- [Phase 3] `git log -20 -- drivers/dma/altera-msgdma.c`: no prior MMIO
fix
- [Phase 3] `git format-patch -1 94963138cce29 | git apply --check`:
clean apply
- [Phase 4] `b4 dig -c 94963138cce29`: lore URL found; v2 series
- [Phase 4] `b4 dig -c 94963138cce29 -w`: Vinod Koul, Frank Li, Stefan
Roese CC'd
- [Phase 4] `b4 dig -m /tmp/altera-msgdma.mbx`: Vinod applied; author
acknowledged duplicate assert; no stable nomination
- [Phase 5] Call chain verified in source:
`msgdma_issue_pending`/`msgdma_irq_handler` → `msgdma_start_transfer`
→ `msgdma_copy_desc_to_fifo` → `msgdma_copy_one`
- [Phase 6] `git describe HEAD`: v6.18.44; `make kernelversion`: 6.18.44
- [Phase 6] Buggy `memcpy((void __force *)hw_desc` confirmed at line 518
in local tree
- [Phase 6] `git merge-base --is-ancestor 94963138cce29
stable/linux-6.18.y`: NOT ancestor (exit 1)
- [Phase 6] `grep memcpy_toio drivers/dma/altera-msgdma.c`: no match in
local tree (fix absent)
- [Phase 7] `grep CONFIG_ALTERA_MSGDMA drivers/dma/Kconfig`: tristate,
depends on HAS_IOMEM
- [Phase 8] Struct layout verified: `control` is last field at offset 28
in 32-byte `msgdma_extended_desc`
**YES**
drivers/dma/altera-msgdma.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c
index a203fdd84950e..e1811a9be0bbe 100644
--- a/drivers/dma/altera-msgdma.c
+++ b/drivers/dma/altera-msgdma.c
@@ -498,6 +498,11 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
{
void __iomem *hw_desc = mdev->desc;
+ /* Ensure control is the last field — required for correct FIFO flush ordering */
+ static_assert(offsetof(struct msgdma_extended_desc, control) ==
+ sizeof(struct msgdma_extended_desc) - sizeof(u32),
+ "control must be the last field in msgdma_extended_desc");
+
/*
* Check if the DESC FIFO it not full. If its full, we need to wait
* for at least one entry to become free again
@@ -506,17 +511,18 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
MSGDMA_CSR_STAT_DESC_BUF_FULL)
mdelay(1);
+ /* Ensure control is the last field — required for correct FIFO flush ordering */
+ static_assert(offsetof(struct msgdma_extended_desc, control) ==
+ sizeof(struct msgdma_extended_desc) - sizeof(u32),
+ "control must be the last field in msgdma_extended_desc");
+
/*
- * The descriptor needs to get copied into the descriptor FIFO
- * of the DMA controller. The descriptor will get flushed to the
- * FIFO, once the last word (control word) is written. Since we
- * are not 100% sure that memcpy() writes all word in the "correct"
- * order (address from low to high) on all architectures, we make
- * sure this control word is written last by single coding it and
- * adding some write-barriers here.
+ * Copy the descriptor into the descriptor FIFO of the DMA controller,
+ * excluding the control word. The FIFO is flushed and the descriptor
+ * becomes valid once the control word is written last.
*/
- memcpy((void __force *)hw_desc, &desc->hw_desc,
- sizeof(desc->hw_desc) - sizeof(u32));
+ memcpy_toio(hw_desc, &desc->hw_desc,
+ offsetof(struct msgdma_extended_desc, control));
/* Write control word last to flush this descriptor into the FIFO */
mdev->idle = false;
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread