On 7/9/19 1:10 AM, Bryan O'Donoghue wrote:
These two patches fix some inconsistent usage around efi_add_memory_map().
The first patch fixes the case where there is a mapping for an address starting at 0 as is the case on RPI3. We should not print an error for this. efi_add_memory_map(start = 0, ...) succeeds but efi_carve_out_dt_rsv() does not properly parse the result code.
The second patch fixes the result code returned by efi_add_memory_map() in two instances. Returing zero is the same as returning EFI_SUCCESS, we should return one of the error codes from include/efi.h only, not zero to indicate failure.
Bryan O'Donoghue (2): efi_loader: Check the result of efi_add_memory_map against input addr efi_loader: Return non-zero for error in efi_add_memory_map()
cmd/bootefi.c | 4 ++-- lib/efi_loader/efi_memory.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Hello Bryon,
thanks for pointing out this design error.
I found no instance in our code where the return value of efi_add_memory_map() is used for anything else but checking if the operation was successful. It seems appropriate to change the signature of the function to
efi_status_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, bool overlap_only_ram);
and return EFI_SUCCESS or an error code. It would be very kind if you could send an updated patch.
Best regards
Heinrich