efi_loader: only evaluate EFI_EXIT()'s ret once

PATCH New Rob Clark 2 participants 16 messages Jul 27, 2017 – Jul 28, 2017

lore

Share & tools

lore

Rob Clark

author

2017-07-27 12:04 UTC #0 lore raw

PATCH 1/4 efi_loader: only evaluate EFI_EXIT()'s ret once

There are a couple spots doing things like:

   return EFI_EXIT(some_fxn(...));

which I handn't noticed before. With addition of printing return value in the EFI_EXIT() macro, now the fxn call was getting evaluated twice. Which we didn't really want.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_loader.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

include/efi_loader.h +3 -2

index f384cbbe77..9700a88d69 100644
@@ -21,8 +21,9 @@
 	} while(0)
 
 #define EFI_EXIT(ret) ({ \
-	debug("EFI: Exit: %s: %u\n", __func__, (u32)((ret) & ~EFI_ERROR_MASK)); \
-	efi_exit_func(ret); \
+	efi_status_t _r = ret; \
+	debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
+	efi_exit_func(_r); \
 	})
 
 extern struct efi_runtime_services efi_runtime_services;

Signature · 1 line

2.13.0

Rob Clark

author

2017-07-27 12:04 UTC #1 lore raw

PATCH 2/4 efi_loader: Add an EFI_CALL() macro

Rather than open-coding EFI_EXIT() + callback + EFI_ENTRY(), introduce an EFI_CALL() macro. This makes callbacks into UEFI world (of which there will be more in the future) more concise and easier to locate in the code.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_loader.h          | 17 +++++++++++++++++
 lib/efi_loader/efi_boottime.c |  4 +---
 2 files changed, 18 insertions(+), 3 deletions(-)

include/efi_loader.h +17 -0

index 9700a88d69..eb16c14b69 100644
@@ -15,17 +15,34 @@
 
 #include <linux/list.h>
 
+/*
+ * Enter the u-boot world from UEFI:
+ */
 #define EFI_ENTRY(format, ...) do { \
 	efi_restore_gd(); \
 	debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
 	} while(0)
 
+/*
+ * Exit the u-boot world back to UEFI:
+ */
 #define EFI_EXIT(ret) ({ \
 	efi_status_t _r = ret; \
 	debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
 	efi_exit_func(_r); \
 	})
 
+/*
+ * Callback into UEFI world from u-boot:
+ */
+#define EFI_CALL(exp) do { \
+	debug("EFI: Call: %s\n", #exp); \
+	efi_exit_func(EFI_SUCCESS); \
+	exp; \
+	efi_restore_gd(); \
+	debug("EFI: Return From: %s\n", #exp); \
+	} while(0)
+
 extern struct efi_runtime_services efi_runtime_services;
 extern struct efi_system_table systab;

lib/efi_loader/efi_boottime.c +1 -3

index 17c531a480..849d229821 100644
@@ -120,9 +120,7 @@ void efi_signal_event(struct efi_event *event)
 		return;
 	event->signaled = 1;
 	if (event->type & EVT_NOTIFY_SIGNAL) {
-		EFI_EXIT(EFI_SUCCESS);
-		event->notify_function(event, event->notify_context);
-		EFI_ENTRY("returning from notification function");
+		EFI_CALL(event->notify_function(event, event->notify_context));
 	}
 }

Signature · 1 line

2.13.0

Alexander Graf

Applied

2017-07-28 22:29 UTC #2 lore raw

PATCH 2/4 efi_loader: Add an EFI_CALL() macro

Quoted 6 lines

Rather than open-coding EFI_EXIT() + callback + EFI_ENTRY(), introduce an EFI_CALL() macro. This makes callbacks into UEFI world (of which there will be more in the future) more concise and easier to locate in the code.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex

Rob Clark

author

2017-07-27 12:04 UTC #3 lore raw

PATCH 3/4 efi_loader: add checking for incorrect use of EFI_ENTRY/EXIT

In reply to Rob Clark

