From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: gpu: nova-core: convert to new dma::Coherent API
Date: Wed, 04 Mar 2026 07:03:03 +1000
Message-ID: <review-patch8-20260303162314.94363-9-dakr@kernel.org>
In-Reply-To: <20260303162314.94363-9-dakr@kernel.org>
References: <20260303162314.94363-1-dakr@kernel.org> <20260303162314.94363-9-dakr@kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

This is the final cleanup patch converting all remaining `CoherentAllocatio=
n` usage.

**`from_data` safety scope**:
```rust
unsafe { dma_obj.as_mut()[..data.len()].copy_from_slice(data) };
```
The `unsafe` block wraps the entire expression, but only `as_mut()` is the =
unsafe operation. The slice indexing and `copy_from_slice` are safe. This i=
s a minor style issue =E2=80=94 narrower unsafe blocks are preferred in ker=
nel Rust.

**`falcon.rs` change**:
```rust
// Before:
fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?
// After:
fw.dma_handle() + DmaAddress::from(load_offsets.src_start)
```
The original used `dma_handle_with_offset` which performed bounds checking =
(returning `EINVAL` if out of range). The new code does raw arithmetic with=
out bounds checking. This is a subtle semantic change =E2=80=94 if `src_sta=
rt` were ever invalid, the old code would return an error while the new cod=
e would silently produce a wrong DMA address. Worth confirming this is inte=
ntional.

**`PteArray` adds `FromBytes`**:
```rust
/// SAFETY: arrays of `u64` implement `FromBytes` and we are but a wrapper =
around one.
unsafe impl<const NUM_ENTRIES: usize> FromBytes for PteArray<NUM_ENTRIES> {}
```
This is correct =E2=80=94 `[u64; N]` is trivially `FromBytes` and the `#[re=
pr(C)]` wrapper preserves this.

**`cmdq.rs` simplifications**: The replacements of `start_ptr()` =E2=86=92 =
`as_ptr()` and `start_ptr_mut()` =E2=86=92 `as_mut_ptr()` throughout are me=
chanical and correct. The removal of "CoherentAllocation contains exactly o=
ne object" from safety comments is appropriate since `Coherent<GspMem>` is =
inherently a single object.

---
Generated by Claude Code Patch Reviewer