Re: [PATCH V2 4/4] efi/x86: make efi_memmap_reserve only insert into boot mem areas

[Nicolai Stange]

On Tue, Jan 17 2017, Ard Biesheuvel wrote:

> On 16 January 2017 at 02:45, Dave Young <dyoung@xxxxxxxxxx> wrote:
>> efi_mem_reserve cares only about boot services regions, for making sure
>> later efi_free_boot_services does not free areas which are still useful,
>> such as bgrt image buffer.
>>
>> So add a new argument to efi_memmap_insert for this purpose.
>>
>
> So what happens is we try to efi_mem_reserve() a regions that is not
> bootservices code or data?
> We shouldn't simply ignore it, because it is a serious condition.

The efi_mem_desc_lookup() call in efi_arch_mem_reserve() wouldn't return
anything and the latter would

  pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);

then.

This is so because efi_mem_desc_lookup() searches only for regions that
either
- are of type EFI_BOOT_SERVICES_DATA or EFI_RUNTIME_SERVICES_DATA
- or which have EFI_MEMORY_RUNTIME set already:

	if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
	    md->type != EFI_BOOT_SERVICES_DATA &&
	    md->type != EFI_RUNTIME_SERVICES_DATA) {
		continue;
	}

For EFI_RUNTIME_SERVICES_DATA and EFI_MEMORY_RUNTIME,
efi_arch_mem_reserve() would be a nop.

So we're fine here? Do you want to have a more descriptive error message
than "Failed to lookup EFI memory descriptor"?


For the other checks you suggested in that other thread, i.e. for the
post-slab_is_available() condition and so: let me wait until Dave's
series has stabilized (or even picked) and I'll submit patches for
what remains to be sanity checked then.

Also, since Dave eliminated the need for late efi_mem_reserve()'s,
my 20b1e22d01a4 ("x86/efi: Don't allocate memmap through memblock after
mm_init()") should certainly get reverted at some point.


Thanks,

Nicolai