Missing an EFI_ENTRY() or doubling up EFI_EXIT() leads to non-obvious crashes. Let's add some error checking.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_loader.h          | 17 +++++++++-------
 lib/efi_loader/efi_boottime.c | 45 +++++++++++++++++++++++++++++--------------
 2 files changed, 41 insertions(+), 21 deletions(-)

include/efi_loader.h +10 -7

index eb16c14b69..4262d0ac6b 100644
@@ -15,11 +15,14 @@
 
 #include <linux/list.h>
 
+int __efi_entry_check(void);
+int __efi_exit_check(void);
+
 /*
  * Enter the u-boot world from UEFI:
  */
 #define EFI_ENTRY(format, ...) do { \
-	efi_restore_gd(); \
+	assert(__efi_entry_check()); \
 	debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
 	} while(0)

@@ -29,7 +32,8 @@
 #define EFI_EXIT(ret) ({ \
 	efi_status_t _r = ret; \
 	debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
-	efi_exit_func(_r); \
+	assert(__efi_exit_check()); \
+	_r; \
 	})
 
 /*

@@ -37,9 +41,9 @@
  */
 #define EFI_CALL(exp) do { \
 	debug("EFI: Call: %s\n", #exp); \
-	efi_exit_func(EFI_SUCCESS); \
+	assert(__efi_exit_check()); \
 	exp; \
-	efi_restore_gd(); \
+	assert(__efi_entry_check()); \
 	debug("EFI: Return From: %s\n", #exp); \
 	} while(0)

@@ -139,10 +143,9 @@ void efi_timer_check(void);
 void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info);
 /* Called once to store the pristine gd pointer */
 void efi_save_gd(void);
-/* Called from EFI_ENTRY on callback entry to put gd into the gd register */
+/* Special case handler for error/abort that just tries to dtrt to get
+ * back to u-boot world */
 void efi_restore_gd(void);
-/* Called from EFI_EXIT on callback exit to restore the gd register */
-efi_status_t efi_exit_func(efi_status_t ret);
 /* Call this to relocate the runtime section to an address space */
 void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
 /* Call this to set the current device name */

lib/efi_loader/efi_boottime.c +31 ~~-14~~

74 lines not on this page · raw message

Signature · 1 line

2.13.0

Alexander Graf

Applied

2017-07-28 22:27 UTC #4 lore raw

PATCH 3/4 efi_loader: add checking for incorrect use of EFI_ENTRY/EXIT

Quoted 4 lines

Missing an EFI_ENTRY() or doubling up EFI_EXIT() leads to non-obvious crashes. Let's add some error checking.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex

Rob Clark

author

2017-07-27 12:04 UTC #5 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

In reply to Rob Clark

This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_loader.h          | 12 ++++++++----
 lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +8 -4

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
 
 int __efi_entry_check(void);
 int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
 
 /*
  * Enter the u-boot world from UEFI:
  */
 #define EFI_ENTRY(format, ...) do { \
 	assert(__efi_entry_check()); \
-	debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
+	debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+		__func__, ##__VA_ARGS__); \
 	} while(0)
 
 /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
  */
 #define EFI_EXIT(ret) ({ \
 	efi_status_t _r = ret; \
-	debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
+	debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+		__func__, (u32)(_r & ~EFI_ERROR_MASK)); \
 	assert(__efi_exit_check()); \
 	_r; \
 	})

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
  * Callback into UEFI world from u-boot:
  */
 #define EFI_CALL(exp) do { \
-	debug("EFI: Call: %s\n", #exp); \
+	debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
 	assert(__efi_exit_check()); \
 	exp; \
 	assert(__efi_entry_check()); \
-	debug("EFI: Return From: %s\n", #exp); \
+	debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
 	} while(0)
 
 extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +25 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
 #endif
 
 static int entry_count;
+static int nesting_level;
 
 /* Called on every callback entry */
 int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
 #endif
 }
 
