xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Kiper <daniel.kiper@oracle.com>
To: xen-devel@lists.xenproject.org
Cc: jgross@suse.com, andrew.cooper3@citrix.com,
	stefano.stabellini@eu.citrix.com, cardoe@cardoe.com,
	pgnet.dev@gmail.com, ning.sun@intel.com, david.vrabel@citrix.com,
	jbeulich@suse.com, qiaowei.ren@intel.com,
	richard.l.maliszewski@intel.com, gang.wei@intel.com,
	fu.wei@linaro.org
Subject: Re: [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator
Date: Tue, 5 Jul 2016 20:26:36 +0200	[thread overview]
Message-ID: <20160705182636.GH18259@olila.local.net-space.pl> (raw)
In-Reply-To: <1460723596-13261-13-git-send-email-daniel.kiper@oracle.com>

On Fri, Apr 15, 2016 at 02:33:12PM +0200, Daniel Kiper 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.

I have done more experiments, read more code, etc. You can find results below.

> 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.

I have checked Linux kernel code. It allocates buffer for memory map using
EFI API and later reserve it in e820 memory map. Simple. This should work
for us too but...

>    In later case we must also care about conflicts with e.g. crash
>    kernel regions which could be quite difficult.

This is not a problem since Xen can choose dynamically placement of kdump
region during boot phase and there is no requirement to specify it in boot
command line. This means that it will avoid all allocated/reserved regions
including EFI memory map. However, there is one potential problem which
cannot be avoided simply with current EFI spec. I think about conflicts
with trampoline. It must live below 1 MiB. However, there is not something
similar to "AllocateMaxAddress" option for AllocatePages() which would
ask EFI to allocate memory above a given address (Hmmm... Why UEFI designers
did not added such option, e.g. "AllocateMinAddress"? For me it is obvious
thing if we have "AllocateMaxAddress"). So, it means that we cannot simply
say "give me a memory chunk above 1 MiB". AIUI, Linux guys do not care,
hope that all EFI platforms are smart and AllocatePages() tries hard to
avoid everything below 1 MiB. We can go this way too. However, I am almost
sure that sooner or later we will find crazy platforms which allocate memory
from 0-1 MiB region. We can avoid this by getting EFI memory map, looking for
free regions above 1 MiB and then trying to allocate memory chunk using
AllocatePages() with "AllocateAddress". Does it make sense?

> 2) We may allocate memory area statically somewhere in Xen code which
>    could be used as memory pool for early dynamic allocations. Looks
>    quite simple. Additionally, it would not depend on EFI at all and
>    could be used on legacy BIOS platforms if we need it. However, we
>    must carefully choose size of this pool. We do not want increase
>    Xen binary size too much and waste too much memory but also we must fit
>    at least memory map on x86 EFI platforms. As I saw on small machine,
>    e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more
>    than 200 entries. Every entry on x86-64 platform is 40 bytes in size.
>    So, it means that we need more than 8 KiB for EFI memory map only.
>    Additionally, if we want to use this memory pool for Xen and modules
>    command line storage (it would be used when xen.efi is executed as EFI
>    application) then we should add, I think, about 1 KiB. In this case,
>    to be on safe side, we should assume at least 64 KiB pool for early
>    memory allocations, which is about 4 times of our earlier calculations.
>    However, during discussion on Xen-devel Jan Beulich suggested that
>    just in case we should use 1 MiB memory pool like it was in original
>    place_string() implementation. So, let's use 1 MiB as it was proposed.
>    If we think that we should not waste unallocated memory in the pool
>    on running system then we can mark this region as __initdata and move
>    all required data to dynamically allocated places somewhere in __start_xen().

2a) We can create something like .init.bss and put this thing at the end of
    regular .bss section. Then allocate memory chunks starting from lowest
    address. After init phase we can free unused memory as in case of .init.text
    or .init.data sections. This way we do not need allocate any space in
    image file and freeing of unused memory should be simple. What do you
    think about that one?

Daniel

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

  parent reply	other threads:[~2016-07-05 18:26 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-15 12:33 [PATCH v3 00/16] x86: multiboot2 protocol support Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 01/16] x86/boot: do not create unwind tables Daniel Kiper
2016-04-15 15:45   ` Andrew Cooper
2016-04-15 12:33 ` [PATCH v3 02/16] x86: zero BSS using stosl instead of stosb Daniel Kiper
2016-04-15 13:57   ` Konrad Rzeszutek Wilk
2016-04-15 15:48   ` Andrew Cooper
2016-04-15 12:33 ` [PATCH v3 03/16] x86/boot: call reloc() using cdecl calling convention Daniel Kiper
2016-04-15 15:56   ` Andrew Cooper
2016-06-17  8:41     ` Daniel Kiper
2016-06-17  9:30       ` Jan Beulich
2016-05-24  8:42   ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 04/16] x86/boot/reloc: create generic alloc and copy functions Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 05/16] x86/boot: use %ecx instead of %eax Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 06/16] x86/boot/reloc: Rename some variables and rearrange code a bit Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 07/16] x86/boot: create *.lnk files with linker script Daniel Kiper
2016-04-15 14:04   ` Konrad Rzeszutek Wilk
2016-05-24  9:05   ` Jan Beulich
2016-05-24 12:28     ` Daniel Kiper
2016-05-24 12:52       ` Jan Beulich
2016-06-17  9:06         ` Daniel Kiper
2016-06-17 10:04           ` Jan Beulich
2016-06-17 10:34             ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 08/16] x86: add multiboot2 protocol support Daniel Kiper
2016-05-24 15:46   ` Jan Beulich
2016-05-25 16:34     ` Daniel Kiper
2016-05-26 10:28       ` Andrew Cooper
2016-05-27  8:08         ` Jan Beulich
2016-05-27  8:13           ` Andrew Cooper
2016-05-27  8:24             ` Jan Beulich
2016-05-27  8:11       ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 09/16] efi: explicitly define efi struct in xen/arch/x86/efi/stub.c Daniel Kiper
2016-05-25  7:03   ` Jan Beulich
2016-05-25 16:45     ` Daniel Kiper
2016-05-27  8:16       ` Jan Beulich
2016-06-01 15:07         ` Daniel Kiper
2016-07-05 18:33         ` Daniel Kiper
2016-07-06  6:55           ` Jan Beulich
2016-07-06 10:27             ` Daniel Kiper
2016-07-06 12:00               ` Jan Beulich
2016-07-06 12:55                 ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 10/16] efi: create efi_enabled() Daniel Kiper
2016-05-25  7:20   ` Jan Beulich
2016-05-25 17:15     ` Daniel Kiper
2016-05-26 10:31       ` Andrew Cooper
2016-05-27  8:22       ` Jan Beulich
2016-06-01 15:23         ` Daniel Kiper
2016-06-01 15:41           ` Jan Beulich
2016-06-01 19:28             ` Daniel Kiper
2016-06-02  8:06               ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 11/16] efi: build xen.gz with EFI code Daniel Kiper
2016-05-25  7:53   ` Jan Beulich
2016-05-25 19:07     ` Daniel Kiper
2016-05-27  8:31       ` Jan Beulich
2016-06-01 15:48         ` Daniel Kiper
2016-06-01 15:58           ` Jan Beulich
2016-06-01 19:39             ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator Daniel Kiper
2016-05-25  8:39   ` Jan Beulich
2016-05-25 19:48     ` Daniel Kiper
2016-05-27  8:37       ` Jan Beulich
2016-06-01 15:58         ` Daniel Kiper
2016-06-01 16:02           ` Jan Beulich
2016-06-01 19:53             ` Daniel Kiper
2016-06-02  8:11               ` Jan Beulich
2016-06-02 10:43                 ` Daniel Kiper
2016-06-02 11:10                   ` Jan Beulich
2016-06-01 16:01         ` Daniel Kiper
2016-07-05 18:26   ` Daniel Kiper [this message]
2016-07-06  7:22     ` Jan Beulich
2016-07-06 11:15       ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms Daniel Kiper
2016-05-25  9:32   ` Jan Beulich
2016-05-25 10:29     ` Jan Beulich
2016-05-25 21:02     ` Daniel Kiper
2016-05-27  9:02       ` Jan Beulich
2016-06-01 19:03         ` Daniel Kiper
2016-06-02  8:34           ` Jan Beulich
2016-06-02 16:12             ` Daniel Kiper
2016-06-03  9:26               ` Jan Beulich
2016-06-03 17:06                 ` Konrad Rzeszutek Wilk
2016-04-15 12:33 ` [PATCH v3 14/16] x86/boot: implement early command line parser in C Daniel Kiper
2016-05-25 10:33   ` Jan Beulich
2016-05-25 21:36     ` Daniel Kiper
2016-05-27  9:33       ` Jan Beulich
2016-06-02  8:15         ` Daniel Kiper
2016-06-02  8:39           ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 15/16 - RFC] x86: make Xen early boot code relocatable Daniel Kiper
2016-05-25 10:48   ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 16/16] x86: add multiboot2 protocol support for relocatable images Daniel Kiper
2016-05-25 11:03   ` Jan Beulich
2016-06-01 13:35     ` Daniel Kiper
2016-06-01 14:44       ` Jan Beulich
2016-06-01 19:16         ` Daniel Kiper
2016-06-02  8:41           ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160705182636.GH18259@olila.local.net-space.pl \
    --to=daniel.kiper@oracle.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=cardoe@cardoe.com \
    --cc=david.vrabel@citrix.com \
    --cc=fu.wei@linaro.org \
    --cc=gang.wei@intel.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=ning.sun@intel.com \
    --cc=pgnet.dev@gmail.com \
    --cc=qiaowei.ren@intel.com \
    --cc=richard.l.maliszewski@intel.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).