On 25.07.17 14:28, Rob Clark wrote:
On Tue, Jul 25, 2017 at 8:16 AM, Alexander Graf <ag...@suse.de> wrote:


On 25.07.17 14:08, Rob Clark wrote:

On Tue, Jul 25, 2017 at 7:17 AM, Alexander Graf <ag...@suse.de> wrote:



On 25.07.17 13:10, Rob Clark wrote:


On Tue, Jul 25, 2017 at 4:10 AM, Alexander Graf <ag...@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 <robdcl...@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.


I personally think having _ext functions in anything exposed to UEFI
payloads makes more sense.


having 2x all the interfaces for file related protocols would be

Why 2x the interfaces?

unfortunate.  (Also currently they can stay static in efi_file.c and
just be called via same efi_file_handle fxn ptrs that the UEFI world
would be using, which is kinda nice.)

Ah, that indeed is nice.

So, there is a third option if you want to tackle it:

  3) Remove register variable need for gd

If gd is a simple global variable rather than a register variable, we wouldn't have that problem. On x86 it's just a variable for example. I don't know why arm and aarch64 have it as register variable, but I can't see an immediate need for it.

If you manage to solve that, we don't need any ext functions or reference counters ;).


Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to