+/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be
+ * enough for anyone ;-)
+ */
+static const char *indent_string(int level)
+{
+	static const char *indent = "                    ";
+	const int max = strlen(indent);
+	level = min(max, level * 2);
+	return &indent[max - level];
+}
+
+const char *__efi_nesting_inc(void)
+{
+	return indent_string(nesting_level++);
+}
+
+const char *__efi_nesting_dec(void)
+{
+	return indent_string(--nesting_level);
+}
+
 /* Low 32 bit */
 #define EFI_LOW32(a) (a & 0xFFFFFFFFULL)
 /* High 32 bit */

@@ -748,9 +771,11 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 		return EFI_EXIT(info->exit_status);
 	}
 
+	__efi_nesting_dec();
 	__efi_exit_check();
 	entry(image_handle, &systab);
 	__efi_entry_check();
+	__efi_nesting_inc();
 
 	/* Should usually never get here */
 	return EFI_EXIT(EFI_SUCCESS);

Signature · 1 line

2.13.0

Alexander Graf

2017-07-28 07:24 UTC #6 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 5 lines · Rob Clark

This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>

Doesn't the previous patch ensure that we're always only going 1 level deep?

Quoted 64 lines

---
  include/efi_loader.h          | 12 ++++++++----
  lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
  2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +8 -4

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
 
  int __efi_entry_check(void);
  int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
 
  /*
   * Enter the u-boot world from UEFI:
   */
  #define EFI_ENTRY(format, ...) do { \
  	assert(__efi_entry_check()); \
-	debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
+	debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+		__func__, ##__VA_ARGS__); \
  	} while(0)
 
  /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
   */
  #define EFI_EXIT(ret) ({ \
  	efi_status_t _r = ret; \
-	debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
+	debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+		__func__, (u32)(_r & ~EFI_ERROR_MASK)); \
  	assert(__efi_exit_check()); \
  	_r; \
  	})

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
   * Callback into UEFI world from u-boot:
   */
  #define EFI_CALL(exp) do { \
-	debug("EFI: Call: %s\n", #exp); \
+	debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
  	assert(__efi_exit_check()); \
  	exp; \
  	assert(__efi_entry_check()); \
-	debug("EFI: Return From: %s\n", #exp); \
+	debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
  	} while(0)
 
  extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +1 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
  #endif
 
  static int entry_count;
+static int nesting_level;
 
  /* Called on every callback entry */
  int __efi_entry_check(void)

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 106

Quoted 10 lines

lib/efi_loader/efi_boottime.c +7 -0

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
  #endif
  }
 
+/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be
+ * enough for anyone ;-)
+ */
+static const char *indent_string(int level)
+{
+	static const char *indent = "                    ";

There's no need for this to be static, no?

Alex

Rob Clark

author

2017-07-28 09:19 UTC #7 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 4 lines · Alexander Graf

Quoted 1 line · Rob Clark

Quoted 8 lines

Quoted 5 lines
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>
Doesn't the previous patch ensure that we're always only going 1 level deep?

two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)

Quoted 64 lines

---
  include/efi_loader.h          | 12 ++++++++----
  lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
  2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +6 -2

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
    int __efi_entry_check(void);
  int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
    /*
   * Enter the u-boot world from UEFI:
   */
  #define EFI_ENTRY(format, ...) do { \
        assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+               __func__, ##__VA_ARGS__); \
        } while(0)
    /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
   */
  #define EFI_EXIT(ret) ({ \
        efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
        assert(__efi_exit_check()); \
        _r; \

})

include/efi_loader.h +2 -2

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
   * Callback into UEFI world from u-boot:
   */
  #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
        assert(__efi_exit_check()); \
        exp; \
        assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
        } while(0)
    extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +2 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
  #endif
    static int entry_count;
+static int nesting_level;
    /* Called on every callback entry */
  int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
  #endif
  }
  +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be

Quoted 8 lines

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 106
Quoted 5 lines
lib/efi_loader/efi_boottime.c +5 -0
+ * enough for anyone ;-)
+ */
+static const char *indent_string(int level)
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?

