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.
From bcd1cd9209bdfce283f7d31e2a8b1fc99cce8b37 Mon Sep 17 00:00:00 2001
From: Sebastian <[email protected]>
Date: Thu, 1 Oct 2026 13:33:05 +0200
Subject: [PATCH] lib/hw-time-timer: Fix type of timer_monotonic_get out
parameter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The Ada import declares the out parameter as Interfaces.C.long, which
is 32-bit on a 32-bit ramstage, while C has
struct mono_time { uint64_t microseconds; };
and timer_monotonic_get() ends in `*mt = mono_counter.time;', an 8-byte
store. GNAT allocates a 4-byte temporary for the out parameter, so the
callee writes 4 bytes past it, and the upper half of the counter is
dropped and the result sign-extended.
The declaration was correct when it was added; commit b11f9f7e16
("timer: Switch mono_time to uint64_t") widened the C field without
updating the import.
Without LTO this is mostly benign: Raw_Value_Min is a standalone
function whose frame has room, so the extra 4 bytes land on unused
stack. With LTO the function is inlined into its callers, and in
HW.Time.Ms_From_Now, HW.Time.Us_From_Now and GMA.Panel.Wait_On the
temporary sits at the top of a tight frame, so the write overwrites the
saved %ebx. On GCC 15.2.0 this breaks libgfxinit on 32-bit ramstage:
in the inlined Probe_Port, %ebx holds the port being probed, so after
Panel.Wait_On the internal panel is passed to Read_EDID as Disabled,
To_Display_Type falls into its default arm and returns DP, and the
panel is probed over AUX channel A instead of GMBUS. Reported on
hp/2170p and lenovo/x301.
Declare the parameter as Word64 to match the C side.
Signed-off-by: Sebastian Müller <[email protected]>
---
src/lib/hw-time-timer.adb | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/lib/hw-time-timer.adb b/src/lib/hw-time-timer.adb
index 1d1e5d4c4c..7aa699afe8 100644
--- a/src/lib/hw-time-timer.adb
+++ b/src/lib/hw-time-timer.adb
@@ -1,20 +1,19 @@
-- SPDX-License-Identifier: GPL-2.0-only
-with Interfaces.C;
-
package body HW.Time.Timer
with Refined_State => (Timer_State => null,
Abstract_Time => null)
is
- procedure Timer_Monotonic_Get (MT : out Interfaces.C.long);
+ -- C declares this as `struct mono_time { uint64_t microseconds; }'.
+ procedure Timer_Monotonic_Get (MT : out Word64);
pragma Import (C, Timer_Monotonic_Get, "timer_monotonic_get");
function Raw_Value_Min return T
with
SPARK_Mode => Off
is
- Microseconds : Interfaces.C.long;
+ Microseconds : Word64;
begin
Timer_Monotonic_Get (Microseconds);
return T (Microseconds);
--
2.53.0
_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]
