Skip to content

drm/asahi: pgtable: check the struct page of a table outside the reserved regions - #427

Merged
RyanTheTide merged 1 commit into
aurora-silicon:integration/aurorafrom
jgoyvaerts:m4pro/asahi-pgtable-checked-borrow
Oct 25, 2026
Merged

RyanTheTide merged 1 commit into
aurora-silicon:integration/aurorafrom
jgoyvaerts:m4pro/asahi-pgtable-checked-borrow

Conversation

@jgoyvaerts

Copy link
Copy Markdown
Contributor

The with_table point from the #408 review, which didn't go in with #420.

With reserved table regions (no-map systems such as the T6040), with_table reached a page-table page outside the two regions through Page::borrow_phys_unchecked(). That is undefined behaviour when the page is other no-map memory without a struct page. When reserved is set it now uses the checked Page::borrow_phys() and refuses such a page with -EIO. Systems without reserved regions keep the unchecked borrow of a page-table page they allocated, as before.

On the coherency question, the commit documents what holds and what is assumed:

  • Holds: the one clean and invalidate of the reserved regions runs after the boot, when the host has already written entries (fixed kernel objects are mapped before boot()). That's safe because the T6040 also sets UatQuirks::clean_tables, so every host table store is cleaned to the PoC as it's made, and the invalidate drops only clean lines.
  • Assumed: not invalidating again relies on the firmware no longer writing the tables the host walks once it has booted. We haven't proven that with a trace. Our J616s has run this way since its GPU came up, and we've never traced a fault to a stale table. The comment says so rather than presenting it as a fact.

The invalidate_reserved_tables() comment in mmu.rs said this ran "before the first host edit", which isn't true; it's corrected too.

As you said in the review, this path can go once the boot loader puts the ranges in /memory. Until then it's what the T6040 runs on.

Tested: on our J616s since 7.3.0-m4.17 (daily kernel jgoyvaerts/linux m4/aurora-7.3, now this commit on 420a1c8): GPU desktop, GL and Vulkan, no -EIO from this path. KUnit (asahi, QEMU) 120/120.

