Re: [PATCH v4 1/4] efi/boot.c: add file.need_to_free

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.09.2020 08:43, Roger Pau Monné wrote:
> On Mon, Sep 14, 2020 at 07:50:10AM -0400, Trammell Hudson wrote:
>> @@ -279,13 +280,13 @@ void __init noreturn blexit(const CHAR16 *str)
>>      if ( !efi_bs )
>>          efi_arch_halt();
>>  
>> -    if ( cfg.addr )
>> +    if ( cfg.addr && cfg.need_to_free )
>>          efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>> -    if ( kernel.addr )
>> +    if ( kernel.addr && kernel.need_to_free )
>>          efi_bs->FreePages(kernel.addr, PFN_UP(kernel.size));
>> -    if ( ramdisk.addr )
>> +    if ( ramdisk.addr && ramdisk.need_to_free )
>>          efi_bs->FreePages(ramdisk.addr, PFN_UP(ramdisk.size));
>> -    if ( xsm.addr )
>> +    if ( xsm.addr && xsm.need_to_free )
>>          efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));

All these look to be able to become just "if ( xyz.need_to_free )"
if ...

>> @@ -572,6 +573,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, 
>> CHAR16 *name,
>>                           HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
>>          ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
>>                                      PFN_UP(size), &file->addr);
>> +        file->need_to_free = true;
> 
> Strictly speaking, don't you need to set need_to_free only if
> AllocatePages has succeed?

... this was followed, so I think the adjustment wants making.

> I guess it doesn't matter much because addr
> would be zapped to 0 if allocation fails.

Perhaps this zapping then also becomes unnecessary, albeit I
didn't look very closely yet.

Jan