[PATCH 2/2] efi_loader: Fix kernel panic when FTRACE is enabled

[Heinrich Schuchardt]

On 12/20/25 14:52, Patrick Rudolph wrote:
> On Fri, Dec 19, 2025 at 9:44 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 12/17/25 11:39, Ilias Apalodimas wrote:
>>> Thanks Patrick,
>>>
>>> On Wed, 17 Dec 2025 at 09:26, Patrick Rudolph
>>> <patrick.rudolph at 9elements.com> wrote:
>>>>
>>>> When FTRACE is enabled the EFI runtime will call the tracing functions
>>>> that are not marked as __efi_runtime and cause a kernel panic since
>>>> the functions have been previously unmapped by the kernel.
>>>>
>>>> Resolve this issue by adding the 'notrace' attribute to '__efi_runtime'
>>>> attribute and thus disable tracing the EFI runtime. If required this
>>>> can be worked out latter.
>>
>> What is the point of actually booting with a U-Boot binary with FTRACE
>> enabled? All your trace data will be lost.
>>
>> Wouldn't it be preferable to error out at ExitBootServices() and
>> retrieve the trace?
>>
>> The current implementation of FTRACE looks flawed. The trace buffer
>> should be allocated from LMB so that it cannot overwrite images or EFI data.
>>
> Hi Heinrich,
> I'm not that familiar with the tracing yet. My expectation was that
> the trace buffer is marked as reserved
> and could be retrieved from /dev/mem, however that's not the case.
> I updated the CONFIG_BOOTCOMMAND to write the trace file to disk
> before it runs "bootflow scan"
> as described in the documentation. I can then retrieve the trace data
> from disk. That works for me.
> 
> Regards,
> Patrick Rudolph

Trace logic
===========

The logic introduced by commit 852d4dbd70ba ("trace: Detect an infinite 
loop") just switches of tracing when __cyg_profile_func_enter() is 
invoked with hdr->trace_locked=true. This lets us end up with having to 
mark dozens of functions as notrace because they are invoked by 
add_ftrace().

A better approach than switching of tracing is to simply return without 
recording the entry by adding

         if (!hdr || hdr->trace_locked)
                 return;

at the start of __cyg_profile_func_enter() and __cyg_profile_func_exit().

Once the outermost __cyg_profile_func_enter() is finished it will unlock 
the trace.

Missing relocation
==================

Unfortunately there are more bugs that need to be solved to make tracing 
work properly.

On qemu-riscv64_smode_defconfig I found that there is also an issue in 
notify_dynamic() which is invoked when searching for the timer driver.

When looping over state->spy_head, list_for_each_entry_safe() does not 
discover the end of the list and loops forever though the list is empty.

The reason is that the address of state->spy_head has been relocated but 
state->spy_head.next and state>spy_head.prev still point to the 
unrelocated address. Therefore list_for_entry_safe() does not discover 
the end of the list.

We should not start tracing before initr_reloc().

When placing INITCALL(initr_trace) directly after INITCALL(initr_reloc) 
the timer driver panics with

     Could not initialize timer (err -11).

In initcall_run_r() we need to place INITCALL(initr_trace) after 
INITCALL(initr_dm) to let the timer driver work properly.


Defaults
========

The current defaults for tracing are not helpful. The observed call 
depth for `bootefi hello` is 42. The default of 16 MiB as trace buffer 
is way too low to catch a boot process. Rather think of 1-2 GiB.

Best regards

Heinrich>

>>>
>>> So I agree with this. Fixing is just a matter of moving the missing
>>> functions to the runtime section, but I don;t think it's worth it,
>>> unless someone *really* wants to instrument runtime calls.
>>> OTOH runtime calls in the OS are rare and very limited
>>>
>>> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>>
>>>
>>>> TEST=Can boot a UEFI compatible linux OS on qemu-q35 when U-Boot was
>>>>        built with FTRACE=1.
>>>>
>>>> Signed-off-by: Patrick Rudolph <patrick.rudolph at 9elements.com>
>>>> ---
>>>>    include/efi_loader.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>> index 3e70ac07055..7a13c390cbe 100644
>>>> --- a/include/efi_loader.h
>>>> +++ b/include/efi_loader.h
>>>> @@ -66,7 +66,7 @@ struct bootflow;
>>>>     *
>>>>     *   static __efi_runtime compute_my_table(void);
>>>>     */
>>>> -#define __efi_runtime __section(".text.efi_runtime")
>>>> +#define __efi_runtime __section(".text.efi_runtime") notrace
>>>>
>>>>    /*
>>>>     * Call this with mmio_ptr as the _pointer_ to a pointer to an MMIO region
>>>> --
>>>> 2.52.0
>>>>
>>