[v16,5/9] x86: change default load address from 1 MiB to 2 MiB
diff mbox

Message ID 1487704799-21162-6-git-send-email-daniel.kiper@oracle.com
State New, archived
Headers show

Commit Message

Daniel Kiper Feb. 21, 2017, 7:19 p.m. UTC
Subsequent patches introducing relocatable early boot code play with
page tables using 2 MiB huge pages. If load address is not aligned at
2 MiB then code touching such page tables must have special cases for
start and end of Xen image memory region. So, let's make life easier
and move default load address from 1 MiB to 2 MiB. This way page table
code will be nice and easy. Hence, there is a chance that it will be
less error prone too... :-)))

Additionally, drop first 2 MiB mapping from Xen image mapping.
It is no longer needed.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
---
v8 - suggestions/fixes:
   - drop first 2 MiB mapping from Xen image mapping
     (suggested by Jan Beulich),
   - improve commit message.

v7 - suggestions/fixes:
   - minor cleanups
     (suggested by Jan Beulich).
---
 xen/arch/x86/Makefile      |    2 +-
 xen/arch/x86/Rules.mk      |    3 +++
 xen/arch/x86/boot/head.S   |    8 --------
 xen/arch/x86/boot/x86_64.S |    5 +++--
 xen/arch/x86/setup.c       |    3 ++-
 xen/arch/x86/xen.lds.S     |    2 +-
 6 files changed, 10 insertions(+), 13 deletions(-)

Comments

Jan Beulich March 1, 2017, 9:05 a.m. UTC | #1
>>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
> Subsequent patches introducing relocatable early boot code play with
> page tables using 2 MiB huge pages. If load address is not aligned at
> 2 MiB then code touching such page tables must have special cases for
> start and end of Xen image memory region. So, let's make life easier
> and move default load address from 1 MiB to 2 MiB. This way page table
> code will be nice and easy. Hence, there is a chance that it will be
> less error prone too... :-)))
> 
> Additionally, drop first 2 MiB mapping from Xen image mapping.
> It is no longer needed.

What about the memory allocated for it? Aiui at least in the xen.efi
case there would be a 2Mb chunk left unused, but also never freed.

Jan
Daniel Kiper March 1, 2017, 10:13 a.m. UTC | #2
On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
> >>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
> > Subsequent patches introducing relocatable early boot code play with
> > page tables using 2 MiB huge pages. If load address is not aligned at
> > 2 MiB then code touching such page tables must have special cases for
> > start and end of Xen image memory region. So, let's make life easier
> > and move default load address from 1 MiB to 2 MiB. This way page table
> > code will be nice and easy. Hence, there is a chance that it will be
> > less error prone too... :-)))
> >
> > Additionally, drop first 2 MiB mapping from Xen image mapping.
> > It is no longer needed.
>
> What about the memory allocated for it? Aiui at least in the xen.efi
> case there would be a 2Mb chunk left unused, but also never freed.

Memory is not allocated for it.

Daniel
Jan Beulich March 1, 2017, 10:34 a.m. UTC | #3
>>> On 01.03.17 at 11:13, <daniel.kiper@oracle.com> wrote:
> On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
>> >>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
>> > Subsequent patches introducing relocatable early boot code play with
>> > page tables using 2 MiB huge pages. If load address is not aligned at
>> > 2 MiB then code touching such page tables must have special cases for
>> > start and end of Xen image memory region. So, let's make life easier
>> > and move default load address from 1 MiB to 2 MiB. This way page table
>> > code will be nice and easy. Hence, there is a chance that it will be
>> > less error prone too... :-)))
>> >
>> > Additionally, drop first 2 MiB mapping from Xen image mapping.
>> > It is no longer needed.
>>
>> What about the memory allocated for it? Aiui at least in the xen.efi
>> case there would be a 2Mb chunk left unused, but also never freed.
> 
> Memory is not allocated for it.

Are you sure? Where would the PE32+ header live in that case?

Jan
Daniel Kiper March 1, 2017, 10:51 a.m. UTC | #4
On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote:
> >>> On 01.03.17 at 11:13, <daniel.kiper@oracle.com> wrote:
> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
> >> >>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
> >> > Subsequent patches introducing relocatable early boot code play with
> >> > page tables using 2 MiB huge pages. If load address is not aligned at
> >> > 2 MiB then code touching such page tables must have special cases for
> >> > start and end of Xen image memory region. So, let's make life easier
> >> > and move default load address from 1 MiB to 2 MiB. This way page table
> >> > code will be nice and easy. Hence, there is a chance that it will be
> >> > less error prone too... :-)))
> >> >
> >> > Additionally, drop first 2 MiB mapping from Xen image mapping.
> >> > It is no longer needed.
> >>
> >> What about the memory allocated for it? Aiui at least in the xen.efi
> >> case there would be a 2Mb chunk left unused, but also never freed.
> >
> > Memory is not allocated for it.
>
> Are you sure? Where would the PE32+ header live in that case?

Is the PE32+ header loaded into memory? I think that only code and data stuff
is put there. Header is useful only for loader. Am I missing something?

Daniel
Jan Beulich March 1, 2017, 11:21 a.m. UTC | #5
>>> On 01.03.17 at 11:51, <daniel.kiper@oracle.com> wrote:
> On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote:
>> >>> On 01.03.17 at 11:13, <daniel.kiper@oracle.com> wrote:
>> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
>> >> >>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
>> >> > Subsequent patches introducing relocatable early boot code play with
>> >> > page tables using 2 MiB huge pages. If load address is not aligned at
>> >> > 2 MiB then code touching such page tables must have special cases for
>> >> > start and end of Xen image memory region. So, let's make life easier
>> >> > and move default load address from 1 MiB to 2 MiB. This way page table
>> >> > code will be nice and easy. Hence, there is a chance that it will be
>> >> > less error prone too... :-)))
>> >> >
>> >> > Additionally, drop first 2 MiB mapping from Xen image mapping.
>> >> > It is no longer needed.
>> >>
>> >> What about the memory allocated for it? Aiui at least in the xen.efi
>> >> case there would be a 2Mb chunk left unused, but also never freed.
>> >
>> > Memory is not allocated for it.
>>
>> Are you sure? Where would the PE32+ header live in that case?
> 
> Is the PE32+ header loaded into memory? I think that only code and data stuff
> is put there. Header is useful only for loader. Am I missing something?

I think EFI's loader simply follows Windows' one(s). And yes, the
headers can be useful - to the loader itself (albeit then the loader
assumes that no-one fiddles with them).

Jan
Daniel Kiper March 6, 2017, 2:51 p.m. UTC | #6
On Wed, Mar 01, 2017 at 04:21:35AM -0700, Jan Beulich wrote:
> >>> On 01.03.17 at 11:51, <daniel.kiper@oracle.com> wrote:
> > On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote:
> >> >>> On 01.03.17 at 11:13, <daniel.kiper@oracle.com> wrote:
> >> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
> >> >> >>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
> >> >> > Subsequent patches introducing relocatable early boot code play with
> >> >> > page tables using 2 MiB huge pages. If load address is not aligned at
> >> >> > 2 MiB then code touching such page tables must have special cases for
> >> >> > start and end of Xen image memory region. So, let's make life easier
> >> >> > and move default load address from 1 MiB to 2 MiB. This way page table
> >> >> > code will be nice and easy. Hence, there is a chance that it will be
> >> >> > less error prone too... :-)))
> >> >> >
> >> >> > Additionally, drop first 2 MiB mapping from Xen image mapping.
> >> >> > It is no longer needed.
> >> >>
> >> >> What about the memory allocated for it? Aiui at least in the xen.efi
> >> >> case there would be a 2Mb chunk left unused, but also never freed.
> >> >
> >> > Memory is not allocated for it.
> >>
> >> Are you sure? Where would the PE32+ header live in that case?
> >
> > Is the PE32+ header loaded into memory? I think that only code and data stuff
> > is put there. Header is useful only for loader. Am I missing something?
>
> I think EFI's loader simply follows Windows' one(s). And yes, the

Sadly, I do not know how Windows' one(s) work(s).

> headers can be useful - to the loader itself (albeit then the loader
> assumes that no-one fiddles with them).

I have looked at OVMF. It looks that OVMF loads PE32+ header aside, parses it, puts
Xen code and data into allocated memory (EfiLoaderCode type), does relocations and
jumps into entry point. So, more or less as I expected. It looks that it does not
load whole file at once. Hence, taking into account that memory allocated for Xen
image is marked as EfiLoaderCode it means that from Xen point of view this region
is free. Of course later everything between _start and _end is reserved. So, it
seems to me that everything between __image_base__ and _start is free and
available for Xen memory allocator. Am I missing something here?

