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. Best regards Heinrich > >> >>> 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
