[U-Boot] [PATCH 1/1] efi_loader: allow return value in EFI_CALL

[Rob Clark]

On Thu, Aug 17, 2017 at 12:57 PM, Heinrich Schuchardt
<xypron.glpk at gmx.de> wrote:
> On 08/17/2017 01:49 PM, Rob Clark wrote:
>> On Tue, Aug 15, 2017 at 3:42 PM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>> Macro EFI_CALL was introduced to call an UEFI function.
>>> Unfortunately is did not support return values.
>>> Most UEFI functions have a return value.
>>>
>>> So let's rename EFI_CALL to EFI_CALL_VOID and introduce a
>>> new EFI_CALL macro that supports return values.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>  include/efi_loader.h          | 16 ++++++++++++++--
>>>  lib/efi_loader/efi_boottime.c |  3 ++-
>>>  2 files changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index 037cc7c543..1cee10ea0c 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -41,9 +41,21 @@ const char *__efi_nesting_dec(void);
>>>         })
>>>
>>>  /*
>>> - * Callback into UEFI world from u-boot:
>>> + * Call non-void UEFI function from u-boot and retrieve return value:
>>>   */
>>> -#define EFI_CALL(exp) do { \
>>> +#define EFI_CALL(exp) ({ \
>>> +       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
>>> +       assert(__efi_exit_check()); \
>>> +       typeof(exp) r = exp; \
>>> +       assert(__efi_entry_check()); \
>>> +       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
>>> +       r; \
>>> +})
>>> +
>>
>> So, I had considered something similar, for the purposes of being able
>> to print return value, but in the end decided that the EFI_EXIT()
>> print was usually sufficient (except in cases where you enable DEBUG
>> in some files but not others), and opted for the simpler MACRO(ret =
>> somecall()) approach.. also the longer macro name makes not going over
>> 80 chars harder in some cases ;-)
>>
>> I don't really feel strongly one way or another.. but I guess if we do
>> this we might as well add the return value (cast to u64 and print as
>> hex to cover both ptrs and efi_status_t?) to the "Return From" debug
>> print.
>
> We already print the return value in EFI_EXIT.
> Why should we print it twice?
>
> In c160d2f5ec9
>

the main reason would be enabling debug in a subset of the files (such
as the one calling the API but not the one implementing the API),
which is something I do pretty regularly.  And I guess in the future
we could have payloads installing protocols?

BR,
-R