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

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; \ +}) + +/* + * Call void UEFI function world from u-boot: + */ +#define EFI_CALL_VOID(exp) do { \ debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \ assert(__efi_exit_check()); \ exp; \ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 43f32385fa..6489a32505 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -158,7 +158,8 @@ void efi_signal_event(struct efi_event *event) return; event->signaled = 1; if (event->type & EVT_NOTIFY_SIGNAL) { - EFI_CALL(event->notify_function(event, event->notify_context)); + EFI_CALL_VOID(event->notify_function(event, + event->notify_context)); } }

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.
BR, -R
+/*
- Call void UEFI function world from u-boot:
- */
+#define EFI_CALL_VOID(exp) do { \ debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \ assert(__efi_exit_check()); \ exp; \ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 43f32385fa..6489a32505 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -158,7 +158,8 @@ void efi_signal_event(struct efi_event *event) return; event->signaled = 1; if (event->type & EVT_NOTIFY_SIGNAL) {
EFI_CALL(event->notify_function(event, event->notify_context));
EFI_CALL_VOID(event->notify_function(event,
event->notify_context)); }
}
-- 2.14.1

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
Regards
Heinrich
BR, -R
+/*
- Call void UEFI function world from u-boot:
- */
+#define EFI_CALL_VOID(exp) do { \ debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \ assert(__efi_exit_check()); \ exp; \ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 43f32385fa..6489a32505 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -158,7 +158,8 @@ void efi_signal_event(struct efi_event *event) return; event->signaled = 1; if (event->type & EVT_NOTIFY_SIGNAL) {
EFI_CALL(event->notify_function(event, event->notify_context));
EFI_CALL_VOID(event->notify_function(event,
event->notify_context)); }
}
-- 2.14.1

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
participants (2)
-
Heinrich Schuchardt
-
Rob Clark