I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.

BR,
-R

Alexander Graf

2017-07-28 09:25 UTC #8 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 9 lines · Rob Clark

Quoted 8 lines · Alexander Graf
Quoted 5 lines · Rob Clark
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Rob Clark <[email protected]>
Doesn't the previous patch ensure that we're always only going 1 level deep?
two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)

Ah, so this basically gives you the EFI_CALL nesting level? Wouldn't it make sense to also increase the nesting level on every application invocation?

Quoted 74 lines

Quoted 65 lines


---
   include/efi_loader.h          | 12 ++++++++----
   lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
   2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +6 -2

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
     int __efi_entry_check(void);
   int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
     /*
    * Enter the u-boot world from UEFI:
    */
   #define EFI_ENTRY(format, ...) do { \
         assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+               __func__, ##__VA_ARGS__); \
         } while(0)
     /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
    */
   #define EFI_EXIT(ret) ({ \
         efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
         assert(__efi_exit_check()); \
         _r; \

})

include/efi_loader.h +2 -2

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
    * Callback into UEFI world from u-boot:
    */
   #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
         assert(__efi_exit_check()); \
         exp; \
         assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
         } while(0)
     extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +2 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
   #endif
     static int entry_count;
+static int nesting_level;
     /* Called on every callback entry */
   int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
   #endif
   }
   +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be

Quoted 8 lines

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 106
Quoted 5 lines
lib/efi_loader/efi_boottime.c +5 -0
+ * enough for anyone ;-)
+ */
+static const char *indent_string(int level)
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?

Quoted 5 lines

I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.

I don't mean the function, I mean the indent. If you do

   static const char *indent = <const value>;

it should be practically the same as

   const char *indent = <const value>;

no?

Alex

Rob Clark

author

2017-07-28 09:34 UTC #9 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 23 lines · Alexander Graf

Quoted 20 lines · Rob Clark
Quoted 16 lines · Alexander Graf
Quoted 7 lines · Rob Clark
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>
Doesn't the previous patch ensure that we're always only going 1 level deep?

Quoted 6 lines

Quoted 2 lines
two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)
Ah, so this basically gives you the EFI_CALL nesting level? Wouldn't it make sense to also increase the nesting level on every application invocation?

I specifically avoided that since (at least at what I was looking at) each successive application invocation never returns.

Maybe instead we should just do something like:
debug("========================================\n") to show the application invocation boundaries more easily?

Quoted 87 lines

Quoted 78 lines

Quoted 68 lines



---
   include/efi_loader.h          | 12 ++++++++----
   lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
   2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +6 -2

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
     int __efi_entry_check(void);
   int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
     /*
    * Enter the u-boot world from UEFI:
    */
   #define EFI_ENTRY(format, ...) do { \
         assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+               __func__, ##__VA_ARGS__); \
         } while(0)
     /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
    */
   #define EFI_EXIT(ret) ({ \
         efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
         assert(__efi_exit_check()); \
         _r; \

})

include/efi_loader.h +2 -2

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
    * Callback into UEFI world from u-boot:
    */
   #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
         assert(__efi_exit_check()); \
         exp; \
         assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
         } while(0)
     extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +3 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
   #endif
     static int entry_count;
+static int nesting_level;
     /* Called on every callback entry */
   int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
   #endif
   }
   +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be
+ * enough for anyone ;-)

Quoted 8 lines

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 106
Quoted 4 lines
lib/efi_loader/efi_boottime.c +4 -0
+ */
+static const char *indent_string(int level)
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?

Quoted 5 lines

I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.

I don't mean the function, I mean the indent. If you do

Quoted 7 lines

  static const char *indent = <const value>;
it should be practically the same as
  const char *indent = <const value>;
no?

hmm, I didn't want the compiler to instantiate the array on the stack. But I suppose I need to check the generated asm to see how clever it is.

BR,
-R

Alexander Graf

2017-07-28 09:36 UTC #10 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 31 lines · Rob Clark

Quoted 23 lines · Alexander Graf
Quoted 20 lines · Rob Clark
Quoted 16 lines · Alexander Graf
Quoted 7 lines · Rob Clark
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>
Doesn't the previous patch ensure that we're always only going 1 level deep?
Quoted 6 lines
Quoted 2 lines
two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)
Ah, so this basically gives you the EFI_CALL nesting level? Wouldn't it make sense to also increase the nesting level on every application invocation?

Quoted 6 lines

I specifically avoided that since (at least at what I was looking at) each successive application invocation never returns.
Maybe instead we should just do something like:
debug("========================================\n") to show the application invocation boundaries more easily?

Sounds like a good idea to me :). Ideally with a bit more information such as the file path.

Quoted 92 lines

Quoted 79 lines

Quoted 69 lines




---
    include/efi_loader.h          | 12 ++++++++----
    lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
    2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +6 -2

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
      int __efi_entry_check(void);
    int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
      /*
     * Enter the u-boot world from UEFI:
     */
    #define EFI_ENTRY(format, ...) do { \
          assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+               __func__, ##__VA_ARGS__); \
          } while(0)
      /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
     */
    #define EFI_EXIT(ret) ({ \
          efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
          assert(__efi_exit_check()); \
          _r; \

})

include/efi_loader.h +2 -2

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
     * Callback into UEFI world from u-boot:
     */
    #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
          assert(__efi_exit_check()); \
          exp; \
          assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp); \
          } while(0)
      extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +3 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
    #endif
      static int entry_count;
+static int nesting_level;
      /* Called on every callback entry */
    int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
    #endif
    }
    +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be
+ * enough for anyone ;-)

Quoted 8 lines

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 106
Quoted 4 lines
lib/efi_loader/efi_boottime.c +4 -0
+ */
+static const char *indent_string(int level)
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?

Quoted 5 lines

I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.

I don't mean the function, I mean the indent. If you do

   static const char *indent = <const value>;

it should be practically the same as

Quoted 7 lines

Quoted 3 lines
   const char *indent = <const value>;
no?
hmm, I didn't want the compiler to instantiate the array on the stack. But I suppose I need to check the generated asm to see how clever it is.

It really shouldn't do that. As long as you're just juggling pointers to a region in .rodata it should know exactly what's going on.

Alex

Rob Clark

author

2017-07-28 10:11 UTC #11 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 47 lines · Alexander Graf

Quoted 44 lines · Rob Clark
Quoted 29 lines · Alexander Graf
Quoted 25 lines · Rob Clark
Quoted 19 lines · Alexander Graf
Quoted 8 lines · Rob Clark
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>
Doesn't the previous patch ensure that we're always only going 1 level deep?
Quoted 8 lines
Quoted 2 lines
two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)
Ah, so this basically gives you the EFI_CALL nesting level? Wouldn't it make
sense to also increase the nesting level on every application invocation?
I specifically avoided that since (at least at what I was looking at) each successive application invocation never returns.

Quoted 8 lines

Quoted 3 lines
Maybe instead we should just do something like:
debug("========================================\n") to show the application invocation boundaries more easily?
Sounds like a good idea to me :). Ideally with a bit more information such as the file path.

well, until my RFC patchset, there is no guarantee to be a file path ;-)

And both the device and file path will be efi_device_path. I have some thoughts to spiff out device_path_to_text a bit more to make it more complete and also suitable for internal debugging use.. but that is going to be post-MW. Once that happens we can add some more information, but for now the boring "===========" is all we can do.

Quoted 103 lines

Quoted 101 lines

Quoted 84 lines

Quoted 73 lines





---
    include/efi_loader.h          | 12 ++++++++----
    lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
    2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +6 -2

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
      int __efi_entry_check(void);
    int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
      /*
     * Enter the u-boot world from UEFI:
     */
    #define EFI_ENTRY(format, ...) do { \
          assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__);
 \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+               __func__, ##__VA_ARGS__); \
          } while(0)
      /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
     */
    #define EFI_EXIT(ret) ({ \
          efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
          assert(__efi_exit_check()); \
          _r; \

})

include/efi_loader.h +2 -2

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
     * Callback into UEFI world from u-boot:
     */
    #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
          assert(__efi_exit_check()); \
          exp; \
          assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp);
 \
          } while(0)
      extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +4 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
    #endif
      static int entry_count;
+static int nesting_level;
      /* Called on every callback entry */
    int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
    #endif
    }
    +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be
+ * enough for anyone ;-)
+ */

Quoted 8 lines

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 106
Quoted 3 lines
lib/efi_loader/efi_boottime.c +3 -0
+static const char *indent_string(int level)
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?

Quoted 5 lines

I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.

I don't mean the function, I mean the indent. If you do

   static const char *indent = <const value>;

it should be practically the same as

   const char *indent = <const value>;

no?

Quoted 8 lines

Quoted 3 lines
hmm, I didn't want the compiler to instantiate the array on the stack. But I suppose I need to check the generated asm to see how clever it is.
It really shouldn't do that. As long as you're just juggling pointers to a region in .rodata it should know exactly what's going on.

I'll double check when I get to the office.. making it static doesn't hurt and forces the compiler to do what we want (.rodata). Without the 'static' it might end up doing the same thing.

BR,
-R

Alexander Graf

2017-07-28 10:22 UTC #12 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 57 lines · Rob Clark

Quoted 47 lines · Alexander Graf
Quoted 44 lines · Rob Clark
Quoted 29 lines · Alexander Graf
Quoted 25 lines · Rob Clark
Quoted 19 lines · Alexander Graf
Quoted 8 lines · Rob Clark
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>
Doesn't the previous patch ensure that we're always only going 1 level deep?
Quoted 8 lines
Quoted 2 lines
two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)
Ah, so this basically gives you the EFI_CALL nesting level? Wouldn't it make
sense to also increase the nesting level on every application invocation?
I specifically avoided that since (at least at what I was looking at) each successive application invocation never returns.
Quoted 8 lines
Quoted 3 lines
Maybe instead we should just do something like:
debug("========================================\n") to show the application invocation boundaries more easily?
Sounds like a good idea to me :). Ideally with a bit more information such as the file path.

Quoted 7 lines

well, until my RFC patchset, there is no guarantee to be a file path ;-)
And both the device and file path will be efi_device_path. I have some thoughts to spiff out device_path_to_text a bit more to make it more complete and also suitable for internal debugging use.. but that is going to be post-MW. Once that happens we can add some more information, but for now the boring "===========" is all we can do.

No worries, we can introduce a fancy ========= when the time is ripe :).

Quoted 109 lines

Quoted 107 lines

Quoted 95 lines

Quoted 85 lines

Quoted 74 lines






---
     include/efi_loader.h          | 12 ++++++++----
     lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
     2 files changed, 33 insertions(+), 4 deletions(-)

include/efi_loader.h +6 -2

index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
       int __efi_entry_check(void);
     int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
       /*
      * Enter the u-boot world from UEFI:
      */
     #define EFI_ENTRY(format, ...) do { \
           assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__);
 \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(), \
+               __func__, ##__VA_ARGS__); \
           } while(0)
       /*

@@ -31,7 +34,8 @@ int __efi_exit_check(void);
      */
     #define EFI_EXIT(ret) ({ \
           efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
           assert(__efi_exit_check()); \
           _r; \

})

include/efi_loader.h +2 -2

@@ -40,11 +44,11 @@ int __efi_exit_check(void);
      * Callback into UEFI world from u-boot:
      */
     #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
           assert(__efi_exit_check()); \
           exp; \
           assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(), #exp);
 \
           } while(0)
       extern struct efi_runtime_services efi_runtime_services;

lib/efi_loader/efi_boottime.c +4 -0

index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
     #endif
       static int entry_count;
