efi_loader: EFI file paths should be DOS style

PATCH Applied Rob Clark 2 participants 8 messages Jul 24, 2017 – Jul 28, 2017

lore

Share & tools

lore

Rob Clark

author

2017-07-24 11:59 UTC #0 lore raw

PATCH 1/1 efi_loader: EFI file paths should be DOS style

shim.efi, for example, actually tries to parse this, but is expecting backslashes.

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

---
 cmd/bootefi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

cmd/bootefi.c +5 -1

index b6d5047301..689eb09942 100644
@@ -379,7 +379,7 @@ static int parse_partnum(const char *devnr)
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 {
 	char devname[32] = { 0 }; /* dp->str is u16[32] long */
-	char *colon;
+	char *colon, *s;
 
 #if defined(CONFIG_BLK) || CONFIG_IS_ENABLED(ISO_PARTITION)
 	desc = blk_get_dev(dev, simple_strtol(devnr, NULL, 10));

@@ -431,6 +431,10 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 	} else {
 		snprintf(devname, sizeof(devname), "%s", path);
 	}
+	/* DOS style file path: */
+	s = devname;
+	while ((s = strchr(s, '/')))
+		*s++ = '\\';
 	ascii2unicode(bootefi_image_path[0].str, devname);
 
 	loaded_image_info.device_handle = real_bootefi_device_path;

Signature · 1 line

2.13.0

Rob Clark

author

2017-07-24 11:59 UTC #1 lore raw

PATCH 1/1 efi_loader: log EFI return values too

Turns out this is rather useful to tracking down where things fail.

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

---
I've been carrying this around locally for a while.. but I find it useful and I expect others would too.

 include/efi_loader.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

include/efi_loader.h +4 -1

index 043b29edd3..98d69a6dab 100644
@@ -20,7 +20,10 @@
 	debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
 	} while(0)
 
-#define EFI_EXIT(ret) efi_exit_func(ret);
+#define EFI_EXIT(ret) ({ \
+	debug("EFI: Exit: %s: %u\n", __func__, (u32)((ret) & 0xffff)); \
+	efi_exit_func(ret); \
+	})
 
 extern struct efi_runtime_services efi_runtime_services;
 extern struct efi_system_table systab;

Signature · 1 line

2.13.0

Alexander Graf

2017-07-24 12:36 UTC #2 lore raw

PATCH 1/1 efi_loader: log EFI return values too

Quoted 15 lines · Rob Clark

Turns out this is rather useful to tracking down where things fail.
Signed-off-by Rob Clark <[email protected]>
---
I've been carrying this around locally for a while.. but I find it useful and I expect others would too.
  include/efi_loader.h | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
include/efi_loader.h +0 -0
index 043b29edd3..98d69a6dab 100644

include/efi_loader.h line 23

Quoted 5 lines

include/efi_loader.h +1 -1

@@ -20,7 +20,10 @@
  	debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
  	} while(0)
 
-#define EFI_EXIT(ret) efi_exit_func(ret);
+#define EFI_EXIT(ret) ({ \

For consistency, please follow the same construct as in EFI_ENTRY().

include/efi_loader.h line 24

Quoted 1 line

include/efi_loader.h +1 -0

+	debug("EFI: Exit: %s: %u\n", __func__, (u32)((ret) & 0xffff)); \

I guess you just want to mask out EFI_ERROR_MASK?

Alex

Rob Clark

author

2017-07-24 12:53 UTC #3 lore raw

PATCH 1/1 efi_loader: log EFI return values too

Quoted 14 lines · Alexander Graf

Quoted 11 lines · Rob Clark
Turns out this is rather useful to tracking down where things fail.
Signed-off-by Rob Clark <[email protected]>
---
I've been carrying this around locally for a while.. but I find it useful and I expect others would too.
  include/efi_loader.h | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

Quoted 12 lines

include/efi_loader.h line 23
Quoted 9 lines
include/efi_loader.h +1 -0
index 043b29edd3..98d69a6dab 100644
@@ -20,7 +20,10 @@
        debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
        } while(0)
  -#define EFI_EXIT(ret) efi_exit_func(ret);
