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()?? > 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 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.) _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot