From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 27 Mar 2019 11:50:11 +0900 Subject: [U-Boot] [RFC 1/8] efi_loader: boottime: don't add device path protocol to image handle In-Reply-To: <7e84b00f-805a-f5b6-e459-d35c69d2d036@gmx.de> References: <20190305055337.3793-1-takahiro.akashi@linaro.org> <20190305055337.3793-2-takahiro.akashi@linaro.org> <20190306002705.GD20286@linaro.org> <1fddba27-03d8-86d8-01c4-830c7a92b2b1@gmx.de> <7e84b00f-805a-f5b6-e459-d35c69d2d036@gmx.de> Message-ID: <20190327025009.GP9937@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Mar 06, 2019 at 06:29:14AM +0100, Heinrich Schuchardt wrote: > On 3/6/19 6:04 AM, Heinrich Schuchardt wrote: > > On 3/6/19 1:27 AM, AKASHI Takahiro wrote: > >> On Tue, Mar 05, 2019 at 08:48:37PM +0100, Heinrich Schuchardt wrote: > >>> On 3/5/19 6:53 AM, AKASHI Takahiro wrote: > >>>> It is just wrong to add devcie path protocol to image handle. > >>>> > >>>> Signed-off-by: AKASHI Takahiro > >>>> --- > >>>> lib/efi_loader/efi_boottime.c | 11 +---------- > >>>> 1 file changed, 1 insertion(+), 10 deletions(-) > >>>> > >>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > >>>> index bd8b8a17ae71..7bd9c0a952d4 100644 > >>>> --- a/lib/efi_loader/efi_boottime.c > >>>> +++ b/lib/efi_loader/efi_boottime.c > >>>> @@ -1540,17 +1540,8 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > >>>> info->file_path = file_path; > >>>> info->system_table = &systab; > >>>> > >>>> - if (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); > >>> > >>> Installing the device path is not the problem. It is the GUID that is > >>> wrong. Use EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID here. > >> > >> Okay, but I believe that we need duplicate device_path here > >> before installing it as EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID. > >> > >> See this line: > >> > >>>> info->device_handle = efi_dp_find_obj(device_path, NULL); > >> > >> Normally, device_path is expected to be already associated with > >> another handle. We should not allow two handles to share one protocol(data). > >> That is also why I initially believed that add_protocol() should be removed. > > > > The spec says we should use a copy of the unchanged DevicePath parameter > > of LoadImage() which may be NULL. > > > > So we have to rework all callers to get the device_path parameter of > > efi_setup_loaded_image() right. > > > > Why are we constructing a dummy memory device path at all in cmd/bootefi? > > The commit message of patch bf19273e81eb "efi_loader: Add mem-mapped for > fallback" that introduced this does not give a valid answer as it is > explicitly allowable to call LoadImage with DevicePath = NULL if > SourceBuffer is provided. As far as I know, if we load EDK2's Shell.efi by calling LoadImage *without* DevicePath, it will fail to boot at some assertion check. -Takahiro Akashi > So I suggest we rid ourselves of the dummy device path with this patch > series. > > Best regards > > Heinrich