+#define EFI_EXIT(ret) ({ \
For consistency, please follow the same construct as in EFI_ENTRY().

You mean the do { ... } while (0)? That won't work since EFI_EXIT() has to evaluate to ret. Or did I misunderstand you.

Quoted 5 lines

include/efi_loader.h line 24
Quoted 1 line
include/efi_loader.h +1 -0
+       debug("EFI: Exit: %s: %u\n", __func__, (u32)((ret) & 0xffff)); \
I guess you just want to mask out EFI_ERROR_MASK?

Yup.. is there a better way?

BR,
-R

Alexander Graf

2017-07-24 13:05 UTC #4 lore raw

PATCH 1/1 efi_loader: log EFI return values too

Quoted 28 lines · Rob Clark

Quoted 14 lines · Alexander Graf
Quoted 11 lines · Rob Clark
Turns out this is rather useful to tracking down where things fail.
Signed-off-by Rob Clark <[email protected]>
---
I've been carrying this around locally for a while.. but I find it useful and I expect others would too.
   include/efi_loader.h | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-)
Quoted 12 lines
include/efi_loader.h line 23
Quoted 9 lines
include/efi_loader.h +1 -0
index 043b29edd3..98d69a6dab 100644
@@ -20,7 +20,10 @@
         debug("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \
         } while(0)
   -#define EFI_EXIT(ret) efi_exit_func(ret);
+#define EFI_EXIT(ret) ({ \
For consistency, please follow the same construct as in EFI_ENTRY().

Quoted 2 lines

You mean the do { ... } while (0)? That won't work since EFI_EXIT() has to evaluate to ret. Or did I misunderstand you.

Ah, right. No, I just misunderstood the reason for the change :).

Quoted 8 lines

Quoted 6 lines
include/efi_loader.h line 24
Quoted 2 lines
include/efi_loader.h +1 -0
+       debug("EFI: Exit: %s: %u\n", __func__, (u32)((ret) & 0xffff)); \
I guess you just want to mask out EFI_ERROR_MASK?
Yup.. is there a better way?

Yeah, & ~EFI_ERROR_MASK :).

Alex

Rob Clark

author

2017-07-24 11:59 UTC #5 lore raw

PATCH 1/1 efi_loader: move guidcmp to header

In reply to Rob Clark

Want to re-use this for file protocol, which I'm working on.

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

---
 include/efi_loader.h          | 5 +++++
 lib/efi_loader/efi_boottime.c | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

include/efi_loader.h +5 -0

index 739404142a..b6bdf99a38 100644
@@ -171,6 +171,11 @@ static inline void ascii2unicoden(u16 *unicode, const char *ascii, unsigned n)
 	}
 }
 
+static inline int guidcmp(const efi_guid_t *g1, const efi_guid_t *g2)
+{
+	return memcmp(g1, g2, sizeof(efi_guid_t));
+}
+
 /*
  * Use these to indicate that your code / data should go into the EFI runtime
  * section and thus still be available when the OS is running

lib/efi_loader/efi_boottime.c +0 -5

index a45de39919..bcec080edc 100644
@@ -87,11 +87,6 @@ static efi_status_t efi_unsupported(const char *funcname)
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
-static int guidcmp(const efi_guid_t *g1, const efi_guid_t *g2)
-{
-	return memcmp(g1, g2, sizeof(efi_guid_t));
-}
-
 static unsigned long EFIAPI efi_raise_tpl(unsigned long new_tpl)
 {
 	EFI_ENTRY("0x%lx", new_tpl);

Signature · 1 line

2.13.0

Alexander Graf

Applied

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

[U-Boot] efi_loader: move guidcmp to header

Quoted 3 lines

Want to re-use this for file protocol, which I'm working on.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex

Alexander Graf

Applied

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

In reply to Rob Clark

Quoted 4 lines

shim.efi, for example, actually tries to parse this, but is expecting backslashes.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex