Repository navigation
drm/asahi: pgtable: check the struct page of a table outside the reserved regions - #427
Conversation
…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
left a comment
There was a problem hiding this comment.
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.
|
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. |
|
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. |
|
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. |
|
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. |
|
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:
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. |
|
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. |
The
with_tablepoint from the #408 review, which didn't go in with #420.With reserved table regions (no-map systems such as the T6040),
with_tablereached a page-table page outside the two regions throughPage::borrow_phys_unchecked(). That is undefined behaviour when the page is other no-map memory without a struct page. Whenreservedis set it now uses the checkedPage::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:
boot()). That's safe because the T6040 also setsUatQuirks::clean_tables, so every host table store is cleaned to the PoC as it's made, and the invalidate drops only clean lines.The
invalidate_reserved_tables()comment inmmu.rssaid 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/linuxm4/aurora-7.3, now this commit on 420a1c8): GPU desktop, GL and Vulkan, no-EIOfrom 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 ownfetch_updatedeprecations and the unusedMMstatic remain with our rustc 1.99); checkpatch --strict clean.🤖 Generated with Claude Code