Re: [Xen-devel] [PATCH v2 22/23] x86: make Xen early boot code relocatable

[Jan Beulich]

>>> On 14.08.15 at 15:59, <address@hidden> wrote:
> On Fri, Aug 14, 2015 at 06:49:18AM -0600, Jan Beulich wrote:
>> >>> On 14.08.15 at 13:52, <address@hidden> wrote:
>> > On Tue, Aug 11, 2015 at 12:48:06PM -0400, Konrad Rzeszutek Wilk wrote:
>> >> On Mon, Jul 20, 2015 at 04:29:17PM +0200, Daniel Kiper wrote:
>> >> > diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
>> >> > index 87b3341..27481ac 100644
>> >> > --- a/xen/include/asm-x86/page.h
>> >> > +++ b/xen/include/asm-x86/page.h
>> >> > @@ -283,7 +283,7 @@ extern root_pgentry_t 
> idle_pg_table[ROOT_PAGETABLE_ENTRIES];
>> >> >  extern l2_pgentry_t  *compat_idle_pg_table_l2;
>> >> >  extern unsigned int   m2p_compat_vstart;
>> >> >  extern l2_pgentry_t l2_xenmap[L2_PAGETABLE_ENTRIES],
>> >> > -    l2_bootmap[L2_PAGETABLE_ENTRIES];
>> >> > +    l2_bootmap[4*L2_PAGETABLE_ENTRIES];
>> >>
>> >> ? Why do we need to expand this to be 16kB?
>> >
>> > TBH, we need 8 KiB in the worst case. The worst case is when
>> > next GiB starts (e.g. 1 GiB ends and 2 GiB starts) in the middle
>> > of Xen image. In this situation we must hook up lower l2_bootmap
>> > table with lower l3_bootmap entry, higher l2_bootmap table with
>> > higher l3_bootmap entry and finally fill l2_bootmap relevant
>> > tables in proper way. Sadly, this method requires more calculations.
>> > To avoid that I have added 3 l2_bootmap tables and simply hook up
>> > one after another with relevant l3_bootmap entries. However, if
>> > you wish we can reduce number of l2_bootmap tables to two. This
>> > way code will be more complicated but we will save about 8 KiB.
>>
>> Wouldn't it be better (simpler) to enforce, say, 16Mb alignment
>> in the PE32+ header (which the EFI loader would then honor)?
> 
> Good idea but then we must enforce this for multiboot protocol (v1 and v2) 
> too.
> multiboot2 with my patches supports that solution. However, multiboot (v1) 
> could
> be a bit problematic because it means that we must set load address to 16 
> MiB.
> Are we sure that this region is available on all machines like region 
> starting
> at 1 MiB?

"This region" being which one?

Jan