Re: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Wed, May 25, 2016 at 02:39:57AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33, <daniel.kiper@xxxxxxxxxx> wrote:
> > There is a problem with place_string() which is used as early memory
> > allocator. It gets memory chunks starting from start symbol and
> > going down. Sadly this does not work when Xen is loaded using multiboot2
> > protocol because start lives on 1 MiB address. So, I tried to use
> > mem_lower address calculated by GRUB2. However, it works only on some
> > machines. There are machines in the wild (e.g. Dell PowerEdge R820)
> > which uses first ~640 KiB for boot services code or data... :-(((
> >
> > In case of multiboot2 protocol we need that place_string() only allocate
> > memory chunk for EFI memory map. However, I think that it should be fixed
> > instead of making another function used just in one case. I thought about
> > two solutions.
> >
> > 1) We could use native EFI allocation functions (e.g. AllocatePool()
> >    or AllocatePages()) to get memory chunk. However, later (somewhere
> >    in __start_xen()) we must copy its contents to safe place or reserve
> >    this in e820 memory map and map it in Xen virtual address space.
> >    In later case we must also care about conflicts with e.g. crash
> >    kernel regions which could be quite difficult.
>
> I don't see why that would be: Simply use an allocation type that
> doesn't lead to the area getting consumed as normal RAM. Nor do
> I see the kexec collision potential. Furthermore (and I think I've
> said so before) ARM is already using AllocatePool() - just with an
> unsuitable memory type -, so doing so on x86 too would allow for

Nope, they are using standard EfiLoaderData.

> efi_arch_allocate_mmap_buffer() to go away.

That would be great, so, I will think how to solve this issue.

> > Jan Beulich added 1b) Do away with efi_arch_allocate_mmap_buffer() and use
> >    AllocatePages() uniformly, perhaps with a per-arch specified memory type
> >    (by means of which you can control whether the memory contents will 
> > remain
> >    preserved until the time you want to look at it). That will eliminate the
> >    only place_string() you're concerned about, with a patch with better
> >    diffstat (largely due to the questionable arch hook gone).
> >
> > However, this solution does not solve conflicts problem described in #1
> > because EFI memory map is needed during Xen runtime after init phase.
> > So, finally we would get back to #1. Hmmm... Should I check how Linux
> > and others cope with that problem?
>
> Ah, here you mention it actually. Yet you don't explain what conflict
> potential you see once using EfiRuntimeServicesData for the allocation.

Good point! IMO, if crash kernel region conflicts with EfiRuntimeServices*
then we should display warning that it cannot be allocated. By the way,
once you mentioned that you have in your queue (I suppose that it is
extremely long) kdump patch which adds functionality to automatically
establish crash kernel region placement. I think that could solve (at
least partially) problem with conflicts. Could you post it?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel