On Thu, Aug 17, 2017 at 12:57 PM, Heinrich Schuchardt xypron.glpk@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@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@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