Daniel
Jan Beulich March 6, 2017, 2:59 p.m. UTC | #7
>>> On 06.03.17 at 15:51, <daniel.kiper@oracle.com> wrote:
> On Wed, Mar 01, 2017 at 04:21:35AM -0700, Jan Beulich wrote:
>> >>> On 01.03.17 at 11:51, <daniel.kiper@oracle.com> wrote:
>> > On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote:
>> >> >>> On 01.03.17 at 11:13, <daniel.kiper@oracle.com> wrote:
>> >> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
>> >> >> >>> On 21.02.17 at 20:19, <daniel.kiper@oracle.com> wrote:
>> >> >> > Subsequent patches introducing relocatable early boot code play with
>> >> >> > page tables using 2 MiB huge pages. If load address is not aligned at
>> >> >> > 2 MiB then code touching such page tables must have special cases for
>> >> >> > start and end of Xen image memory region. So, let's make life easier
>> >> >> > and move default load address from 1 MiB to 2 MiB. This way page table
>> >> >> > code will be nice and easy. Hence, there is a chance that it will be
>> >> >> > less error prone too... :-)))
>> >> >> >
>> >> >> > Additionally, drop first 2 MiB mapping from Xen image mapping.
>> >> >> > It is no longer needed.
>> >> >>
>> >> >> What about the memory allocated for it? Aiui at least in the xen.efi
>> >> >> case there would be a 2Mb chunk left unused, but also never freed.
>> >> >
>> >> > Memory is not allocated for it.
>> >>
>> >> Are you sure? Where would the PE32+ header live in that case?
>> >
>> > Is the PE32+ header loaded into memory? I think that only code and data 
> stuff
>> > is put there. Header is useful only for loader. Am I missing something?
>>
>> I think EFI's loader simply follows Windows' one(s). And yes, the
> 
> Sadly, I do not know how Windows' one(s) work(s).
> 
>> headers can be useful - to the loader itself (albeit then the loader
>> assumes that no-one fiddles with them).
> 
> I have looked at OVMF. It looks that OVMF loads PE32+ header aside, parses it, puts
> Xen code and data into allocated memory (EfiLoaderCode type), does relocations and
> jumps into entry point. So, more or less as I expected. It looks that it does not
> load whole file at once. Hence, taking into account that memory allocated for Xen
> image is marked as EfiLoaderCode it means that from Xen point of view this region
> is free. Of course later everything between _start and _end is reserved. So, it
> seems to me that everything between __image_base__ and _start is free and
> available for Xen memory allocator. Am I missing something here?

Oh, right, the EFI memory type is what removes the need to free
anything here. I'm sorry for the noise.

Jan

Patch
diff mbox

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d6980b5..fead99a 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -91,7 +91,7 @@  all_symbols =
 endif
 
 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
-	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) 0x100000 \
+	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \
 	               `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'`
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 72be8b2..568657e 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -1,9 +1,12 @@ 
 ########################################
 # x86-specific definitions
 
+XEN_IMG_OFFSET := 0x200000
+
 CFLAGS += -I$(BASEDIR)/include
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
+CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
 CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
 
 # Prevent floating-point variables from creeping into Xen.
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index a2177c3..b5d9ec8 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -524,14 +524,6 @@  trampoline_setup:
         mov     %eax,sym_phys(boot_tsc_stamp)
         mov     %edx,sym_phys(boot_tsc_stamp+4)
 
-        /*
-         * During boot, hook 4kB mappings of first 2MB of memory into L2.
-         * This avoids mixing cachability for the legacy VGA region, and is
-         * corrected when Xen relocates itself.
-         */
-        mov     $sym_phys(l1_identmap)+__PAGE_HYPERVISOR,%edi
-        mov     %edi,sym_phys(l2_xenmap)
-
         /* Apply relocations to bootstrap trampoline. */
         mov     sym_phys(trampoline_phys),%edx
         mov     $sym_phys(__trampoline_rel_start),%edi
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 4d507fb..f9d1022 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -121,8 +121,9 @@  GLOBAL(l2_identmap)
  * page.
  */
 GLOBAL(l2_xenmap)
-        idx = 0
-        .rept 8
+        .quad 0
+        idx = 1
+        .rept 7
         .quad sym_phys(__image_base__) + (idx << L2_PAGETABLE_SHIFT) + (PAGE_HYPERVISOR | _PAGE_PSE)
         idx = idx + 1
         .endr
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 66b7aba..e2a5f76 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -998,7 +998,8 @@  void __init noreturn __start_xen(unsigned long mbi_p)
              * Undo the temporary-hooking of the l1_identmap.  __2M_text_start
              * is contained in this PTE.
              */
-            BUG_ON(l2_table_offset((unsigned long)_erodata) ==
+            BUG_ON(using_2M_mapping() &&
+                   l2_table_offset((unsigned long)_erodata) ==
                    l2_table_offset((unsigned long)_stext));
             *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
                                    PAGE_HYPERVISOR_RX | _PAGE_PSE);
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 066aca6..ff93a0b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -54,7 +54,7 @@  SECTIONS
   __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
 #endif
 
-  . = __XEN_VIRT_START + MB(1);
+  . = __XEN_VIRT_START + XEN_IMG_OFFSET;
   _start = .;
   .text : {
         _stext = .;            /* Text and read-only data */