[PATCH 6/8] efi/arm*: libstub: wire up GOP handling into the ARM UEFI stub

[Ard Biesheuvel]

On 10 March 2016 at 16:03, Ingo Molnar <mingo at kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote:
>
>> > So screen_info_guid should probably not be a 'const': you have to cast it away
>> > anyway, adding artificial linebreaks and uglifying the code. It's also a bad
>> > practice to cast away const-ness, it hinders move-consts-to-readonly-sections
>> > efforts.
>>
>> The problem here is that the UEFI spec never uses const qualifiers in
>> its APIs for by-ref parameters that are obviously never modified by
>> the caller, such as these GUIDs. [...]
>
> Ah, ok. Two related thoughts came up:
>
> 1)
>
> While I was looking at this code and was asking myself why the EFI runtime is
> generally invoked via a relatively fragile, non-type-checking vararg construct.
>
> Wouldn't you be better off by explicitly defining all the API variants, and then
> internally calling the EFI runtime?
>
> That would neatly solve such const artifacts as well.
>
> So instead of:
>
>
> +       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
> +                               sizeof(*si), (void **)&si);
>
> we could have something like:
>
>         status = efi_early__allocate_pool(EFI_RUNTIME_SERVICES_DATA, sizeof(*si), &si);
>
>         ...
>
>         efi_early__free_pool(si);
>
>
> i.e. it would look a lot more like a properly distributed, typed, structured
> family of C function APIs, instead of this single central bastard of an ioctl()
> interface.
>
> There's over 100 invocations of the EFI runtime in the Linux kernel, I think it
> would be worth the effort. The wrapper inlines should be mostly trivial.
>
> That would also add an opportunity to actually document most of these calls.
>

If only. The ARM and arm64 wrappers actually simply resolve to
correctly typed calls. While I am not an expert on the x86 side of
things, I think the vararg stuff is needed to be able to perform the
thunking required to do function calls using the MS ABI for x86_64.

As far as documentation is concerned, all of these functions and
protocol methods are documented in the UEFI spec, which is freely
accessible. It may be somewhat redundant to have our own documentation
for them.

> 2)
>
> Another suggestion: would it make sense to unify the 'EFI' and 'EFI early' calls -
> is there any deep reason why they are invoked via separate names? Why not use a
> single namespace:
>
>         efi__allocate_pool()
>         efi__free_pool()
>
> and have a 'current EFI configuration' pointer internaly that can be switched from
> the early to the later variant during bootup. The various typed API wrappers would
> use this pointer.
>

Actually, having statically initialized pointer variables is more of a
concern. The nice thing about plain function calls is that they can
usually be resolved at link time to relative branches on most
architectures, rather than require GOT fixups or other magic to be
able to execute in the UEFI environment.

Thanks,
Ard.