| Message ID | 20190327044042.13707-2-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted, archived |
| Commit | bc8fc32855d27b2999ed6667af10123f341b3159 |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: rework bootefi/bootmgr | expand |
On 3/27/19 5:40 AM, AKASHI Takahiro wrote: > To meet UEFI spec v2.7a section 9.2, we should add > EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL to image handle, > instead of EFI_DEVICE_PATH_PROTOCOL. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > include/efi_api.h | 4 ++++ > include/efi_loader.h | 1 + > lib/efi_loader/efi_boottime.c | 19 ++++++++++++------- > lib/efi_loader/efi_image_loader.c | 2 ++ > 4 files changed, 19 insertions(+), 7 deletions(-) > > diff --git a/include/efi_api.h b/include/efi_api.h > index ccf608653d4a..63b703c951a7 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -333,6 +333,10 @@ struct efi_system_table { > EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, \ > 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) > > +#define LOADED_IMAGE_DEVICE_PATH_GUID \ > + EFI_GUID(0xbc62157e, 0x3e33, 0x4fec, \ > + 0x99, 0x20, 0x2d, 0x3b, 0x36, 0xd7, 0x50, 0xdf) > + > #define EFI_LOADED_IMAGE_PROTOCOL_REVISION 0x1000 > > struct efi_loaded_image { > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 512880ab8fbf..3f54e354bdcd 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -135,6 +135,7 @@ extern const efi_guid_t efi_guid_event_group_reset_system; > /* GUID of the device tree table */ > extern const efi_guid_t efi_guid_fdt; > extern const efi_guid_t efi_guid_loaded_image; > +extern const efi_guid_t efi_guid_loaded_image_device_path; > extern const efi_guid_t efi_guid_device_path_to_text_protocol; > extern const efi_guid_t efi_simple_file_system_protocol_guid; > extern const efi_guid_t efi_file_info_guid; > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 391681260c27..578b32a05ffd 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -1519,6 +1519,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > efi_status_t ret; > struct efi_loaded_image *info = NULL; > struct efi_loaded_image_obj *obj = NULL; > + struct efi_device_path *dp; > > /* In case of EFI_OUT_OF_RESOURCES avoid illegal free by caller. */ > *handle_ptr = NULL; > @@ -1542,15 +1543,19 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > > if (device_path) { > info->device_handle = efi_dp_find_obj(device_path, NULL); > - /* > - * When asking for the device path interface, return > - * bootefi_device_path > - */ > - ret = efi_add_protocol(&obj->header, > - &efi_guid_device_path, device_path); > - if (ret != EFI_SUCCESS) > + > + dp = efi_dp_append(device_path, file_path); I think we should pass the original device_path as a parameter to efi_setup_loaded_image instead of first splitting and then recombining devicepath elements. But let's leave this as a TODO after the rest of the refactoring. > + if (!dp) { > + ret = EFI_OUT_OF_RESOURCES; > goto failure; > + } > + } else { > + dp = NULL; > } > + ret = efi_add_protocol(&obj->header, > + &efi_guid_loaded_image_device_path, dp); I wondered about the loaded imaged device path protocol possibly being NULL. But the spec explicitely states: It is legal to call LoadImage() for a buffer in memory with a NULL DevicePath parameter. In this case, the Loaded Image Device Path Protocol is installed with a NULL interface pointer. Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > + if (ret != EFI_SUCCESS) > + goto failure; > > /* > * When asking for the loaded_image interface, just > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > index fe66e7b9ffe1..c6177b9ff832 100644 > --- a/lib/efi_loader/efi_image_loader.c > +++ b/lib/efi_loader/efi_image_loader.c > @@ -14,6 +14,8 @@ > const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > const efi_guid_t efi_guid_device_path = DEVICE_PATH_GUID; > const efi_guid_t efi_guid_loaded_image = LOADED_IMAGE_GUID; > +const efi_guid_t efi_guid_loaded_image_device_path > + = LOADED_IMAGE_DEVICE_PATH_GUID; > const efi_guid_t efi_simple_file_system_protocol_guid = > EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID; > const efi_guid_t efi_file_info_guid = EFI_FILE_INFO_GUID; >
On 3/27/19 6:58 AM, Heinrich Schuchardt wrote: > On 3/27/19 5:40 AM, AKASHI Takahiro wrote: >> To meet UEFI spec v2.7a section 9.2, we should add >> EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL to image handle, >> instead of EFI_DEVICE_PATH_PROTOCOL. >> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> --- Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Added to for efi-2019-07 branch.
On Wed, Mar 27, 2019 at 06:58:31AM +0100, Heinrich Schuchardt wrote: > On 3/27/19 5:40 AM, AKASHI Takahiro wrote: > > To meet UEFI spec v2.7a section 9.2, we should add > > EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL to image handle, > > instead of EFI_DEVICE_PATH_PROTOCOL. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > include/efi_api.h | 4 ++++ > > include/efi_loader.h | 1 + > > lib/efi_loader/efi_boottime.c | 19 ++++++++++++------- > > lib/efi_loader/efi_image_loader.c | 2 ++ > > 4 files changed, 19 insertions(+), 7 deletions(-) > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > index ccf608653d4a..63b703c951a7 100644 > > --- a/include/efi_api.h > > +++ b/include/efi_api.h > > @@ -333,6 +333,10 @@ struct efi_system_table { > > EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, \ > > 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) > > > > +#define LOADED_IMAGE_DEVICE_PATH_GUID \ > > + EFI_GUID(0xbc62157e, 0x3e33, 0x4fec, \ > > + 0x99, 0x20, 0x2d, 0x3b, 0x36, 0xd7, 0x50, 0xdf) > > + > > #define EFI_LOADED_IMAGE_PROTOCOL_REVISION 0x1000 > > > > struct efi_loaded_image { > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index 512880ab8fbf..3f54e354bdcd 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -135,6 +135,7 @@ extern const efi_guid_t efi_guid_event_group_reset_system; > > /* GUID of the device tree table */ > > extern const efi_guid_t efi_guid_fdt; > > extern const efi_guid_t efi_guid_loaded_image; > > +extern const efi_guid_t efi_guid_loaded_image_device_path; > > extern const efi_guid_t efi_guid_device_path_to_text_protocol; > > extern const efi_guid_t efi_simple_file_system_protocol_guid; > > extern const efi_guid_t efi_file_info_guid; > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > > index 391681260c27..578b32a05ffd 100644 > > --- a/lib/efi_loader/efi_boottime.c > > +++ b/lib/efi_loader/efi_boottime.c > > @@ -1519,6 +1519,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > > efi_status_t ret; > > struct efi_loaded_image *info = NULL; > > struct efi_loaded_image_obj *obj = NULL; > > + struct efi_device_path *dp; > > > > /* In case of EFI_OUT_OF_RESOURCES avoid illegal free by caller. */ > > *handle_ptr = NULL; > > @@ -1542,15 +1543,19 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > > > > if (device_path) { > > info->device_handle = efi_dp_find_obj(device_path, NULL); > > - /* > > - * When asking for the device path interface, return > > - * bootefi_device_path > > - */ > > - ret = efi_add_protocol(&obj->header, > > - &efi_guid_device_path, device_path); > > - if (ret != EFI_SUCCESS) > > + > > + dp = efi_dp_append(device_path, file_path); > > I think we should pass the original device_path as a parameter to > efi_setup_loaded_image instead of first splitting and then recombining > devicepath elements. Agree, but I didn't modify the code because efi_setup_loaded_image() is still used to load "efi_selftest" in bootefi. I hope that you will work on do_efi_selftest(). > But let's leave this as a TODO after the rest of the refactoring. > > > + if (!dp) { > > + ret = EFI_OUT_OF_RESOURCES; > > goto failure; > > + } > > + } else { > > + dp = NULL; > > } > > + ret = efi_add_protocol(&obj->header, > > + &efi_guid_loaded_image_device_path, dp); > > I wondered about the loaded imaged device path protocol possibly being > NULL. But the spec explicitely states: > > It is legal to call LoadImage() for a buffer in memory with a NULL > DevicePath parameter. In this case, the Loaded Image Device Path > Protocol is installed with a NULL interface pointer. Right, I have also found this statement. -Takahiro Akashi > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > + if (ret != EFI_SUCCESS) > > + goto failure; > > > > /* > > * When asking for the loaded_image interface, just > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > > index fe66e7b9ffe1..c6177b9ff832 100644 > > --- a/lib/efi_loader/efi_image_loader.c > > +++ b/lib/efi_loader/efi_image_loader.c > > @@ -14,6 +14,8 @@ > > const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > > const efi_guid_t efi_guid_device_path = DEVICE_PATH_GUID; > > const efi_guid_t efi_guid_loaded_image = LOADED_IMAGE_GUID; > > +const efi_guid_t efi_guid_loaded_image_device_path > > + = LOADED_IMAGE_DEVICE_PATH_GUID; > > const efi_guid_t efi_simple_file_system_protocol_guid = > > EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID; > > const efi_guid_t efi_file_info_guid = EFI_FILE_INFO_GUID; > > >
diff --git a/include/efi_api.h b/include/efi_api.h index ccf608653d4a..63b703c951a7 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -333,6 +333,10 @@ struct efi_system_table { EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, \ 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) +#define LOADED_IMAGE_DEVICE_PATH_GUID \ + EFI_GUID(0xbc62157e, 0x3e33, 0x4fec, \ + 0x99, 0x20, 0x2d, 0x3b, 0x36, 0xd7, 0x50, 0xdf) + #define EFI_LOADED_IMAGE_PROTOCOL_REVISION 0x1000 struct efi_loaded_image { diff --git a/include/efi_loader.h b/include/efi_loader.h index 512880ab8fbf..3f54e354bdcd 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -135,6 +135,7 @@ extern const efi_guid_t efi_guid_event_group_reset_system; /* GUID of the device tree table */ extern const efi_guid_t efi_guid_fdt; extern const efi_guid_t efi_guid_loaded_image; +extern const efi_guid_t efi_guid_loaded_image_device_path; extern const efi_guid_t efi_guid_device_path_to_text_protocol; extern const efi_guid_t efi_simple_file_system_protocol_guid; extern const efi_guid_t efi_file_info_guid; diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 391681260c27..578b32a05ffd 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1519,6 +1519,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, efi_status_t ret; struct efi_loaded_image *info = NULL; struct efi_loaded_image_obj *obj = NULL; + struct efi_device_path *dp; /* In case of EFI_OUT_OF_RESOURCES avoid illegal free by caller. */ *handle_ptr = NULL; @@ -1542,15 +1543,19 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, if (device_path) { info->device_handle = efi_dp_find_obj(device_path, NULL); - /* - * When asking for the device path interface, return - * bootefi_device_path - */ - ret = efi_add_protocol(&obj->header, - &efi_guid_device_path, device_path); - if (ret != EFI_SUCCESS) + + dp = efi_dp_append(device_path, file_path); + if (!dp) { + ret = EFI_OUT_OF_RESOURCES; goto failure; + } + } else { + dp = NULL; } + ret = efi_add_protocol(&obj->header, + &efi_guid_loaded_image_device_path, dp); + if (ret != EFI_SUCCESS) + goto failure; /* * When asking for the loaded_image interface, just diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index fe66e7b9ffe1..c6177b9ff832 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -14,6 +14,8 @@ const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; const efi_guid_t efi_guid_device_path = DEVICE_PATH_GUID; const efi_guid_t efi_guid_loaded_image = LOADED_IMAGE_GUID; +const efi_guid_t efi_guid_loaded_image_device_path + = LOADED_IMAGE_DEVICE_PATH_GUID; const efi_guid_t efi_simple_file_system_protocol_guid = EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID; const efi_guid_t efi_file_info_guid = EFI_FILE_INFO_GUID;
To meet UEFI spec v2.7a section 9.2, we should add EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL to image handle, instead of EFI_DEVICE_PATH_PROTOCOL. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- include/efi_api.h | 4 ++++ include/efi_loader.h | 1 + lib/efi_loader/efi_boottime.c | 19 ++++++++++++------- lib/efi_loader/efi_image_loader.c | 2 ++ 4 files changed, 19 insertions(+), 7 deletions(-)