Message ID | 20220428045012.52286-1-takahiro.akashi@linaro.org |
---|---|
State | New |
Headers | show |
Series | efi_loader: fix a problem in loading an image from a short-path | expand |
On 4/28/22 06:50, AKASHI Takahiro wrote: > Booting from a short-form device path which starts with the first element > being a File Path Media Device Path failed because it doesn't contain > any valid device with simple file system protocol and efi_dp_find_obj() > in efi_load_image_from_path() will return NULL. > For instance, > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/\helloworld.efi > -> shortened version: /\helloworld.efi Thanks for enabling this. Please, enhance test/py/tests/test_efi_bootmgr to test booting from a filename only device path. > > With this patch applied, all the media devices with simple file system > protocol are enumerated and the boot manager attempts to boot temporarily > generated device paths one-by-one. > > This new implementation is still a bit incompatible with the UEFI > specification in terms of: > * not creating real boot options The UEFI specification has: "These new boot options must not be saved to non volatile storage, and may not be added to BootOrder." Is there really something missing? > * not distinguishing removable media and fix media struct blk_desc has field 'removable' which is mirrored in struct efi_disk_obj. mmc_bind() always sets removable to 1 irrespective of the device being eMMC or SD-card. I guess this would need to be fixed. > (See section 3.1.3 "Boot Options".) > > But it still gives us a closer and better solution than the current. > > Fixes: commit 9cdf470274ff ("efi_loader: support booting via short-form device-path") > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > include/efi_loader.h | 3 ++ > lib/efi_loader/efi_boottime.c | 87 +++++++++++++++++++++++++++++++---- > lib/efi_loader/efi_file.c | 35 ++++++++++---- > 3 files changed, 107 insertions(+), 18 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index ba79a9afb404..9730c1375a55 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -661,6 +661,9 @@ void efi_signal_event(struct efi_event *event); > struct efi_simple_file_system_protocol *efi_simple_file_system( > struct blk_desc *desc, int part, struct efi_device_path *dp); > > +/* open file from simple file system */ > +struct efi_file_handle *efi_file_from_fs(struct efi_simple_file_system_protocol *v, > + struct efi_device_path *fp); > /* open file from device-path: */ > struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp); > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 5bcb8253edba..39b0e8f7ade0 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -1868,19 +1868,21 @@ out: > } > > /** > - * efi_load_image_from_file() - load an image from file system > + * __efi_load_image_from_file() - load an image from file system > * > * Read a file into a buffer allocated as EFI_BOOT_SERVICES_DATA. It is the > * callers obligation to update the memory type as needed. > * > + * @v: simple file system Please, use a meaningful variable name. > * @file_path: the path of the image to load > * @buffer: buffer containing the loaded image > * @size: size of the loaded image > * Return: status code > */ > static > -efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, > - void **buffer, efi_uintn_t *size) > +efi_status_t __efi_load_image_from_file(struct efi_simple_file_system_protocol *v, > + struct efi_device_path *file_path, > + void **buffer, efi_uintn_t *size) > { > struct efi_file_handle *f; > efi_status_t ret; > @@ -1888,7 +1890,11 @@ efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, > efi_uintn_t bs; > > /* Open file */ > - f = efi_file_from_path(file_path); > + if (v) > + f = efi_file_from_fs(v, file_path); > + else > + /* file_path should have a device path */ > + f = efi_file_from_path(file_path); > if (!f) > return EFI_NOT_FOUND; > > @@ -1921,6 +1927,64 @@ error: > return ret; > } > > +/** > + * efi_load_image_from_file() - load an image from file system > + * > + * Read a file into a buffer allocated as EFI_BOOT_SERVICES_DATA. It is the > + * callers obligation to update the memory type as needed. > + * > + * @file_path: the path of the image to load > + * @buffer: buffer containing the loaded image > + * @size: size of the loaded image > + * Return: status code > + */ > +static > +efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, > + void **buffer, efi_uintn_t *size) > +{ > + efi_uintn_t no_handles; > + efi_handle_t *handles; > + struct efi_handler *handler; > + struct efi_simple_file_system_protocol *fs; > + int i; > + efi_status_t ret; > + > + /* if a file_path contains a device path */ > + if (!EFI_DP_TYPE(file_path, MEDIA_DEVICE, FILE_PATH)) > + return __efi_load_image_from_file(NULL, file_path, buffer, size); > + > + /* no explicit device specified */ > + ret = EFI_CALL(efi_locate_handle_buffer( > + BY_PROTOCOL, > + &efi_simple_file_system_protocol_guid, > + NULL, > + &no_handles, > + &handles)); > + if (ret != EFI_SUCCESS) > + return ret; > + if (!no_handles) > + return EFI_NOT_FOUND; > + > + for (i = 0; i < no_handles; i++) { > + ret = efi_search_protocol(handles[i], > + &efi_simple_file_system_protocol_guid, > + &handler); > + if (ret != EFI_SUCCESS) > + /* unlikely */ > + continue; > + > + fs = handler->protocol_interface; > + if (!fs) > + continue; > + > + ret = __efi_load_image_from_file(fs, file_path, buffer, size); > + if (ret == EFI_SUCCESS) > + return ret; > + } > + > + return EFI_NOT_FOUND; > +} > + > /** > * efi_load_image_from_path() - load an image using a file path > * > @@ -1940,7 +2004,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy, > { > efi_handle_t device; > efi_status_t ret; > - struct efi_device_path *dp, *rem; > + struct efi_device_path *rem; > struct efi_load_file_protocol *load_file_protocol = NULL; > efi_uintn_t buffer_size; > uint64_t addr, pages; > @@ -1950,12 +2014,15 @@ efi_status_t efi_load_image_from_path(bool boot_policy, > *buffer = NULL; > *size = 0; > > - dp = file_path; > - device = efi_dp_find_obj(dp, NULL, &rem); > - ret = efi_search_protocol(device, &efi_simple_file_system_protocol_guid, > - NULL); > + /* try first for simple file system protocols */ > + ret = efi_load_image_from_file(file_path, buffer, size); > if (ret == EFI_SUCCESS) > - return efi_load_image_from_file(file_path, buffer, size); > + return ret; > + > + /* TODO: does this really make sense? */ > + device = efi_dp_find_obj(file_path, NULL, &rem); > + if (!device) > + return EFI_NOT_FOUND; > > ret = efi_search_protocol(device, &efi_guid_load_file_protocol, NULL); > if (ret == EFI_SUCCESS) { > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c > index 7a7077e6d032..2d6a432b168b 100644 > --- a/lib/efi_loader/efi_file.c > +++ b/lib/efi_loader/efi_file.c > @@ -1083,16 +1083,16 @@ static const struct efi_file_handle efi_file_handle_protocol = { > /** > * efi_file_from_path() - open file via device path > * > - * @fp: device path > + * @v: simple file system > + * @fp: file path > * Return: EFI_FILE_PROTOCOL for the file or NULL > */ > -struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > +struct efi_file_handle *efi_file_from_fs(struct efi_simple_file_system_protocol *v, > + struct efi_device_path *fp) > { > - struct efi_simple_file_system_protocol *v; > struct efi_file_handle *f; > efi_status_t ret; > > - v = efi_fs_from_path(fp); > if (!v) > return NULL; > > @@ -1100,10 +1100,6 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > if (ret != EFI_SUCCESS) > return NULL; > > - /* Skip over device-path nodes before the file path. */ > - while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) > - fp = efi_dp_next(fp); > - > /* > * Step through the nodes of the directory path until the actual file > * node is reached which is the final node in the device path. > @@ -1138,6 +1134,29 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > return f; > } > > +/** > + * efi_file_from_path() - open file via device path > + * > + * @fp: device path 'fp' does not resonate 'device path'. How about using dp? > + * Return: EFI_FILE_PROTOCOL for the file or NULL > + */ > +struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > +{ > + struct efi_simple_file_system_protocol *v; Please, provide a meaningful variable name. Best regards Heinrich > + > + v = efi_fs_from_path(fp); > + if (!v) > + return NULL; > + > + /* Skip over device-path nodes before the file path. */ > + while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) > + fp = efi_dp_next(fp); > + if (!fp) > + return NULL; > + > + return efi_file_from_fs(v, fp); > +} > + > static efi_status_t EFIAPI > efi_open_volume(struct efi_simple_file_system_protocol *this, > struct efi_file_handle **root)
On Fri, Apr 29, 2022 at 12:57:15PM +0200, Heinrich Schuchardt wrote: > On 4/28/22 06:50, AKASHI Takahiro wrote: > > Booting from a short-form device path which starts with the first element > > being a File Path Media Device Path failed because it doesn't contain > > any valid device with simple file system protocol and efi_dp_find_obj() > > in efi_load_image_from_path() will return NULL. > > For instance, > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/\helloworld.efi > > -> shortened version: /\helloworld.efi > > Thanks for enabling this. > > Please, enhance test/py/tests/test_efi_bootmgr to test booting from a > filename only device path. Okay, but you should have added tests in your patch. > > > > With this patch applied, all the media devices with simple file system > > protocol are enumerated and the boot manager attempts to boot temporarily > > generated device paths one-by-one. > > > > This new implementation is still a bit incompatible with the UEFI > > specification in terms of: > > * not creating real boot options > > The UEFI specification has: > > "These new boot options must not be saved to non volatile storage, > and may not be added to BootOrder." > > Is there really something missing? When the boot manager attempts to boot a short-form File Path Media Device Path, it will enumerate all removable media devices, followed by all fixed media devices, creating boot options for each device. ^^^^^^^^^^^^^^^^^^^^^ > > * not distinguishing removable media and fix media > > struct blk_desc has field 'removable' which is mirrored in struct > efi_disk_obj. Instead of relying on such an internal structure, we can and should use a public interface, efi_block_io(->media.removable_media). > mmc_bind() always sets removable to 1 irrespective of the device being > eMMC or SD-card. I guess this would need to be fixed. > > > (See section 3.1.3 "Boot Options".) > > > > But it still gives us a closer and better solution than the current. > > > > Fixes: commit 9cdf470274ff ("efi_loader: support booting via short-form device-path") > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > include/efi_loader.h | 3 ++ > > lib/efi_loader/efi_boottime.c | 87 +++++++++++++++++++++++++++++++---- > > lib/efi_loader/efi_file.c | 35 ++++++++++---- > > 3 files changed, 107 insertions(+), 18 deletions(-) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index ba79a9afb404..9730c1375a55 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -661,6 +661,9 @@ void efi_signal_event(struct efi_event *event); > > struct efi_simple_file_system_protocol *efi_simple_file_system( > > struct blk_desc *desc, int part, struct efi_device_path *dp); > > > > +/* open file from simple file system */ > > +struct efi_file_handle *efi_file_from_fs(struct efi_simple_file_system_protocol *v, > > + struct efi_device_path *fp); > > /* open file from device-path: */ > > struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp); > > > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > > index 5bcb8253edba..39b0e8f7ade0 100644 > > --- a/lib/efi_loader/efi_boottime.c > > +++ b/lib/efi_loader/efi_boottime.c > > @@ -1868,19 +1868,21 @@ out: > > } > > > > /** > > - * efi_load_image_from_file() - load an image from file system > > + * __efi_load_image_from_file() - load an image from file system > > * > > * Read a file into a buffer allocated as EFI_BOOT_SERVICES_DATA. It is the > > * callers obligation to update the memory type as needed. > > * > > + * @v: simple file system > > Please, use a meaningful variable name. I will fully re-write this patch. While efi boot manager should be responsible for handling a short-form device path starting with a file path, my current implementation is made in efi_load_image_from_path() hence LoadImage API. Instead, the logic of enumerating file_system_protocol devices will be added to try_load_entry()/efi_bootmgr_load(). -Takahiro Akashi > > * @file_path: the path of the image to load > > * @buffer: buffer containing the loaded image > > * @size: size of the loaded image > > * Return: status code > > */ > > static > > -efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, > > - void **buffer, efi_uintn_t *size) > > +efi_status_t __efi_load_image_from_file(struct efi_simple_file_system_protocol *v, > > + struct efi_device_path *file_path, > > + void **buffer, efi_uintn_t *size) > > { > > struct efi_file_handle *f; > > efi_status_t ret; > > @@ -1888,7 +1890,11 @@ efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, > > efi_uintn_t bs; > > > > /* Open file */ > > - f = efi_file_from_path(file_path); > > + if (v) > > + f = efi_file_from_fs(v, file_path); > > + else > > + /* file_path should have a device path */ > > + f = efi_file_from_path(file_path); > > if (!f) > > return EFI_NOT_FOUND; > > > > @@ -1921,6 +1927,64 @@ error: > > return ret; > > } > > > > +/** > > + * efi_load_image_from_file() - load an image from file system > > + * > > + * Read a file into a buffer allocated as EFI_BOOT_SERVICES_DATA. It is the > > + * callers obligation to update the memory type as needed. > > + * > > + * @file_path: the path of the image to load > > + * @buffer: buffer containing the loaded image > > + * @size: size of the loaded image > > + * Return: status code > > + */ > > +static > > +efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, > > + void **buffer, efi_uintn_t *size) > > +{ > > + efi_uintn_t no_handles; > > + efi_handle_t *handles; > > + struct efi_handler *handler; > > + struct efi_simple_file_system_protocol *fs; > > + int i; > > + efi_status_t ret; > > + > > + /* if a file_path contains a device path */ > > + if (!EFI_DP_TYPE(file_path, MEDIA_DEVICE, FILE_PATH)) > > + return __efi_load_image_from_file(NULL, file_path, buffer, size); > > + > > + /* no explicit device specified */ > > + ret = EFI_CALL(efi_locate_handle_buffer( > > + BY_PROTOCOL, > > + &efi_simple_file_system_protocol_guid, > > + NULL, > > + &no_handles, > > + &handles)); > > + if (ret != EFI_SUCCESS) > > + return ret; > > + if (!no_handles) > > + return EFI_NOT_FOUND; > > + > > + for (i = 0; i < no_handles; i++) { > > + ret = efi_search_protocol(handles[i], > > + &efi_simple_file_system_protocol_guid, > > + &handler); > > + if (ret != EFI_SUCCESS) > > + /* unlikely */ > > + continue; > > + > > + fs = handler->protocol_interface; > > + if (!fs) > > + continue; > > + > > + ret = __efi_load_image_from_file(fs, file_path, buffer, size); > > + if (ret == EFI_SUCCESS) > > + return ret; > > + } > > + > > + return EFI_NOT_FOUND; > > +} > > + > > /** > > * efi_load_image_from_path() - load an image using a file path > > * > > @@ -1940,7 +2004,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy, > > { > > efi_handle_t device; > > efi_status_t ret; > > - struct efi_device_path *dp, *rem; > > + struct efi_device_path *rem; > > struct efi_load_file_protocol *load_file_protocol = NULL; > > efi_uintn_t buffer_size; > > uint64_t addr, pages; > > @@ -1950,12 +2014,15 @@ efi_status_t efi_load_image_from_path(bool boot_policy, > > *buffer = NULL; > > *size = 0; > > > > - dp = file_path; > > - device = efi_dp_find_obj(dp, NULL, &rem); > > - ret = efi_search_protocol(device, &efi_simple_file_system_protocol_guid, > > - NULL); > > + /* try first for simple file system protocols */ > > + ret = efi_load_image_from_file(file_path, buffer, size); > > if (ret == EFI_SUCCESS) > > - return efi_load_image_from_file(file_path, buffer, size); > > + return ret; > > + > > + /* TODO: does this really make sense? */ > > + device = efi_dp_find_obj(file_path, NULL, &rem); > > + if (!device) > > + return EFI_NOT_FOUND; > > > > ret = efi_search_protocol(device, &efi_guid_load_file_protocol, NULL); > > if (ret == EFI_SUCCESS) { > > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c > > index 7a7077e6d032..2d6a432b168b 100644 > > --- a/lib/efi_loader/efi_file.c > > +++ b/lib/efi_loader/efi_file.c > > @@ -1083,16 +1083,16 @@ static const struct efi_file_handle efi_file_handle_protocol = { > > /** > > * efi_file_from_path() - open file via device path > > * > > - * @fp: device path > > + * @v: simple file system > > + * @fp: file path > > * Return: EFI_FILE_PROTOCOL for the file or NULL > > */ > > -struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > > +struct efi_file_handle *efi_file_from_fs(struct efi_simple_file_system_protocol *v, > > + struct efi_device_path *fp) > > { > > - struct efi_simple_file_system_protocol *v; > > struct efi_file_handle *f; > > efi_status_t ret; > > > > - v = efi_fs_from_path(fp); > > if (!v) > > return NULL; > > > > @@ -1100,10 +1100,6 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > > if (ret != EFI_SUCCESS) > > return NULL; > > > > - /* Skip over device-path nodes before the file path. */ > > - while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) > > - fp = efi_dp_next(fp); > > - > > /* > > * Step through the nodes of the directory path until the actual file > > * node is reached which is the final node in the device path. > > @@ -1138,6 +1134,29 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > > return f; > > } > > > > +/** > > + * efi_file_from_path() - open file via device path > > + * > > + * @fp: device path > > 'fp' does not resonate 'device path'. How about using dp? > > > + * Return: EFI_FILE_PROTOCOL for the file or NULL > > + */ > > +struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > > +{ > > + struct efi_simple_file_system_protocol *v; > > Please, provide a meaningful variable name. > > Best regards > > Heinrich > > > + > > + v = efi_fs_from_path(fp); > > + if (!v) > > + return NULL; > > + > > + /* Skip over device-path nodes before the file path. */ > > + while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) > > + fp = efi_dp_next(fp); > > + if (!fp) > > + return NULL; > > + > > + return efi_file_from_fs(v, fp); > > +} > > + > > static efi_status_t EFIAPI > > efi_open_volume(struct efi_simple_file_system_protocol *this, > > struct efi_file_handle **root) >
diff --git a/include/efi_loader.h b/include/efi_loader.h index ba79a9afb404..9730c1375a55 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -661,6 +661,9 @@ void efi_signal_event(struct efi_event *event); struct efi_simple_file_system_protocol *efi_simple_file_system( struct blk_desc *desc, int part, struct efi_device_path *dp); +/* open file from simple file system */ +struct efi_file_handle *efi_file_from_fs(struct efi_simple_file_system_protocol *v, + struct efi_device_path *fp); /* open file from device-path: */ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 5bcb8253edba..39b0e8f7ade0 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1868,19 +1868,21 @@ out: } /** - * efi_load_image_from_file() - load an image from file system + * __efi_load_image_from_file() - load an image from file system * * Read a file into a buffer allocated as EFI_BOOT_SERVICES_DATA. It is the * callers obligation to update the memory type as needed. * + * @v: simple file system * @file_path: the path of the image to load * @buffer: buffer containing the loaded image * @size: size of the loaded image * Return: status code */ static -efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, - void **buffer, efi_uintn_t *size) +efi_status_t __efi_load_image_from_file(struct efi_simple_file_system_protocol *v, + struct efi_device_path *file_path, + void **buffer, efi_uintn_t *size) { struct efi_file_handle *f; efi_status_t ret; @@ -1888,7 +1890,11 @@ efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, efi_uintn_t bs; /* Open file */ - f = efi_file_from_path(file_path); + if (v) + f = efi_file_from_fs(v, file_path); + else + /* file_path should have a device path */ + f = efi_file_from_path(file_path); if (!f) return EFI_NOT_FOUND; @@ -1921,6 +1927,64 @@ error: return ret; } +/** + * efi_load_image_from_file() - load an image from file system + * + * Read a file into a buffer allocated as EFI_BOOT_SERVICES_DATA. It is the + * callers obligation to update the memory type as needed. + * + * @file_path: the path of the image to load + * @buffer: buffer containing the loaded image + * @size: size of the loaded image + * Return: status code + */ +static +efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, + void **buffer, efi_uintn_t *size) +{ + efi_uintn_t no_handles; + efi_handle_t *handles; + struct efi_handler *handler; + struct efi_simple_file_system_protocol *fs; + int i; + efi_status_t ret; + + /* if a file_path contains a device path */ + if (!EFI_DP_TYPE(file_path, MEDIA_DEVICE, FILE_PATH)) + return __efi_load_image_from_file(NULL, file_path, buffer, size); + + /* no explicit device specified */ + ret = EFI_CALL(efi_locate_handle_buffer( + BY_PROTOCOL, + &efi_simple_file_system_protocol_guid, + NULL, + &no_handles, + &handles)); + if (ret != EFI_SUCCESS) + return ret; + if (!no_handles) + return EFI_NOT_FOUND; + + for (i = 0; i < no_handles; i++) { + ret = efi_search_protocol(handles[i], + &efi_simple_file_system_protocol_guid, + &handler); + if (ret != EFI_SUCCESS) + /* unlikely */ + continue; + + fs = handler->protocol_interface; + if (!fs) + continue; + + ret = __efi_load_image_from_file(fs, file_path, buffer, size); + if (ret == EFI_SUCCESS) + return ret; + } + + return EFI_NOT_FOUND; +} + /** * efi_load_image_from_path() - load an image using a file path * @@ -1940,7 +2004,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy, { efi_handle_t device; efi_status_t ret; - struct efi_device_path *dp, *rem; + struct efi_device_path *rem; struct efi_load_file_protocol *load_file_protocol = NULL; efi_uintn_t buffer_size; uint64_t addr, pages; @@ -1950,12 +2014,15 @@ efi_status_t efi_load_image_from_path(bool boot_policy, *buffer = NULL; *size = 0; - dp = file_path; - device = efi_dp_find_obj(dp, NULL, &rem); - ret = efi_search_protocol(device, &efi_simple_file_system_protocol_guid, - NULL); + /* try first for simple file system protocols */ + ret = efi_load_image_from_file(file_path, buffer, size); if (ret == EFI_SUCCESS) - return efi_load_image_from_file(file_path, buffer, size); + return ret; + + /* TODO: does this really make sense? */ + device = efi_dp_find_obj(file_path, NULL, &rem); + if (!device) + return EFI_NOT_FOUND; ret = efi_search_protocol(device, &efi_guid_load_file_protocol, NULL); if (ret == EFI_SUCCESS) { diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 7a7077e6d032..2d6a432b168b 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -1083,16 +1083,16 @@ static const struct efi_file_handle efi_file_handle_protocol = { /** * efi_file_from_path() - open file via device path * - * @fp: device path + * @v: simple file system + * @fp: file path * Return: EFI_FILE_PROTOCOL for the file or NULL */ -struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) +struct efi_file_handle *efi_file_from_fs(struct efi_simple_file_system_protocol *v, + struct efi_device_path *fp) { - struct efi_simple_file_system_protocol *v; struct efi_file_handle *f; efi_status_t ret; - v = efi_fs_from_path(fp); if (!v) return NULL; @@ -1100,10 +1100,6 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) if (ret != EFI_SUCCESS) return NULL; - /* Skip over device-path nodes before the file path. */ - while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) - fp = efi_dp_next(fp); - /* * Step through the nodes of the directory path until the actual file * node is reached which is the final node in the device path. @@ -1138,6 +1134,29 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) return f; } +/** + * efi_file_from_path() - open file via device path + * + * @fp: device path + * Return: EFI_FILE_PROTOCOL for the file or NULL + */ +struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) +{ + struct efi_simple_file_system_protocol *v; + + v = efi_fs_from_path(fp); + if (!v) + return NULL; + + /* Skip over device-path nodes before the file path. */ + while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) + fp = efi_dp_next(fp); + if (!fp) + return NULL; + + return efi_file_from_fs(v, fp); +} + static efi_status_t EFIAPI efi_open_volume(struct efi_simple_file_system_protocol *this, struct efi_file_handle **root)
Booting from a short-form device path which starts with the first element being a File Path Media Device Path failed because it doesn't contain any valid device with simple file system protocol and efi_dp_find_obj() in efi_load_image_from_path() will return NULL. For instance, /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/\helloworld.efi -> shortened version: /\helloworld.efi With this patch applied, all the media devices with simple file system protocol are enumerated and the boot manager attempts to boot temporarily generated device paths one-by-one. This new implementation is still a bit incompatible with the UEFI specification in terms of: * not creating real boot options * not distinguishing removable media and fix media (See section 3.1.3 "Boot Options".) But it still gives us a closer and better solution than the current. Fixes: commit 9cdf470274ff ("efi_loader: support booting via short-form device-path") Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- include/efi_loader.h | 3 ++ lib/efi_loader/efi_boottime.c | 87 +++++++++++++++++++++++++++++++---- lib/efi_loader/efi_file.c | 35 ++++++++++---- 3 files changed, 107 insertions(+), 18 deletions(-)