+static int nesting_level;
       /* Called on every callback entry */
     int __efi_entry_check(void)

@@ -96,6 +97,28 @@ void efi_restore_gd(void)
     #endif
     }
     +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to be
+ * enough for anyone ;-)
+ */

Quoted 8 lines

lib/efi_loader/efi_boottime.c void efi_restore_gd(void) · line 105
Quoted 3 lines
lib/efi_loader/efi_boottime.c +3 -0
+static const char *indent_string(int level)
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?

Quoted 5 lines

I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.

I don't mean the function, I mean the indent. If you do

Quoted 7 lines

    static const char *indent = <const value>;

it should be practically the same as

    const char *indent = <const value>;

no?

hmm, I didn't want the compiler to instantiate the array on the stack. But I suppose I need to check the generated asm to see how clever it is.

Quoted 7 lines

Quoted 3 lines
It really shouldn't do that. As long as you're just juggling pointers to a region in .rodata it should know exactly what's going on.
I'll double check when I get to the office.. making it static doesn't hurt and forces the compiler to do what we want (.rodata). Without the 'static' it might end up doing the same thing.

Well, every time i see a "static" declared variable inside a function scope my alarm bells ring :). So I'd prefer we had as few as possible.

In fact, I *think* what your line does is it puts the pointer into a global in .data which really isn't what we want.

Alex

Rob Clark

author

2017-07-28 11:54 UTC #13 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

Quoted 216 lines · Alexander Graf

Quoted 73 lines · Rob Clark
Quoted 66 lines · Alexander Graf
Quoted 53 lines · Rob Clark
Quoted 43 lines · Alexander Graf
Quoted 32 lines · Rob Clark
Quoted 21 lines · Alexander Graf
Quoted 10 lines · Rob Clark
This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY()
and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>
Quoted 3 lines
Doesn't the previous patch ensure that we're always only going 1 level
deep?
Quoted 2 lines
two separate counters for nesting and entry level. We can be more deeply nested when EFI_CALL() is used :-)
Quoted 4 lines
Ah, so this basically gives you the EFI_CALL nesting level? Wouldn't it make
sense to also increase the nesting level on every application invocation?
Quoted 6 lines
I specifically avoided that since (at least at what I was looking at) each successive application invocation never returns.
Maybe instead we should just do something like:
debug("========================================\n") to show the application invocation boundaries more easily?
Quoted 4 lines
Sounds like a good idea to me :). Ideally with a bit more information such
as the file path.
Quoted 7 lines
well, until my RFC patchset, there is no guarantee to be a file path ;-)
And both the device and file path will be efi_device_path. I have some thoughts to spiff out device_path_to_text a bit more to make it more complete and also suitable for internal debugging use.. but that is going to be post-MW. Once that happens we can add some more information, but for now the boring "===========" is all we can do.
No worries, we can introduce a fancy ========= when the time is ripe :).
Quoted 118 lines
Quoted 118 lines
Quoted 115 lines
Quoted 102 lines
Quoted 91 lines
Quoted 79 lines
Quoted 79 lines
---
     include/efi_loader.h          | 12 ++++++++----
     lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++
     2 files changed, 33 insertions(+), 4 deletions(-)
include/efi_loader.h +6 -2
index 4262d0ac6b..037cc7c543 100644
@@ -17,13 +17,16 @@
       int __efi_entry_check(void);
     int __efi_exit_check(void);
+const char *__efi_nesting_inc(void);
+const char *__efi_nesting_dec(void);
       /*
      * Enter the u-boot world from UEFI:
      */
     #define EFI_ENTRY(format, ...) do { \
           assert(__efi_entry_check()); \
