[U-Boot] [PATCH] efi_loader: allow efi_loader code to call EFI interfaces

Tue Jul 25 19:12:43 UTC 2017

On 07/25/2017 07:52 PM, Rob Clark wrote:
> On Tue, Jul 25, 2017 at 12:56 PM, Heinrich Schuchardt
> <xypron.glpk at gmx.de> wrote:
>> On 07/25/2017 04:28 PM, Alexander Graf wrote:
>>>
>>>
>>> On 25.07.17 14:28, Rob Clark wrote:
>>>> On Tue, Jul 25, 2017 at 8:16 AM, Alexander Graf <agraf at suse.de> wrote:
>>>>>
>>>>>
>>>>> On 25.07.17 14:08, Rob Clark wrote:
>>>>>>
>>>>>> On Tue, Jul 25, 2017 at 7:17 AM, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 25.07.17 13:10, Rob Clark wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jul 25, 2017 at 4:10 AM, Alexander Graf <agraf at suse.de>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 25.07.17 01:47, Rob Clark wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> To implement efi_load_image() for the case of loading an image
>>>>>>>>>> from a
>>>>>>>>>> device path rather than image already loaded into source_buffer,
>>>>>>>>>> it is
>>>>>>>>>> convenient to be able to re-use simple-file-system and efi-file
>>>>>>>>>> interfaces.  But these are already using EFI_ENTER()/EFI_EXIT().
>>>>>>>>>> Allow
>>>>>>>>>> entry into EFI interfaces to be recursive, since this greatly
>>>>>>>>>> simplifies
>>>>>>>>>> things.
>>>>>>>>>>
>>>>>>>>>> (And hypothetically this would be needed anyways to allow grub
>>>>>>>>>> to call
>>>>>>>>>> into interfaces installed by something outside of grub.)
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> So there are 2 ways to do this:
>>>>>>>>>
>>>>>>>>>      1) Keep a refcount, only transition when passing the 0 level
>>>>>>>>>      2) Explicitly split functions with ENTRY/EXIT from their core
>>>>>>>>> functions
>>>>>>>>>
>>>>>>>>> So far we used approach 2, splitting functions that are used by both
>>>>>>>>> internal and external code into _ext (for externally called) and
>>>>>>>>> normal
>>>>>>>>> functions. You can see this pattern quite a few times throughout
>>>>>>>>> efi_loader.
>>>>>>>>>
>>>>>>>>> I definitely did try the refcount approach back when I decided for
>>>>>>>>> approach
>>>>>>>>> 2 and it failed on me in some case, but I can't remember where.
>>>>>>>>>
>>>>>>>>> Either way, we should definitely be consistent. Either we always use
>>>>>>>>> refcounting or we shouldn't bother with it. So if you can make a
>>>>>>>>> version
>>>>>>>>> work where all _ext variants disappear and we're magically
>>>>>>>>> reentrant,
>>>>>>>>> I'll
>>>>>>>>> be happy to take that. I'm fairly sure it'll break somewhere
>>>>>>>>> though :).
>>>>>>>>>
>>>>>>>>
>>>>>>>> for load_image via file-path, we end up needing a *bunch* of
>>>>>>>> functions
>>>>>>>> normally called via EFI.. so it is going to be a lot more _ext
>>>>>>>> variants.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Yes, but imagine a case like this:
>>>>>>>
>>>>>>> open_protocol
>>>>>>>     -> open()
>>>>>>>       -> random_junk()
>>>>>>>         -> efi_timer_check()
>>>>>>>           -> timer notifier
>>>>>>>             -> console print()
>>>>>>>
>>>>>>> Here the refcounting will simply fail on us, as the timer notifier
>>>>>>> needs
>>>>>>> to
>>>>>>> be run in UEFI context while efi_timer_check() as well as console
>>>>>>> print()
>>>>>>> need to be run in U-Boot context.
>>>>>>>
>>>>>>> And if we start to mix and mesh the 2 approaches, I can promise you
>>>>>>> that
>>>>>>> 2
>>>>>>> weeks down some corner case nobody thought of falls apart because
>>>>>>> people
>>>>>>> don't manage to fully grasp the code flow anymore.
>>>>>>>
>>>>>>
>>>>>> ugg.. gd is a gd pita ;-)
>>>>>>
>>>>>> how many places do we have callbacks into UEFI context like this?  If
>>>>>> just one or two maybe we can suppress them while in u-boot context and
>>>>>> handle in efi_exit_func() when entry_count drops to zero?
>>>>>
>>>>>
>>>>> What do you mean by suppressing? We need to call those helpers
>>>>> synchronously. And yes, we can probably hand massage the refcount on the
>>>>> ones that we remember, but I'm just afraid that the whole thing will
>>>>> be so
>>>>> complicated eventually that nobody understands what's going on.
>>>>
>>>> Maybe suppress isn't the right word.. I was thinking of delaying the
>>>> callback until EFI_EXIT() that transitions back to the UEFI world.  So
>>>> from the PoV of the UEFI world, it is still synchronous.
>>>>
>>>> I haven't looked at the efi_event stuff until just now, but from a
>>>> 30sec look, maybe efi_signal_event() could just put the event on a
>>>> list of signalled events and not immediately call
>>>> event->notify_function(), and handle the calling of notify_function()
>>>> in the last EFI_EXIT()??
>>>
>>> Maybe, I'll leave Heinrich to comment on that.
>>
>> Let the application call WaitForEvent for a keyboard event.
>> Rob suggests that neither timer events nor any other events are served
>> until the user possibly presses a key.
>>
>> No, we have to call notification functions immediately to serve time
>> critical communication like network traffic.
>>
> 
> Does WaitForEvent or anything else that dispatches events dispatch
> more than one event, or is it just one event and then return to UEFI
> world to poll again?  We aren't really multi-threaded so there can't
> really be async callbacks.
> 
> Either way, I think we can make this work by changing
> EFI_EXIT()/callback/EFI_ENTER() to EFI_CALLBACK(callback).. for the
> non-nested EFI_ENTER() case we don't have to do anything special (and
> the nested case probably should never happen in places where we need
> EFI_CALLBACK())

WaitForEvent and CheckEvent calls will serve all notify_functions of
timer events this includes a notify_function checking the keyboard and
in future I want to check the watchdog in a notify_function.

Please, consider that in the notify functions any number of UEFI API
functions will be called by the EFI application which themselves may
signal events which in turn call notify_functions in which again UEFI
API functions may be called.

So make no assumptions whatsoever about what is going to happen in a
notify function or about the level of nesting.

Best regards

Heinrich