Base: integration/aurora (420a1c8f5). Built with W=1 (CONFIG_DRM_ASAHI=m, CONFIG_DRM_ASAHI_G16=y): no new warnings (the tip's own fetch_update deprecations and the unused MM static remain with our rustc 1.99); checkpatch --strict clean.

🤖 Generated with Claude Code

…rved regions

With reserved table regions (no-map systems such as the T6040), a
page-table page outside them went through Page::borrow_phys_unchecked(),
which is undefined behaviour when the page is other no-map memory
without a struct page. Use the checked borrow there and refuse such a
page with -EIO; systems without reserved regions keep the unchecked
borrow of an allocated page-table page.

Document the single cache invalidate of the reserved regions at boot.
It runs after the host has already written table entries, which is safe
because the T6040 cleans every host table store to the PoC
(UatQuirks::clean_tables), so the invalidate drops only clean lines. Not
invalidating again assumes the firmware stops writing the tables the
host walks once it has booted; say that this is an assumption. The
wrapper's comment claimed the invalidate came before the first host
edit, which it doesn't.

Link: aurora-silicon#408
Signed-off-by: Jonathan Goyvaerts <jonathan.goyvaerts@gmail.com>
Assisted-by: LLM

@RyanTheTide RyanTheTide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the reserved-region lookup, page lifetime and bounds, the checked struct-page fallback and the table-store cache cleaning. The ordinary allocated-page path stays unchanged. The comments now describe the boot ordering and the remaining firmware-write assumption accurately. The attribution check found no additional Gravity-derived implementation in this diff.

Checkpatch is clean. The combined tree passed the apple-dcp host tests, 63/63, and the Asahi KUnit tests, 86/86. The first defconfig plus asahi.config WERROR build passed. The shipping-config build and final fixup-tree builds are still running.

This shared-code change is not merge-ready until the approved M1 Pro and M2 one-shot checks pass. Both baseline kernels are recorded, including native headless Vulkan enumeration. The common config correction in #438 also needs review before the batch can land.

@RyanTheTide

Copy link
Copy Markdown
Contributor

I re-read the PR body, reviews and both comment lists. There are currently no inline review threads or new replies on this PR. The checked-borrow point is resolved at bec833e, and the coherency assumption is stated accurately. My current verdict remains not merge-ready because the required M1 Pro/M2 candidate-kernel one-shot checks and the final universal build are outstanding. The native Vulkan results recorded so far are baseline results on 73w. The #438 config dependency also still needs review.

@RyanTheTide

Copy link
Copy Markdown
Contributor

Final software gates now pass on the combined tree containing these heads and the proposed common config correction in #438. Both defconfig plus asahi.config and the Fedora-base recipe build Image, modules and DTBs with WERROR enabled. The Fedora recipe also covers KUNIT=m. Every requested asahi.config setting survives resolution. Asahi KUnit passes 86/86 and apple-dcp host tests pass 63/63. The targeted J713/J616s binding and DTB checks have no diagnostics.

The code review points are resolved. This remains not merge-ready until the M1 Pro/M2 candidate boots pass and #438 receives the required common-config review.

@RyanTheTide

Copy link
Copy Markdown
Contributor

M2 no-regression qualification now passes with this head in the combined source tree 031f7dec92a1bef6b72509b08c5c597f50474adb. The approved one-shot ran 7.3.0-aurora-t3028. A native Apple GPU compute/readback check verified all 4096 words, the checked kernel fault counters stayed at zero, and device counts, active services and SELinux enforcing matched the baseline. This uses the installed system Mesa to exercise the candidate kernel.

The machine is back on 73w, where the same check passes again. The test kernel, its GRUB entry and fallback units have been removed. The saved default, boot.bin and pinned DTB setting are unchanged.

The M1 Pro candidate check and the #438 common-config review are still outstanding, so this remains not merge-ready.

@RyanTheTide

Copy link
Copy Markdown
Contributor

Final qualification for bec833e. The combined test source is 031f7dec92a1bef6b72509b08c5c597f50474adb, tree 36854d3a72e4efdf2c352166498ab436adaf3c48.

Both defconfig plus asahi.config and the Fedora-base shipping recipe pass the full Image/modules/DTB build with WERROR enabled. Every requested fragment value survives olddefconfig. Asahi KUnit passes 86/86, apple-dcp host tests pass 63/63, and targeted J713/J616s binding/DTB checks have no diagnostics. Strict source-style checks pass; commit sign-offs are checked separately. The attribution audit found no new material Gravity implementation blocks. Existing GPL-2.0-only Cody Ho and Niklas Sheth credit is preserved.

The same candidate image passed approved M1 Pro and M2 one-shot smoke checks against fresh normal baselines. Native GPU compute/readback verified all 4096 words on each device, kernel fault counters stayed at zero, and baseline devices, active services and SELinux enforcing were preserved. Both machines are back on 73w with the checks passing again. Test kernels, task GRUB entries and fallback units are removed. The saved defaults, stock boot.bin files and pinned DTB setting are unchanged.

The checked physical-page borrowing point is resolved. No code hold remains. Not merge-ready yet because the tested common-config correction in #438 still needs the required review.

@RyanTheTide

Copy link
Copy Markdown
Contributor

No code hold left here.

Gates on integration/aurora 653c489 with #394, #435, #436, #446 (the carried #445), #427 and #428 merged (test tree 4582b72703d5), without #438:

  • Universal recipe (Fedora asahi base, asahi.config, olddefconfig, LLVM=1, CONFIG_DRM_WERROR=y): Image, modules and DTBs build with no warnings, and all 130 asahi.config settings survive.
  • KUnit asahi 84/84, drm/apple display set 212/212.
  • apple-dcp host selftests 62/63, the same as the base (dwc3-teardown fails there too).
  • dt_binding_check and CHECK_DTBS clean for the touched bindings and the J616s, J713, J700, M1 and M2 DTBs.

J616s (M4 Pro, 26.6.2 firmware): this kernel boots like the daily one. Wi-Fi, Bluetooth, audio devices, PMP, NVMe, cpufreq and cpuidle all match, with no DART faults or SErrors and no IRQ-safe genpd warnings. With the T6041 display handoff it also drives the internal panel. The Neo smoke boot of the same image is queued.

The #438 config review doesn't hold this up: these gates were built without #438.

@RyanTheTide
RyanTheTide merged commit 8bc8f12 into aurora-silicon:integration/aurora Oct 25, 2026
RyanTheTide added a commit that referenced this pull request Oct 25, 2026
Include the qualified M4 Pro/Max display and GPU identity updates from #435, #427, #428 and #446 on top of the existing consolidation. Regate this final tree before publication; earlier-head gate results do not qualify it.

Signed-off-by: Ryan Murray <ryan@aurorasilicon.org>

Assisted-by: LLM
@RyanTheTide

Copy link
Copy Markdown
Contributor

This PR is already present in integration/aurora and is superseded as an open merge candidate. The shared baseline was rechecked with the current dependency patches.

On integration/aurora at 421e314 with shared #438 and #456, DCP is PASS (63/63). KUnit has 905 passes and 1 policy skips. Universal WERROR is PASS; Fedora-base WERROR is PASS.

The extra Asahi M4 host suite remains 18/23 on the base and every candidate, with the same command-stream, golden, MMU-retention, UAPI and VM-registry failures. The frozen UAPI/model mismatch needs an independent audit before expectations change. G15 passes with two ignored cases. The merged path still needs the shared GPU/MMU, page-table activity, idle/recovery and M1 Pro/M2 hardware regression evidence, including the firmware-writer quiescence assumption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants