On 5/6/19 10:39 AM, Graf, Alexander wrote:
>
> On 06.05.19 09:52, Heinrich Schuchardt wrote:
>> On 5/6/19 9:24 AM, Graf, Alexander wrote:
>>> On 06.05.19 07:42, Heinrich Schuchardt wrote:
>>>> If the file path is NULL, return EFI_INVALID_PARAMETER.
There is a nice contradiction between UEFI Spec 2.7 Errata B and the
UEFI SCT specfication.
The UEFI spec has:
EFI_NOT_FOUND: Both SourceBuffer and DevicePath are NULL
UEFI SCT Specification (2017): 3.4.1 LoadImage, 5.1.4.1.2
BS.LoadImage – LoadImage() returns EFI_INVALID_PARAMETER with NULL FilePath.
>>>> If the file path is invalid, return EFI_NOT_FOUND.
UEFI Spec 2.7 Errata B:
EFI_DEVICE_ERROR - Image was not loaded because the device returned a
read error.
UEFI SCT Specification (2017): 3.4.1 LoadImage, 5.1.4.1.4
BS.LoadImage – LoadImage() returns EFI_NOT_FOUND with a non-existent
FilePath.
>>>
>>> These 2 are a documented inEFI_LOAD_FILE_PROTOCOL.LoadFile(). It might
>>> be a good idea to indicate that for later reference.
>>>
>> See the last sentence of the commit message.
>
>
> I don't understand? The return values are not defined as part of generic
> LoadImage(), but as part of EFI_LOAD_FILE_PROTOCOL.LoadFile() which is
> pretty counterintuitive IMHO.
The definitions in the EFI_LOAD_FILE_PROTOCOL should not prescribe the
return values of LoadImage().
I guess we should stick with the UEFI spec in case of contradiction and
try to convince uefi.org that there is a bug in the SCT.
Best regards
Heinrich
>
>
>>
>>>> If the size of the source buffer is 0, return EFI_LOAD_ERROR.
>>>
>>> The spec says this should be EFI_INVALID_PARAMETER, no? Is this a spec
>>> or an SCT bug?
>> According to the UEFI 2.7A spec:
>> EFI_LOAD_IMAGE should be returned if the image is corrupt.
>>
>> UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
>> 5.1.4.1.6
>
>
> Hm, but LoadFile() explicitly says:
>
> EFI_INVALID_PARAMETER | FilePath is not a valid device path, or
> BufferSize is NULL.
>
> Or are we beyond that point here?
>
>
>>
>>>
>>>> If the parent image handle does not refer to a loaded image return
>>>> EFI_INVALID_PARAMETER.
>>>
>>> I agree that this is a good change, but where is the spec reference? I
>>> couldn't find anything.
>>>
>> UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
>> 5.1.4.1.1
>
>
> Could you please add that reference to the commit message?
>
>
>>
>>>> The change is required to reach UEFI SCT conformance.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
>>>
>>> IMHO this really should be a patch set with multiple individual changes,
>>> each changing one piece of spec conformance at a time.
>>>
>>>
>>> That way if we happen to break anything along the way, bisecting would
>>> point us to exactly the right point where things fell apart.
>>>
>>>
>>>> ---
>>>> v2
>>>> efi_load_image_from_path() must initalize *buffer even in case of
>>>> an error
>>>> ---
>>>> include/efi_loader.h | 1 +
>>>> lib/efi_loader/efi_boottime.c | 17 ++++++++----
>>>> lib/efi_loader/efi_root_node.c | 48
>>>> ++++++++++++++++++----------------
>>>> 3 files changed, 39 insertions(+), 27 deletions(-)
>>>>
>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>> index d3a1d4c465..07ef14ba1c 100644
>>>> --- a/include/efi_loader.h
>>>> +++ b/include/efi_loader.h
>>>> @@ -187,6 +187,7 @@ struct efi_handler {
>>>> */
>>>> enum efi_object_type {
>>>> EFI_OBJECT_TYPE_UNDEFINED = 0,
>>>> + EFI_OBJECT_TYPE_U_BOOT_FIRMWARE,
>>>
>>> This looks like an unrelated change?
>> No. We use efi_root as parent_image. We test if parent_image is an image
>> in LoadImage. So efi_root must be marked as image.
>
>
> I see, that was not obvious :).
>
> Alex
>
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
