Do you have any plan to upload this patch to https://review.coreboot.org/ ? At least I can vote +1 for it as I have confirmed it working.
On Thursday, October 1st, 2026 at 12:24 PM, Sebastian Müller <[email protected]> wrote: > Hi, > I think I found the actual problem, and it does not seem to be in > libgfxinit. That would also fit with the X301 test, where changing the > libgfxinit revision did not make any difference. > > In src/lib/hw-time-timer.adb the monotonic timer is imported like this: > > procedure Timer_Monotonic_Get (MT : out Interfaces.C.long); > pragma Import (C, Timer_Monotonic_Get, "timer_monotonic_get"); > > The C side now uses: > > struct mono_time { > uint64_t microseconds; > }; > > and timer_monotonic_get() eventually does: > > *mt = mono_counter.time; > > So the C function writes 8 bytes. You can also see that directly in the > disassembly: > > mov %eax,(%ebx) > mov %edx,0x4(%ebx) > > The problem is that on a 32-bit ramstage, Interfaces.C.long is only 4 > bytes. GNAT allocates a 4-byte temporary, but the C function writes 8 > bytes into it. > > As far as I can tell, the Ada declaration was correct when it was > originally added. The mismatch appeared later with commit b11f9f7e16 > (timer: Switch mono_time to uint64_t, 2022-08-13), which changed the C > side to 64 bit without changing the Ada import. > > Without LTO this seems to work mostly by accident. Raw_Value_Min stays as > a separate function, has a fairly large stack frame, and the extra four > bytes happen to overwrite unused stack space. > > With LTO and GCC 15.2.0 the situation changes. The timer code gets > inlined, and in HW.Time.Ms_From_Now, HW.Time.Us_From_Now and > GMA.Panel.Wait_On the temporary ends up at 0xc(%esp) in a 16-byte stack > frame. > > That means the upper four bytes written by timer_monotonic_get() overwrite > the saved %ebx. > > When the function returns and restores %ebx, it restores the upper half of > the timer value instead of the original register value. > > That seems to be exactly what breaks the display probe. > > In the inlined Probe_Port code inside gma_gfxinit: > > movzbl %al,%ebx > cmpb $0x0,0x4036320(%ebx) > mov %ebx,%eax > call to_panel > movzbl %al,%eax > call panel__wait_on.part.0 > mov %ebx,%edx > call display_probing__read_edid > > %ebx holds the port number. > > After Panel.Wait_On, %ebx is no longer the port. During early boot the > upper 32 bits of the microsecond counter are still zero, so %ebx > effectively becomes 0, which is Disabled. > > Read_EDID then subtracts one from the port, giving 0xff. The range check > fails, To_Display_Type falls through to its default case and returns DP, > and the DP path ends up using DP_Port'First, which is AUX channel A. > > That explains why it tries DDI_AUX_CTL_A instead of GMBUS pin 3, then > times out and eventually calls Panel.Off. > > It also explains why only the eighth probe is affected. > > For DP1-3, HDMI1-3 and VGA, To_Panel returns No_Panel, so Panel.Wait_On > exits before it reaches the timer code. The internal panel is the only one > that actually goes through the broken path. > > The fact that the X301 behaves the same way also makes sense then, because > this is shared timer code rather than something specific to one graphics > generation. > > The fix is simply to make the Ada declaration match the C type: > > -- C declares this as `struct mono_time { uint64_t microseconds; }'. > procedure Timer_Monotonic_Get (MT : out Word64); > > With that change, the generated code looks sane again. Raw_Value_Min is > called out of line, the 64-bit value comes back in eax:edx, and there is > no 4-byte temporary on the stack for the C function to overwrite. > > It also builds cleanly here for the hp/2170p with GCC 15.2.0 and LTO > enabled. > > As a small side effect, ramstage is about 500 bytes smaller with LTO and > around 240 bytes smaller without LTO. > > I cannot test the actual boot because I do not have a 2170p or X301 here. > The generated code makes the corruption pretty clear though. > > Could you test the patch on both machines with LTO enabled? That should > tell us whether this is indeed the bug you are seeing. > _______________________________________________ > coreboot mailing list -- [email protected] > To unsubscribe send an email to [email protected] > _______________________________________________ coreboot mailing list -- [email protected] To unsubscribe send an email to [email protected]