-       debug("EFI: Entry %s(" format ")\n", __func__,
 ##__VA_ARGS__);
 \
+       debug("%sEFI: Entry %s(" format ")\n", __efi_nesting_inc(),
 \
+               __func__, ##__VA_ARGS__); \
           } while(0)
       /*
@@ -31,7 +34,8 @@ int __efi_exit_check(void);
      */
     #define EFI_EXIT(ret) ({ \
           efi_status_t _r = ret; \
-       debug("EFI: Exit: %s: %u\n", __func__, (u32)(_r &
 ~EFI_ERROR_MASK)); \
+       debug("%sEFI: Exit: %s: %u\n", __efi_nesting_dec(), \
+               __func__, (u32)(_r & ~EFI_ERROR_MASK)); \
           assert(__efi_exit_check()); \
           _r; \
           })
include/efi_loader.h +2 -2
@@ -40,11 +44,11 @@ int __efi_exit_check(void);
      * Callback into UEFI world from u-boot:
      */
     #define EFI_CALL(exp) do { \
-       debug("EFI: Call: %s\n", #exp); \
+       debug("%sEFI: Call: %s\n", __efi_nesting_inc(), #exp); \
           assert(__efi_exit_check()); \
           exp; \
           assert(__efi_entry_check()); \
-       debug("EFI: Return From: %s\n", #exp); \
+       debug("%sEFI: Return From: %s\n", __efi_nesting_dec(),
 #exp);
 \
           } while(0)
       extern struct efi_runtime_services efi_runtime_services;
lib/efi_loader/efi_boottime.c +2 -0
index 66137d4ff9..de338f009c 100644
@@ -50,6 +50,7 @@ static volatile void *efi_gd, *app_gd;
     #endif
       static int entry_count;
+static int nesting_level;
       /* Called on every callback entry */
     int __efi_entry_check(void)
@@ -96,6 +97,28 @@ void efi_restore_gd(void)
     #endif
     }
     +/*
+ * Two spaces per indent level, maxing out at 10.. which ought to
be
+ * enough for anyone ;-)
+ */
+static const char *indent_string(int level)
Quoted 8 lines
lib/efi_loader/efi_boottime.c
Quoted 2 lines
lib/efi_loader/efi_boottime.c +2 -0
+{
+       static const char *indent = "                    ";
There's no need for this to be static, no?
Quoted 5 lines
I suppose it doesn't *need* to be.. but it also doesn't need to have scope outside the file, and usually static is a good hint to the compiler to inline it. (If non-static the compiler needs to emit a non-inlined version of it since it doesn't know it won't be called outside of this object file.
I don't mean the function, I mean the indent. If you do
Quoted 7 lines
    static const char *indent = <const value>;
it should be practically the same as
    const char *indent = <const value>;
no?
hmm, I didn't want the compiler to instantiate the array on the stack. But I suppose I need to check the generated asm to see how clever it is.
Quoted 8 lines
Quoted 4 lines
It really shouldn't do that. As long as you're just juggling pointers to a
region in .rodata it should know exactly what's going on.
I'll double check when I get to the office.. making it static doesn't hurt and forces the compiler to do what we want (.rodata). Without the 'static' it might end up doing the same thing.

Quoted 6 lines

Well, every time i see a "static" declared variable inside a function scope my alarm bells ring :). So I'd prefer we had as few as possible.
In fact, I *think* what your line does is it puts the pointer into a global in .data which really isn't what we want.

it seems to do the identical thing in both cases (based on diffing objdump output)

BR,
-R

Alexander Graf

Applied

2017-07-28 22:25 UTC #14 lore raw

PATCH 4/4 efi_loader: indent entry/exit prints to show nesting level

In reply to Rob Clark

Quoted 5 lines

This should make it easier to see when a callback back to UEFI world calls back in to the u-boot world, and generally match up EFI_ENTRY() and EFI_EXIT() calls.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex

Alexander Graf

Applied

2017-07-28 22:28 UTC #15 lore raw

PATCH 1/4 efi_loader: only evaluate EFI_EXIT()'s ret once

In reply to Rob Clark

Quoted 9 lines

There are a couple spots doing things like:
   return EFI_EXIT(some_fxn(...));
which I handn't noticed before. With addition of printing return value in the EFI_EXIT() macro, now the fxn call was getting evaluated twice. Which we didn't really want.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex