Re: [Xen-devel] [PATCH v12 05/10] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Fri, Jan 20, 2017 at 07:10:32AM -0700, Jan Beulich wrote:
> >>> On 20.01.17 at 14:46, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Fri, Jan 20, 2017 at 05:40:55AM -0700, Jan Beulich wrote:
> >> >>> On 20.01.17 at 12:43, <daniel.kiper@xxxxxxxxxx> wrote:
> >> > On Fri, Jan 20, 2017 at 02:46:47AM -0700, Jan Beulich wrote:
> >> >> >>> On 20.01.17 at 02:34, <daniel.kiper@xxxxxxxxxx> wrote:
> >> MB_TRAMPOLINE_SIZE is in no way the size of the trampoline in the
> >> first place. Perhaps TRAMPOLINE_SPACE (and then covering both
> >> parts)?
> >
> > So, I suppose that TRAMPOLINE_SPACE and TRAMPOLINE_STACK_SPACE is
> > OK for you? And I can agree that this is better naming.
>
> Again, I don't see why you would need TRAMPOLINE_STACK_SPACE.

If we use TRAMPOLINE_SPACE only for both trampoline and its stack
then ASSERT() is not very reliable. There is chance that trampoline
code will fill all space specified in TRAMPOLINE_SPACE and then there
will be no space for stack at all. However, I can agree that chance
is small if we assume 64 KiB space for trampoline and trampoline size
is about 10 KiB right now.

> >> >> two defines - except in the link time assertion you always use
> >> >> the sum of the two.
> >> >>
> >> >> > +#define MBI_SIZE                        (2 * PAGE_SIZE)
> >> >>
> >> >> On top of Doug's question - if it is needed at all, what does this
> >> >
> >> > Please look around reloc() call in xen/arch/x86/boot/head.S. You quickly
> >> > realize that there is following memory layout from top to bottom:
> >> >
> >> >         +------------------+
> >> >         | TRAMPOLINE_STACK |
> >> >         +------------------+
> >> >         |    TRAMPOLINE    |
> >> >         +------------------+
> >> >         |    mbi struct    |
> >> >         +------------------+
> >> >
> >> >> match up with, i.e. how come this is 2 pages (and not 1 or 3)?
> >> >
> >> > Just thought that 8 KiB will be sufficient for Xen/modules arguments,
> >> > memory map and other minor things.
> >>
> >> Even with a couple of dozen modules passed? But the primary
> >
> > Why not? The biggest thing is memory map. The rest are mostly
> > command line strings and a few pointers. Modules itself live
> > in different memory regions.
>
> Command line string may get lengthy.

Do you think that we should have more than 8 KiB there? Anyway,
I am more afraid about memory map size here.

> >> question was left unanswered anyway: Does this need placing in
> >> the low megabyte at all? And even if it does, especially if you
> >
> > I do not think so. I do not expect that anything in trampoline code
> > touches it. So, we can put it anywhere below 4 GiB. However, then
> > we need an allocator to properly allocate mbi struct region. And
> > at this boot stage this is not an easy thing.
>
> Depending for how long you need that data to stay around, there

IIRC, it is needed only during init phase. Probably later it can be dropped.

> may be places to put it without much risk of overflowing anything.

I am not sure about which places do you think.

Daniel

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