On Tue, Jul 25, 2017 at 3:12 PM, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > On 07/25/2017 07:52 PM, Rob Clark wrote: >> On Tue, Jul 25, 2017 at 12:56 PM, Heinrich Schuchardt >> <xypron.g...@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 <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. >>> >>> 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.
sure, one way or another I don't want to call a notifier from nested EFI_ENTER()'s.. so either EFI_CALLBACK() should detect the condition and yell loudly, or it should detect it and queue up callbacks until we are not nested.. I think the first option is probably ok. In the special cases where we might need to dispatch events, _ext() fxns should still be used until gd goes away. BR, -R > So make no assumptions whatsoever about what is going to happen in a > notify function or about the level of nesting. > > Best regards > > Heinrich _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
