Re: [PATCH v3 2/2] efi: efi_mem_reserve(): don't reserve through memblock after mm_init()

[Matt Fleming <matt@codeblueprint.co.uk>]

On Sun, 08 Jan, at 01:24:49AM, Nicolai Stange wrote:
> 
> Out of curiosity, I had a deeper look at the BootServices*-md
> requirement though:
> 
> > Another problem is that we never check that the reservation is covered
> > by a BootServicesData region, which are the only ones that are
> > guaranteed to be retained up to this point.
> 
> I think the "only ones that are guaranteed to be retained" part might
> not be completely correct: at least my firmware seems to report only the
> EFI_CONVENTIONAL_MEMORY, EFI_LOADER_DATA, EFI_LOADER_CODE,
> EFI_BOOT_SERVICES_CODE and EFI_BOOT_SERVICES_DATA as E820_RAM
> (I think that these mappings are dictated by table 15-330 of ACPI 6.1:
> "UEFI Memory Types and mapping to ACPI address range types").
> 
> This would mean, that memblock_x86_fill() adds only these regions to
> memblock.memory.
 
Data required at runtime should only be in EFI_LOADER* regions if it's
part of some setup_data object (see things like SETUP_EFI), and
subsequently has been memblock_reserve()'d at some point.

Nothing valuable should be in EFI_CONVENTIONAL_MEMORY because, by
definition, it's free memory. 

> free_all_bootmem() only operates on the (non-highmem) regions given by
> memblock.memory and thus, any region of a type different from the ones
> listed above would never get freed to the buddy allocator anyway, AFAICS.
 
This is true.

> Thus, the only md type where ranges efi_mem_reserve()'d therein aren't
> retained are EFI_CONVENTIONAL_MEMORY, EFI_LOADER_DATA and
> EFI_LOADER_CODE (and possibly highmem). Hopefully, nobody would ever
> call efi_mem_reserve() on such a range but that assumption might be
> wrong.

I would happily welcome some diagnostic checks to ensure we never get
silently stung by this.