Repository navigation
drm/asahi: g17: Add M5 GPU fault experiment parameters (on #1173) - #1243
rolotrealanis98 wants to merge 12 commits into
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
b1615e3 to
0dd0833
Compare
RyanTheTide
left a comment
There was a problem hiding this comment.
- This branch carries #1173 and #1242, so the items in those reviews apply here first. After that, push only 5b9ef7d on top of the updated #1242 branch. The rebase conflicts in
drivers/gpu/drm/asahi/g17/recovery.rsin the same way. - The comment on
M5_HW_PAUSE_TIMEOUT_USing17/fw/m5/initdata.rsquotes a firmware instruction and its TEXT offset. Please remove that and describe the word by what it does, as shown by measurement (for example, the timeout it produces with a given value). m5_pause_timeout_us,m5_idle_standby_usandm5_idle_holdare experiment knobs. Each one overrides a firmware timing or power default that is otherwise fixed per board. Please keep them out of this branch. If an experiment shows a better default, change the board value in its own commit, with the measurements in the commit message.- The failing
sourcejob (test_aurora_sep_sums_match_tree) comes from the old base, not from this diff.
…cannot build
From macOS 27.2 on, the M5 GPU firmware expects a different InitData
graph than the 26A428 (27.0) one the M5 path builds:
- the root version word gains bit 46 (0x0c8e_0339_8728_04c0 ->
0x0c8e_4339_8728_04c0), in 27.2 and 27.3 alike;
- the power policy grows from 0x126b0 to 0x12728 bytes and the status
config from 0x780 to 0x790, in 27.2 and 27.3 alike;
- 27.3 beta 1 alone inserts a HW data word at 0x2720.
These layouts are not yet confirmed from captured InitData of a
running macOS; the later commits of this series say which parts a
running firmware has accepted.
Name the layouts a build needs (Variant::m5_layouts) and give the 27.2
firmware its own variant, V26B123_M5: the 26A428 M5 protocol with the
26B123 layouts. Until the M5 graph builds them, Gpu::new() refuses such
a build right after identifying it, before anything is written to the
GPU, instead of handing it a graph it would reject ("Host and FW
Interface is mismatched") or misread.
Every M5 build in the tree today is 26A428 and names
M5Layouts::V26A428, so nothing changes for the M5 Max and M5 Ultra.
Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com>
Assisted-by: LLM Claude
…uilds The M5 Pro admits only the 27.0 (26A428) G17S build. Pin the 27.2 (26B123) and 27.3 beta 1 (26C5035r) G17S builds of both roles as well, measured the way fwid defines a build (the SHA-256 of the primary's then the secondary's TEXT with the entropy records, the two header load fields and the two boot-argument sections of each role zeroed): 27.3 beta 1 (26C5035r) b5537c8f06bd...66763600 27.2 (26B123) 2b4613c62a8a...913f0fd9 The images are the G17S (FW Build variant a010) primary and secondary GFX firmware of the public Mac17,8 IPSW for 27.2 and of the 27.3 beta 1 restore bundle (im4p SHA-256 ae7f35b1... and da8ff3fb...). The offsets come from each image's Mach-O load commands: text at 0x50000 + 0x54000, DATA 0x184000 as on the M5 Max, the _rtk_tunables and _rtk_patchbay sections per role. 27.2 needs an entry of its own although its interface matches 27.3's: its primary entropy record sits at 0x49ea4, 27.3's at 0x49eac. Each image header still carries its load fields at 0x224/0x23c (zero in the file, the patchbay's offset and size at 0x22c/0x230, as in 26A428). The 27.0 entry is the one the base already pins: this digest procedure gives the same digest for it, and reproduces the two pinned 26A428 digests of the M5 Max (G17C) and M5 Ultra (G17D). 27.0 runs the 26A428 graph. 27.2 and, for now, 27.3 name V26B123_M5, whose layouts the M5 graph does not build yet, so they are identified and then refused before any GPU write; 27.3 gets its own variant once its HW data is built. The fwid KUnit tests cover the new builds. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
The M5 GPU firmware of macOS 27.2 (26B123) is expected to find a different root version word than 27.0 (26A428): 0x0c8e_4339_8728_04c0 where 27.0 has 0x0c8e_0339_8728_04c0, only bit 46 differs. Its allocation size table differs from 26A428's (ABI_ALLOCATION_SIZES_26A428_M5) in two entries only: index 4, power policy: 0x126b0 -> 0x12728 index 13, status config: 0x780 -> 0x790 The packed layouts derived from it move accordingly: in the shared cluster everything after the policy moves by 0x80 (views 3 and 4, the BRN table and repeated region, the twelve DM rings, both main configs and control rings); in the private cluster everything after the status config moves by 0x40 (both role sets with status A, the secondary control state, secondary extra 1 and the policy state). Both clusters keep their page-rounded sizes (0x34000 and 0x178000), so the fixed windows of the M5 graph still hold them. The HW data of 26B123 is 26A428's. The version word, the sizes and the HW data layout of 26B123 are not yet confirmed from captured InitData. Describe one firmware build's InitData layout as an M5Layout (its size table, the packed clusters derived from it, and how its HW data moves relative to the 26A428 encoding), add M5_LAYOUT_26A428 and M5_LAYOUT_26B123, and a RootProfile J714G17S26B123 that writes the new version word and takes its offsets from the 26B123 layout. build_m5() now takes the firmware build's Variant and builds the profile its M5Layouts names. Gpu::new() keeps refusing every build but 26A428 until the 26B123 power policy and status config are laid out as well (later in this series), so no firmware is handed a partly converted graph. The 26A428 encodings are unchanged: the HW data of every board, the private cluster, the bundle policy, both roots and both main configs encode to the same bytes as before. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
The M5 firmware of macOS 27.3 beta 1 (26C5035r) takes the 26B123 version word and allocation sizes, but its HW data is not 26B123's: one u32 is inserted at 0x2720 and every later field moves by 4 (the kick-shared pointer from 0x27dc to 0x27e0, the last moved word from 0x2846 to 0x284a; everything below 0x2720 is as before). The version word does not tell the two builds apart; only the firmware identity does. The new word is the time the GPU may take to reach a forced idle after a sleep request, in ms; 0 disables the wait. The value written, 5000, is 5 s times 1000. The layout, the meaning of the word and the value are not yet confirmed from captured InitData. Describe the move with the layout bands the HW data already uses (M5_26C5035_HW_DATA_BANDS: 0..0x2720 stays, 0x2720..0x284c moves by 4; the four bytes it drops past 0x284c are zero in the 26A428 encoding), apply them after the 26A428 encoding and write the word, and find the kick-shared pointer and the enabled-core words through them. Add RootProfile J714G17S26C5035, M5Layouts::V26C5035 and its Variant V26C5035_M5, and have the 27.3 (26C5035r) G17S build of the M5 Pro name it instead of V26B123_M5. The fwid KUnit test that checks each M5 Pro build's layouts follows. The encoded 26C5035r HW data of every board equals the 26A428 encoding below 0x2720, has 5000 at 0x2720 and carries the 26A428 bytes 0x2720..0x284c at 0x2724. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
The 26B123 M5 firmware takes a 0x12728-byte power policy instead of
0x126b0 (allocation size table entry 4); 27.3 beta 1 (26C5035r) has
the same layout. Five inserts, as bands in 26A428 offsets, move the
words above them:
[0x0000, 0x4910) +0
[0x4910, 0x491c) +0x10 two u64 words; the three controller input
words at 0x4910 (3500/1000/3000 in the J714
defaults) move to 0x4920
[0x491c, 0x49b8) +0x14 a controller input word at 0x492c
[0x49b8, 0x4a98) +0x18 4 bytes between 0x494c and 0x49b8 that the
M5 encoders do not write
[0x4a98, 0x12668) +0x30 0x18 bytes between 0x4a00 and 0x4a98 that
they do not write either
[0x12668, 0x126b0) +0x78 the table at +0x122f0 grows from 37 to 40
24-byte entries
The state and AON PTD pointers move to 0x8284 and 0x11a4c. The 26B123
policy may use words of its own at 0x94e4..0x954c, in space the M5
encoders do not write in 26A428; they stay zero here.
The policy controller input words the 26B123 policy carries beyond the
26A428 ones are 0x492c = 5000 and 0x4a14 = 1600 (26A428 0x49fc, 500 in
the J714 defaults) for the core types 0x18/0x19, and for the G17D core
type 0x1a 0x4920 = 2500, 0x4928 = 800, 0x492c = 1100, 0x49e4 = 0 and
0x4a14 = 1600. These values and the bands above are not yet confirmed
from captured InitData; they are what a capture session has to check.
Add M5PolicyMap with the 26A428 identity map and the 26B123 map, chosen
from the build's M5Layouts (every layout but 26A428 takes the 26B123
map), and relayout_m5_policy(), which moves the bands of the policy the
26A428 encoders wrote in place, highest first, zeroes the inserts and,
where the board's controller defaults were applied, writes the 26B123
controller words for its core type. For 26A428 it changes nothing.
Not run on hardware: the controller words (they are written only where
the board's controller defaults apply, and no run of this layout has
had them) and the G17D (J775D) words on 26B123; no 27.2+ M5 Max or
M5 Ultra firmware is identified yet.
Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com>
Assisted-by: LLM Claude
The M5 graph's policy encoders (encode_m5_bundle_policy() and the boards' controller defaults) write the 26A428 layout. Keep the build's policy map in InitData, set through set_m5_policy_layouts() from the identified firmware's M5Layouts, and lay the policy out for it in build_m5() once the controller defaults have run: the 26B123 layout moves every word above 0x4910 and adds the 26B123 controller words where the board's defaults applied. The maximum-power word read for region C (0xa18) does not move. The relayout stays within the build's own policy allocation: a policy larger than the space before block_68 of the shared layout is refused. The diagnostic policy read is bounded by the build's policy size. The map defaults to 26A428, so the 26A428 graph is unchanged. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
The 26B123 M5 firmware takes a 0x790-byte status config instead of 0x780 (allocation size table entry 13); 27.3 beta 1 (26C5035r) has the same layout. Everything up to +0x5b4 keeps its offset. From +0x5b4 the object holds two controller blocks, at +0x5b4 and +0x610, of 0x5c bytes each. 26B123 inserts two words into each block, an override slot at block +0x28 and its value at block +0x58, so a block grows to 0x64 bytes, and everything after the second block moves by 0x10: the link word and pointer (0x774/0x778 -> 0x784/0x788) and the submission history (0x73c/0x74c/0x750 -> 0x74c/0x75c/0x760). As bands in 26A428 offsets: [0, 0x5dc) +0, [0x5dc, 0x608) +4, [0x608, 0x638) +8, [0x638, 0x664) +0xc, [0x664, 0x780) +0x10. The new words are 0x5dc and 0x640 (override slots, zero-filled) and 0x60c (5000) and 0x670 (0), the values. The layout and these values are not yet confirmed from captured InitData. Add M5StatusConfigMap with the 26A428 identity map and the 26B123 map, encode the primary status-B span through it, and read the submission history through the map the graph was built with. The 26A428 status config is unchanged: the map path writes the same 0x780 bytes as write_relocated_words() with COMMON26_STATUS_CONFIG_BANDS. The 26A428 words the driver writes from 0x5a4 are not changed here; the 26B123 map moves them like every other word. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
Give RootProfile the status-config map of its M5 graph and have build_m5() pick the profile before it encodes the private cluster, so the status-B span, the link word and pointer and the submission-history reader all follow the profile's layout. Every existing profile keeps the 26A428 map, so the graph is unchanged. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
…3 profiles The 26B123 (macOS 27.2) firmware takes the 0x790-byte status config: two words more in each controller block and the tail, link word and submission history 0x10 higher. Return M5_26B123_STATUS_CONFIG for the 26B123 root profile, and for the 26C5035 (macOS 27.3 beta 1) profile, whose status config is the same as 26B123's. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
…e firmware build build_m5() now selects the power-policy map from the same M5 layouts the root profile comes from, so a 26B123 or 26C5035 firmware gets its policy laid out to 0x12728 bytes and its 0x790-byte status config in the same graph. The policy and status-config sizes of 26B123 are entries 4 and 13 of its allocation size table, and a compile-time check holds every M5 build's policy and status-config maps to its own size table, with the policy at the offset the controller defaults write it. The controller defaults keep writing the 26A428 policy, which the relayout then moves; name that slice once. The private cluster encoder takes the root profile and derives both the cluster layout and the status-config map from it, which leaves the 26A428-only status-B wrapper without callers. With every 27.2+ layout in place, Gpu::new() now refuses only a build that names no M5 layouts, so the M5 Pro's 27.2 (26B123) and 27.3 beta 1 (26C5035r) builds run their graphs. 26A428 builds, the only ones the M5 Max and M5 Ultra identify, take the identity maps and build the same bytes as before this series. The 27.2+ layouts and values of this series are not yet confirmed from captured InitData; the comments say which. What has been observed is the following. An earlier revision of this series, with J716s board tables and a native-address mailbox compatible that are not part of this one, ran on a J716s (16-inch M5 Pro) with the 27.3 beta 1 boot firmware, a device tree that describes the GPU and a loader that hands it over (none of them part of this series): aurora 2300000000.gpu: GPU firmware: 27.3 (26C5035r), G17S aurora 2300000000.gpu: RTKit: Initializing (protocol version 12) aurora 2300000000.gpu: M5 primary initdata acknowledged aurora 2300000000.gpu: M5 secondary initdata acknowledged aurora 2300000000.gpu: M5 firmware opening records retired [drm] Initialized aurora 1.0.0 for 2300000000.gpu on minor 1 with no coprocessor crash, SError, DART or UAT fault for the minutes the system ran, and later through a 25-minute desktop session on the GPU with no GPU errors. That shows the firmware accepting a graph built from these layouts; it does not confirm any single value. Not exercised: the 27.2 firmware on hardware, the forced-idle sleep path, the policy controller words (that board wrote no controller defaults), the M5 Max and M5 Ultra on 27.2+. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
On M5 every firmware recovery logs "reason None slot_mask ... sampled 0 known_keys 0": the driver reads only the slot mask of the firmware's recovery record, so a recovery never says why the firmware stopped or whose work was running. With several GPU clients that leaves nothing to act on. The rest of that record is in the primary status B: the reason at +0x4aac, the number of busy slots walked at +0x4aa0, a word describing the slot the record names at +0x4aa8 and a u64 detail at +0x4ab0 (a page fault leaves its latched MMU fault status there). Only the reason has been read from a running firmware so far: on a J716s with the 27.3 beta 1 firmware, all three recoveries of a ten-minute mixed client load carried reason 5, a scheduling pause that did not complete in time, and the firmware blamed no queue. The other words, and the meaning of reasons 3 (page fault) and 6 (forced idle after a sleep request), are not yet confirmed from captured InitData. Read the record with the slot mask and log it next to the snapshot, with the reason's meaning and the queue the firmware's restart event blames. After the handshake, name the client (process name and tgid) and the retained command count of every queue the recovery table lists and of the blamed queue. The record changes no blame by default: attribution on M5 keeps its unknown reason. The module parameter m5_recovery_reason_blame=1 hands the firmware reason to attribution as on G17P, so a timeout can move the firmware's blame to the VM whose render was running. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
On a J716s, M5 GPU clients trigger firmware recoveries with no MMU fault. Under a ten-minute mixed client load with the HW data word at +0x271c at its encoded value, 150 (the inherited fixed word of the 26A428 HW data, moved by the M5 insert), the GPU recovered three times, and all three recoveries carried firmware reason 5, a scheduling pause that timed out. With 2000 written to the word, the same load produced no recovery in ten minutes. One run each, so the word is shown to bound the pause but 2000 is not shown to be a better default. Name the word (M5_HW_PAUSE_TIMEOUT_US), describe it by that measurement, and hold the offset to the encoded fixed word with a compile-time check. Nothing else changes: the board value stays 150. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
5b9ef7d to
490c980
Compare
|
Updated (490c980, one commit on the new #1242 head e2192e0).
That leaves only the named constant and its offset check, and the board value stays 150: one run each does not show 2000 to be a better default. If that is too little for a PR of its own, I will fold the constant into #1242 and close this one. |
|
Closing: with the experiment knobs removed, what was left is one named constant, which is now the second commit of #1242. |
What this is
Stacked on #1173 (M5 InitData for the 27.2/27.3 firmware; base
funkzy/t6050-gpu-on-integration), with the recovery-cause commit of the companion PR "Name the cause and the clients of every M5 GPU recovery" underneath it, so the diff shows #1173's 13 commits + those 2; only the top commit is new here. It needs #1173'shw_data_offset()(the 27.3 HW data band relocation) for the first parameter.Three firmware policies the driver encodes for M5 become boot parameters of the
auroramodule, all off by default; nothing changes without them:m5_pause_timeout_us: the HW data word at +0x271c bounds how long the firmware waits for the hardware to acknowledge a scheduling (DSF) pause before it raises recovery reason 5. The firmware reads it live each time it pauses (26B123 primary TEXT 0x231d0, poll at 24 ticks per us, "Unexpected scheduling pause timeout after %uus"). The M5 encoding holds the inherited fixed word, 150 us, there (COMMON26_HW_FIXED_WORD_OFFSETmoved by the M5 HW data insert; a compile-time assertion pins the offset). A nonzero value replaces it;0xffffffffwaits without a bound. The default stays 150 us, which is also what macOS writes (checked in the 27.0, 27.2 and 27.3 AGXG17X kexts: a constant store to +0x271c, no variant branch).m5_idle_standby_us: Region C +0x7c8, how long the idle GPU waits before standby (image default 1500 us), clamped to 100..100000.m5_idle_hold: M5 takes no host idle hold today. With 1, theidle_off_delay_mssysfs attribute drives the hold the other G17 parts use, so above 2 ms the cores stay powered until the GPU has been idle that long, changeable at runtime.Why
Ordinary M5 GPU clients (a Qt Quick bar, Chromium, a compositor next to one of them) trigger firmware recoveries with no MMU fault, every few minutes under load and a couple of times an hour in normal use on the J716s (MacBook Pro 16-inch, M5 Pro). The firmware's own record (companion PR) says why: reason 5, "DSF pause timeout", the hardware did not acknowledge a scheduling pause within 150 us. Pauses come from power kicks, compute context stores and buffer-manager rebinds, not from load as such, so an almost idle second client is enough. Testing each candidate policy needed a rebuild; these make them bootargs. Tracker: rolotrealanis98/m4-m5-linux#36; the analysis with the firmware and kext reading: faults/HYPOTHESES.md (H1, H2).
Hardware result (J716s, G17S, firmware 27.3 beta 1 26C5035r)
Kernel 7.3.0-g12c445fa5371 (our J716s tree: integration/aurora 33bec40 + #1012 + #1173 + the J716s DCP/board series, with this commit, the cause-log commit and the three recovery fixes). Same 10 min load twice (3x vkcube, glmark2-es2-wayland, two Qt Quick windows on the GPU backend, a Chromium WebGL tab), Hyprland on zink at 3456x2234@120:
aurora.m5_pause_timeout_usfirmware scheduling pause timeout 2000 usat boot)m5_idle_standby_usandm5_idle_holdwere not exercised on hardware. The code notes that on the M5 Ultra (J775D) a host hold makes the core-power witness read unpowered;m5_idle_hold=1has not been tried on any board, which is why it is a parameter and not a change.Checks
make W=1 CLIPPY=1 drivers/gpu/drm/asahi/(LLVM arm64, defconfig + asahi.config, rustc 1.99.1, clang 21) at the base and at each commit, objects rebuilt from scratch: see the Checks line below.checkpatch.pl --strict: 0 errors, 4 warnings (three module-parameter description strings over 100 columns, as the ten description linesasahi_core.rsalready has; and theAssisted-by:trailer format, as on every commit in drm/asahi: M5: InitData for the macOS 27.2 and 27.3 firmware (G17S bring-up on J716s) #1173).COMMON26_HW_FIXED_WORD_OFFSET + M5_HW_DATA_INSERT_LEN == 0x271cguards the offset against a future relayout.Compile check result:
drivers/gpu/drm/asahi/+drivers/gpu/drm/apple/withW=1 CLIPPY=1give the same 53 warnings at #1173's head (34f2f5b), at the cause-log commit and at this commit; none new, none gone (all pre-existing clippy and deprecation notes).Risks
m5_pause_timeout_usset, the firmware waits longer for a pause acknowledge before it recovers;0xffffffffleaves only the driver's own 10 s timeouts to catch a stuck pause. Withm5_idle_hold=1on the J775D the core-power witness may read unpowered (see the code note); set the delay back to 2 if submissions stall.build_m5) and inidle_hold(); no runtime cost otherwise.Questions
Board::pause_timeout_us) for J716s instead of a parameter?