| Message ID | 20171010122309.25313-11-robdclark@gmail.com |
|---|---|
| State | Accepted |
| Commit | bf19273e81eb5e23c9bc14c3881f92a120565561 |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi_loader: patches for Shell.efi | expand |
On 10/10/2017 02:23 PM, Rob Clark wrote: > When we don't have a real device/image path, such as 'bootefi hello', > construct a mem-mapped device-path. > > This fixes 'bootefi hello' after devicepath refactoring. > > Fixes: 95c5553ea2 ("efi_loader: refactor boot device and loaded_image handling") > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > cmd/bootefi.c | 23 +++++++++++++++++++++++ > include/efi_api.h | 8 ++++++++ > include/efi_loader.h | 3 +++ > lib/efi_loader/efi_device_path.c | 24 ++++++++++++++++++++++++ > lib/efi_loader/efi_device_path_to_text.c | 9 +++++++++ > 5 files changed, 67 insertions(+) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 24958ada46..18176a1266 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -128,6 +128,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > { > struct efi_loaded_image loaded_image_info = {}; > struct efi_object loaded_image_info_obj = {}; > + struct efi_device_path *memdp = NULL; > ulong ret; > > ulong (*entry)(void *image_handle, struct efi_system_table *st) > @@ -136,6 +137,20 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > const efi_guid_t fdt_guid = EFI_FDT_GUID; > bootm_headers_t img = { 0 }; > > + /* > + * Special case for efi payload not loaded from disk, such as > + * 'bootefi hello' or for example payload loaded directly into > + * memory via jtag/etc: > + */ > + if (!device_path && !image_path) { > + printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); > + /* actual addresses filled in after efi_load_pe() */ > + memdp = efi_dp_from_mem(0, 0, 0); > + device_path = image_path = memdp; > + } else { > + assert(device_path && image_path); > + } > + > /* Initialize and populate EFI object list */ > if (!efi_obj_list_initalized) > efi_init_obj_list(); > @@ -182,6 +197,14 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > goto exit; > } > > + if (memdp) { > + struct efi_device_path_memory *mdp = (void *)memdp; > + mdp->memory_type = loaded_image_info.image_code_type; > + mdp->start_address = (uintptr_t)loaded_image_info.image_base; > + mdp->end_address = mdp->start_address + > + loaded_image_info.image_size; > + } > + > /* we don't support much: */ > env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", > "{ro,boot}(blob)0000000000000000"); > diff --git a/include/efi_api.h b/include/efi_api.h > index 9610d03d47..07b2af7020 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -299,8 +299,16 @@ struct efi_mac_addr { > } __packed; > > #define DEVICE_PATH_TYPE_HARDWARE_DEVICE 0x01 > +# define DEVICE_PATH_SUB_TYPE_MEMORY 0x03 > # define DEVICE_PATH_SUB_TYPE_VENDOR 0x04 > > +struct efi_device_path_memory { > + struct efi_device_path dp; > + u32 memory_type; > + u64 start_address; > + u64 end_address; > +} __packed; > + > struct efi_device_path_vendor { > struct efi_device_path dp; > efi_guid_t guid; > diff --git a/include/efi_loader.h b/include/efi_loader.h > index fa4e1cdb1c..db805e898f 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -269,6 +269,9 @@ struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part); > struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part, > const char *path); > struct efi_device_path *efi_dp_from_eth(void); > +struct efi_device_path *efi_dp_from_mem(uint32_t mem_type, > + uint64_t start_address, > + uint64_t end_address); > void efi_dp_split_file_path(struct efi_device_path *full_path, > struct efi_device_path **device_path, > struct efi_device_path **file_path); > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c > index 5d5c3b3464..f6e368e029 100644 > --- a/lib/efi_loader/efi_device_path.c > +++ b/lib/efi_loader/efi_device_path.c > @@ -538,6 +538,30 @@ struct efi_device_path *efi_dp_from_eth(void) > } > #endif > > +/* Construct a device-path for memory-mapped image */ > +struct efi_device_path *efi_dp_from_mem(uint32_t memory_type, > + uint64_t start_address, > + uint64_t end_address) > +{ > + struct efi_device_path_memory *mdp; > + void *buf, *start; > + > + start = buf = dp_alloc(sizeof(*mdp) + sizeof(END)); > + > + mdp = buf; > + mdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; > + mdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MEMORY; > + mdp->dp.length = sizeof(*mdp); > + mdp->memory_type = memory_type; > + mdp->start_address = start_address; > + mdp->end_address = end_address; > + buf = &mdp[1]; > + > + *((struct efi_device_path *)buf) = END; > + > + return start; > +} > + > /* > * Helper to split a full device path (containing both device and file > * parts) into it's constituent parts. > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c > index 1a5ef3919b..62771338f0 100644 > --- a/lib/efi_loader/efi_device_path_to_text.c > +++ b/lib/efi_loader/efi_device_path_to_text.c > @@ -24,6 +24,15 @@ static char *dp_unknown(char *s, struct efi_device_path *dp) > static char *dp_hardware(char *s, struct efi_device_path *dp) > { > switch (dp->sub_type) { > + case DEVICE_PATH_SUB_TYPE_MEMORY: { > + struct efi_device_path_memory *mdp = > + (struct efi_device_path_memory *)dp; > + s += sprintf(s, "/MemoryMapped(0x%x,0x%llx,0x%llx)", > + mdp->memory_type, > + mdp->start_address, > + mdp->end_address); > + break; > + } > case DEVICE_PATH_SUB_TYPE_VENDOR: { > struct efi_device_path_vendor *vdp = > (struct efi_device_path_vendor *)dp; > Providing a dummmy device path for bootefi hello and bootefi selftest is reasonable. Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
On 10.10.17 14:23, Rob Clark wrote: > When we don't have a real device/image path, such as 'bootefi hello', > construct a mem-mapped device-path. > > This fixes 'bootefi hello' after devicepath refactoring. > > Fixes: 95c5553ea2 ("efi_loader: refactor boot device and loaded_image handling") > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > cmd/bootefi.c | 23 +++++++++++++++++++++++ > include/efi_api.h | 8 ++++++++ > include/efi_loader.h | 3 +++ > lib/efi_loader/efi_device_path.c | 24 ++++++++++++++++++++++++ > lib/efi_loader/efi_device_path_to_text.c | 9 +++++++++ > 5 files changed, 67 insertions(+) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 24958ada46..18176a1266 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -128,6 +128,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > { > struct efi_loaded_image loaded_image_info = {}; > struct efi_object loaded_image_info_obj = {}; > + struct efi_device_path *memdp = NULL; > ulong ret; > > ulong (*entry)(void *image_handle, struct efi_system_table *st) > @@ -136,6 +137,20 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > const efi_guid_t fdt_guid = EFI_FDT_GUID; > bootm_headers_t img = { 0 }; > > + /* > + * Special case for efi payload not loaded from disk, such as > + * 'bootefi hello' or for example payload loaded directly into > + * memory via jtag/etc: > + */ > + if (!device_path && !image_path) { > + printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); > + /* actual addresses filled in after efi_load_pe() */ > + memdp = efi_dp_from_mem(0, 0, 0); > + device_path = image_path = memdp; > + } else { > + assert(device_path && image_path); > + } > + > /* Initialize and populate EFI object list */ > if (!efi_obj_list_initalized) > efi_init_obj_list(); > @@ -182,6 +197,14 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > goto exit; > } > > + if (memdp) { > + struct efi_device_path_memory *mdp = (void *)memdp; > + mdp->memory_type = loaded_image_info.image_code_type; > + mdp->start_address = (uintptr_t)loaded_image_info.image_base; > + mdp->end_address = mdp->start_address + > + loaded_image_info.image_size; > + } > + memdp gets leaked after bootefi is done. Putting it on the stack would at least remove that problem ;). We currently expect to only return from bootefi when a payload was successfully quit. Alex
On Wed, Oct 11, 2017 at 10:59 AM, Alexander Graf <agraf@suse.de> wrote: > > > On 10.10.17 14:23, Rob Clark wrote: >> When we don't have a real device/image path, such as 'bootefi hello', >> construct a mem-mapped device-path. >> >> This fixes 'bootefi hello' after devicepath refactoring. >> >> Fixes: 95c5553ea2 ("efi_loader: refactor boot device and loaded_image handling") >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> cmd/bootefi.c | 23 +++++++++++++++++++++++ >> include/efi_api.h | 8 ++++++++ >> include/efi_loader.h | 3 +++ >> lib/efi_loader/efi_device_path.c | 24 ++++++++++++++++++++++++ >> lib/efi_loader/efi_device_path_to_text.c | 9 +++++++++ >> 5 files changed, 67 insertions(+) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index 24958ada46..18176a1266 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -128,6 +128,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> { >> struct efi_loaded_image loaded_image_info = {}; >> struct efi_object loaded_image_info_obj = {}; >> + struct efi_device_path *memdp = NULL; >> ulong ret; >> >> ulong (*entry)(void *image_handle, struct efi_system_table *st) >> @@ -136,6 +137,20 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> const efi_guid_t fdt_guid = EFI_FDT_GUID; >> bootm_headers_t img = { 0 }; >> >> + /* >> + * Special case for efi payload not loaded from disk, such as >> + * 'bootefi hello' or for example payload loaded directly into >> + * memory via jtag/etc: >> + */ >> + if (!device_path && !image_path) { >> + printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); >> + /* actual addresses filled in after efi_load_pe() */ >> + memdp = efi_dp_from_mem(0, 0, 0); >> + device_path = image_path = memdp; >> + } else { >> + assert(device_path && image_path); >> + } >> + >> /* Initialize and populate EFI object list */ >> if (!efi_obj_list_initalized) >> efi_init_obj_list(); >> @@ -182,6 +197,14 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> goto exit; >> } >> >> + if (memdp) { >> + struct efi_device_path_memory *mdp = (void *)memdp; >> + mdp->memory_type = loaded_image_info.image_code_type; >> + mdp->start_address = (uintptr_t)loaded_image_info.image_base; >> + mdp->end_address = mdp->start_address + >> + loaded_image_info.image_size; >> + } >> + > > memdp gets leaked after bootefi is done. Putting it on the stack would > at least remove that problem ;). We currently expect to only return from > bootefi when a payload was successfully quit. > dp's that aren't allocated from pool are a bad idea, in some cases they get free'd by the payload. (Well not really in this particular case but it feels like a bad idea to mix/match how we allocate dp's.. also, it needs an /End node.) I guess it isn't such a critical leak, but the right solution would be to efi_free_pool() it.. BR, -R
> When we don't have a real device/image path, such as 'bootefi hello', > construct a mem-mapped device-path. > > This fixes 'bootefi hello' after devicepath refactoring. > > Fixes: 95c5553ea2 ("efi_loader: refactor boot device and loaded_image handling") > Signed-off-by: Rob Clark <robdclark@gmail.com> > Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Thanks, applied to efi-next Alex
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 24958ada46..18176a1266 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -128,6 +128,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, { struct efi_loaded_image loaded_image_info = {}; struct efi_object loaded_image_info_obj = {}; + struct efi_device_path *memdp = NULL; ulong ret; ulong (*entry)(void *image_handle, struct efi_system_table *st) @@ -136,6 +137,20 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, const efi_guid_t fdt_guid = EFI_FDT_GUID; bootm_headers_t img = { 0 }; + /* + * Special case for efi payload not loaded from disk, such as + * 'bootefi hello' or for example payload loaded directly into + * memory via jtag/etc: + */ + if (!device_path && !image_path) { + printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); + /* actual addresses filled in after efi_load_pe() */ + memdp = efi_dp_from_mem(0, 0, 0); + device_path = image_path = memdp; + } else { + assert(device_path && image_path); + } + /* Initialize and populate EFI object list */ if (!efi_obj_list_initalized) efi_init_obj_list(); @@ -182,6 +197,14 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, goto exit; } + if (memdp) { + struct efi_device_path_memory *mdp = (void *)memdp; + mdp->memory_type = loaded_image_info.image_code_type; + mdp->start_address = (uintptr_t)loaded_image_info.image_base; + mdp->end_address = mdp->start_address + + loaded_image_info.image_size; + } + /* we don't support much: */ env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", "{ro,boot}(blob)0000000000000000"); diff --git a/include/efi_api.h b/include/efi_api.h index 9610d03d47..07b2af7020 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -299,8 +299,16 @@ struct efi_mac_addr { } __packed; #define DEVICE_PATH_TYPE_HARDWARE_DEVICE 0x01 +# define DEVICE_PATH_SUB_TYPE_MEMORY 0x03 # define DEVICE_PATH_SUB_TYPE_VENDOR 0x04 +struct efi_device_path_memory { + struct efi_device_path dp; + u32 memory_type; + u64 start_address; + u64 end_address; +} __packed; + struct efi_device_path_vendor { struct efi_device_path dp; efi_guid_t guid; diff --git a/include/efi_loader.h b/include/efi_loader.h index fa4e1cdb1c..db805e898f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -269,6 +269,9 @@ struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part); struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part, const char *path); struct efi_device_path *efi_dp_from_eth(void); +struct efi_device_path *efi_dp_from_mem(uint32_t mem_type, + uint64_t start_address, + uint64_t end_address); void efi_dp_split_file_path(struct efi_device_path *full_path, struct efi_device_path **device_path, struct efi_device_path **file_path); diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5d5c3b3464..f6e368e029 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -538,6 +538,30 @@ struct efi_device_path *efi_dp_from_eth(void) } #endif +/* Construct a device-path for memory-mapped image */ +struct efi_device_path *efi_dp_from_mem(uint32_t memory_type, + uint64_t start_address, + uint64_t end_address) +{ + struct efi_device_path_memory *mdp; + void *buf, *start; + + start = buf = dp_alloc(sizeof(*mdp) + sizeof(END)); + + mdp = buf; + mdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + mdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MEMORY; + mdp->dp.length = sizeof(*mdp); + mdp->memory_type = memory_type; + mdp->start_address = start_address; + mdp->end_address = end_address; + buf = &mdp[1]; + + *((struct efi_device_path *)buf) = END; + + return start; +} + /* * Helper to split a full device path (containing both device and file * parts) into it's constituent parts. diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 1a5ef3919b..62771338f0 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -24,6 +24,15 @@ static char *dp_unknown(char *s, struct efi_device_path *dp) static char *dp_hardware(char *s, struct efi_device_path *dp) { switch (dp->sub_type) { + case DEVICE_PATH_SUB_TYPE_MEMORY: { + struct efi_device_path_memory *mdp = + (struct efi_device_path_memory *)dp; + s += sprintf(s, "/MemoryMapped(0x%x,0x%llx,0x%llx)", + mdp->memory_type, + mdp->start_address, + mdp->end_address); + break; + } case DEVICE_PATH_SUB_TYPE_VENDOR: { struct efi_device_path_vendor *vdp = (struct efi_device_path_vendor *)dp;
When we don't have a real device/image path, such as 'bootefi hello', construct a mem-mapped device-path. This fixes 'bootefi hello' after devicepath refactoring. Fixes: 95c5553ea2 ("efi_loader: refactor boot device and loaded_image handling") Signed-off-by: Rob Clark <robdclark@gmail.com> --- cmd/bootefi.c | 23 +++++++++++++++++++++++ include/efi_api.h | 8 ++++++++ include/efi_loader.h | 3 +++ lib/efi_loader/efi_device_path.c | 24 ++++++++++++++++++++++++ lib/efi_loader/efi_device_path_to_text.c | 9 +++++++++ 5 files changed, 67 insertions(+)