[PATCH 0/5] Further x86 cleanup in preparation for superpages

[PATCH 0/5] Further x86 cleanup in preparation for superpages

[Andrew Cooper]

I have sadly been delayed with my superpage work, but this cleanup is ready
and need not be delayed.

Andrew Cooper (5):
  xen: Use existing __section() macro instead of opencoding it
  xen/x86: Misc boot/link tweaking
  x86/link: Reduce the number of sections created
  x86/setup: Move CPU0s stack out of the Xen text/data/bss virtual
    region
  x86/boot: Move/copy sections more efficiently

 xen/arch/x86/boot/head.S       |   35 +++++++----------------------------
 xen/arch/x86/boot/x86_64.S     |   20 ++++++++++++++++++++
 xen/arch/x86/hvm/hvm.c         |    2 +-
 xen/arch/x86/mm.c              |    3 +--
 xen/arch/x86/setup.c           |   39 +++++++++++++++++++++++++++------------
 xen/arch/x86/traps.c           |    3 +--
 xen/arch/x86/x86_64/mm.c       |    6 ++----
 xen/arch/x86/xen.lds.S         |   31 +++++++++++++++----------------
 xen/include/asm-arm/device.h   |    2 +-
 xen/include/asm-arm/platform.h |    2 +-
 10 files changed, 76 insertions(+), 67 deletions(-)

-- 
1.7.10.4

[PATCH 1/5] xen: Use existing __section() macro instead of opencoding it

[Andrew Cooper]

No functional change

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/hvm/hvm.c         |    2 +-
 xen/arch/x86/mm.c              |    3 +--
 xen/arch/x86/setup.c           |    2 +-
 xen/arch/x86/x86_64/mm.c       |    6 ++----
 xen/include/asm-arm/device.h   |    2 +-
 xen/include/asm-arm/platform.h |    2 +-
 6 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index b1023bb..f354cb7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -82,7 +82,7 @@
  * the hardware domain which needs a more permissive one.
  */
 #define HVM_IOBITMAP_SIZE (3 * PAGE_SIZE)
-unsigned long __attribute__ ((__section__ (".bss.page_aligned")))
+unsigned long __section(".bss.page_aligned")
     hvm_io_bitmap[HVM_IOBITMAP_SIZE / BYTES_PER_LONG];
 
 /* Xen command-line option to enable HAP */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1c0783f..c1a38bc 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -126,8 +126,7 @@
 #include <asm/pci.h>
 
 /* Mapping of the fixmap space needed early. */
-l1_pgentry_t __attribute__ ((__section__ (".bss.page_aligned")))
-    l1_fixmap[L1_PAGETABLE_ENTRIES];
+l1_pgentry_t __section(".bss.page_aligned") l1_fixmap[L1_PAGETABLE_ENTRIES];
 
 #define MEM_LOG(_f, _a...) gdprintk(XENLOG_WARNING , _f "\n" , ## _a)
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 8e21859..d118dec 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -101,7 +101,7 @@
 
 DEFINE_PER_CPU(struct tss_struct, init_tss);
 
-char __attribute__ ((__section__(".bss.stack_aligned"))) cpu0_stack[STACK_SIZE];
+char __section(".bss.stack_aligned") cpu0_stack[STACK_SIZE];
 
 struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
 
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index e58de58..3ef4618 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -43,10 +43,8 @@
 unsigned int __read_mostly m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
 
 /* Enough page directories to map into the bottom 1GB. */
-l3_pgentry_t __attribute__ ((__section__ (".bss.page_aligned")))
-    l3_bootmap[L3_PAGETABLE_ENTRIES];
-l2_pgentry_t __attribute__ ((__section__ (".bss.page_aligned")))
-    l2_bootmap[L2_PAGETABLE_ENTRIES];
+l3_pgentry_t __section(".bss.page_aligned") l3_bootmap[L3_PAGETABLE_ENTRIES];
+l2_pgentry_t __section(".bss.page_aligned") l2_bootmap[L2_PAGETABLE_ENTRIES];
 
 l2_pgentry_t *compat_idle_pg_table_l2;
 
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index a72f7c9..5d0a4cd 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -71,7 +71,7 @@ int __init device_init(struct dt_device_node *dev, enum device_class class,
 
 #define DT_DEVICE_START(_name, _namestr, _class)                    \
 static const struct device_desc __dev_desc_##_name __used           \
-__attribute__((__section__(".dev.info"))) = {                       \
+__section(".dev.info") = {                                          \
     .name = _namestr,                                               \
     .class = _class,                                                \
 
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index 746e126..283b50f 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -67,7 +67,7 @@ struct platform_desc {
 
 #define PLATFORM_START(_name, _namestr)                         \
 static const struct platform_desc  __plat_desc_##_name __used   \
-__attribute__((__section__(".arch.info"))) = {                  \
+__section(".arch.info") = {                                     \
     .name = _namestr,
 
 #define PLATFORM_END                                            \
-- 
1.7.10.4

Re: [PATCH 1/5] xen: Use existing __section() macro instead of opencoding it

[Jan Beulich]

>>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
> No functional change
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> CC: Tim Deegan <tim@xen.org>
> ---
>  xen/arch/x86/hvm/hvm.c         |    2 +-
>  xen/arch/x86/mm.c              |    3 +--
>  xen/arch/x86/setup.c           |    2 +-
>  xen/arch/x86/x86_64/mm.c       |    6 ++----
>  xen/include/asm-arm/device.h   |    2 +-
>  xen/include/asm-arm/platform.h |    2 +-
>  6 files changed, 7 insertions(+), 10 deletions(-)

Not having seen a ARM maintainer ack so far, I decided to commit
only the x86 parts.

Jan

Re: [PATCH 1/5] xen: Use existing __section() macro instead of opencoding it

[Ian Campbell]

On Mon, 2015-06-08 at 13:20 +0100, Jan Beulich wrote:
> >>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
> > No functional change
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > CC: Keir Fraser <keir@xen.org>
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> > CC: Tim Deegan <tim@xen.org>
> > ---
> >  xen/arch/x86/hvm/hvm.c         |    2 +-
> >  xen/arch/x86/mm.c              |    3 +--
> >  xen/arch/x86/setup.c           |    2 +-
> >  xen/arch/x86/x86_64/mm.c       |    6 ++----
> >  xen/include/asm-arm/device.h   |    2 +-
> >  xen/include/asm-arm/platform.h |    2 +-
> >  6 files changed, 7 insertions(+), 10 deletions(-)
> 
> Not having seen a ARM maintainer ack so far, I decided to commit
> only the x86 parts.

It was only posted Friday afternoon.

Anyway: Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH 1/5] xen: Use existing __section() macro instead of opencoding it

[Jan Beulich]

>>> On 08.06.15 at 14:29, <ian.campbell@citrix.com> wrote:
> On Mon, 2015-06-08 at 13:20 +0100, Jan Beulich wrote:
>> >>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
>> > No functional change
>> > 
>> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> > CC: Keir Fraser <keir@xen.org>
>> > CC: Jan Beulich <JBeulich@suse.com>
>> > CC: Ian Campbell <ian.campbell@citrix.com>
>> > CC: Stefano Stabellini <stefano.stabellini@citrix.com>
>> > CC: Tim Deegan <tim@xen.org>
>> > ---
>> >  xen/arch/x86/hvm/hvm.c         |    2 +-
>> >  xen/arch/x86/mm.c              |    3 +--
>> >  xen/arch/x86/setup.c           |    2 +-
>> >  xen/arch/x86/x86_64/mm.c       |    6 ++----
>> >  xen/include/asm-arm/device.h   |    2 +-
>> >  xen/include/asm-arm/platform.h |    2 +-
>> >  6 files changed, 7 insertions(+), 10 deletions(-)
>> 
>> Not having seen a ARM maintainer ack so far, I decided to commit
>> only the x86 parts.
> 
> It was only posted Friday afternoon.

Right, and this wasn't meant as a complaint or anything; I
simply wanted to get it off my plate, but didn't want to drop
bits silently. I suppose it would have been better to submit
these as independent x86 and ARM patches anyway...

> Anyway: Acked-by: Ian Campbell <ian.campbell@citrix.com>

And now also committed and pushed.

Jan

Re: [PATCH 1/5] xen: Use existing __section() macro instead of opencoding it

[Ian Campbell]

On Mon, 2015-06-08 at 14:42 +0100, Jan Beulich wrote:
> >>> On 08.06.15 at 14:29, <ian.campbell@citrix.com> wrote:
> > On Mon, 2015-06-08 at 13:20 +0100, Jan Beulich wrote:
> >> >>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
> >> > No functional change
> >> > 
> >> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> > CC: Keir Fraser <keir@xen.org>
> >> > CC: Jan Beulich <JBeulich@suse.com>
> >> > CC: Ian Campbell <ian.campbell@citrix.com>
> >> > CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> >> > CC: Tim Deegan <tim@xen.org>
> >> > ---
> >> >  xen/arch/x86/hvm/hvm.c         |    2 +-
> >> >  xen/arch/x86/mm.c              |    3 +--
> >> >  xen/arch/x86/setup.c           |    2 +-
> >> >  xen/arch/x86/x86_64/mm.c       |    6 ++----
> >> >  xen/include/asm-arm/device.h   |    2 +-
> >> >  xen/include/asm-arm/platform.h |    2 +-
> >> >  6 files changed, 7 insertions(+), 10 deletions(-)
> >> 
> >> Not having seen a ARM maintainer ack so far, I decided to commit
> >> only the x86 parts.
> > 
> > It was only posted Friday afternoon.
> 
> Right, and this wasn't meant as a complaint or anything; I
> simply wanted to get it off my plate, but didn't want to drop
> bits silently. I suppose it would have been better to submit
> these as independent x86 and ARM patches anyway...
> 
> > Anyway: Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> And now also committed and pushed.

Thanks.

[PATCH 2/5] xen/x86: Misc boot/link tweaking

[Andrew Cooper]

 * Introduce symbols bounding the multiboot1 header, which helps clarify that
   it is data and not code corruption when viewing the disassembly.
 * Move the __high_start symbol to its implementation, and declare it
   correctly as ENTRY()
 * Move the l1_identmap construction to be with all the other pagetables, and
   within __page_tables_{start,end}.  This won't affect the EFI relocation
   algorithm, as l1_identmap contains no relocations.
 * Move the cpu0_stack alignment check to the linker.  Chances are very good
   that a binary with a misaligned stack won't get as far as the test.
 * Use MB() in linker script.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/boot/head.S   |   26 ++------------------------
 xen/arch/x86/boot/x86_64.S |   20 ++++++++++++++++++++
 xen/arch/x86/setup.c       |    3 ---
 xen/arch/x86/xen.lds.S     |    5 +++--
 4 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 2d0e56c..cfd59dc 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -23,7 +23,7 @@ ENTRY(start)
         jmp     __start
 
         .align 4
-/*** MULTIBOOT HEADER ****/
+multiboot1_header_start:       /*** MULTIBOOT1 HEADER ****/
 #define MULTIBOOT_HEADER_FLAGS (MULTIBOOT_HEADER_MODS_ALIGNED | \
                                 MULTIBOOT_HEADER_WANT_MEMORY)
         /* Magic number indicating a Multiboot header. */
@@ -32,6 +32,7 @@ ENTRY(start)
         .long   MULTIBOOT_HEADER_FLAGS
         /* Checksum: must be the negated sum of the first two fields. */
         .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
+multiboot1_header_end:
 
         .section .init.rodata, "a", @progbits
         .align 4
@@ -212,27 +213,4 @@ ENTRY(trampoline_start)
 #include "trampoline.S"
 GLOBAL(trampoline_end)
 
-        .text
-__high_start:
 #include "x86_64.S"
-
-        .section .data.page_aligned, "aw", @progbits
-        .p2align PAGE_SHIFT
-/*
- * Mapping of first 2 megabytes of memory. This is mapped with 4kB mappings
- * to avoid type conflicts with fixed-range MTRRs covering the lowest megabyte
- * of physical memory. In any case the VGA hole should be mapped with type UC.
- */
-GLOBAL(l1_identmap)
-        pfn = 0
-        .rept L1_PAGETABLE_ENTRIES
-        /* VGA hole (0xa0000-0xc0000) should be mapped UC. */
-        .if pfn >= 0xa0 && pfn < 0xc0
-        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR_NOCACHE | MAP_SMALL_PAGES
-        .else
-        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR | MAP_SMALL_PAGES
-        .endif
-        .long 0
-        pfn = pfn + 1
-        .endr
-        .size l1_identmap, . - l1_identmap
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index bfbafd2..c8bf9d0 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -1,5 +1,7 @@
+        .text
         .code64
 
+ENTRY(__high_start)
         /* Install relocated data selectors. */
         lgdt    gdt_descr(%rip)
         mov     $(__HYPERVISOR_DS64),%ecx
@@ -80,6 +82,24 @@ GLOBAL(boot_cpu_compat_gdt_table)
         .align PAGE_SIZE, 0
 
 GLOBAL(__page_tables_start)
+/*
+ * Mapping of first 2 megabytes of memory. This is mapped with 4kB mappings
+ * to avoid type conflicts with fixed-range MTRRs covering the lowest megabyte
+ * of physical memory. In any case the VGA hole should be mapped with type UC.
+ */
+GLOBAL(l1_identmap)
+        pfn = 0
+        .rept L1_PAGETABLE_ENTRIES
+        /* VGA hole (0xa0000-0xc0000) should be mapped UC. */
+        .if pfn >= 0xa0 && pfn < 0xc0
+        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR_NOCACHE | MAP_SMALL_PAGES
+        .else
+        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR | MAP_SMALL_PAGES
+        .endif
+        .long 0
+        pfn = pfn + 1
+        .endr
+        .size l1_identmap, . - l1_identmap
 
 /* Mapping of first 16 megabytes of memory. */
 GLOBAL(l2_identmap)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index d118dec..d3c1817 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -674,9 +674,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
         panic("dom0 kernel not specified. Check bootloader configuration.");
 
-    if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
-        panic("Misaligned CPU0 stack.");
-
     if ( efi_enabled )
     {
         set_pdx_range(xen_phys_start >> PAGE_SHIFT,
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 5fc6c9f..6553cff 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -38,7 +38,7 @@ SECTIONS
   . = __XEN_VIRT_START;
   __image_base__ = .;
 #endif
-  . = __XEN_VIRT_START + 0x100000;
+  . = __XEN_VIRT_START + MB(1);
   _start = .;
   .text : {
         _stext = .;            /* Text and read-only data */
@@ -187,7 +187,7 @@ SECTIONS
   /* Trick the linker into setting the image size to exactly 16Mb. */
   . = ALIGN(__section_alignment__);
   .pad : {
-    . = ALIGN(0x1000000);
+    . = ALIGN(MB(16));
   } :text
 #else
   efi = .;
@@ -221,3 +221,4 @@ ASSERT(__image_base__ > XEN_VIRT_START ||
        _end <= XEN_VIRT_END - NR_CPUS * PAGE_SIZE,
        "Xen image overlaps stubs area")
 ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
+ASSERT((cpu0_stack & (STACK_SIZE - 1)) == 0, "cpu0_stack misaligned")
-- 
1.7.10.4

[PATCH 3/5] x86/link: Reduce the number of sections created

[Andrew Cooper]

There is no need for top level sections for each of these, so they are
subsumed into more-generic sections.

  .data.read_mostly and .lockprofile.data are moved to .data

  .init.setup, .initcall.init, .xsm_initcall.init are moved to .init.data

This will simplify the later adjustments to use superpages.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>

---
Jan: Judging from the code, it appears safe for .reloc to be moved into
.init.data, although I don't know if this will interfere with some of the PE
parts of EFI.  If a top level .reloc section is still needed, it should at
least move to immediately follow .init.data, which would allow its space to be
reused after boot.
---
 xen/arch/x86/xen.lds.S |   25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 6553cff..8295ecf 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -68,7 +68,7 @@ SECTIONS
   } :text
 
   . = ALIGN(SMP_CACHE_BYTES);
-  .data.read_mostly : {
+  .data : {
        /* Exception table */
        __start___ex_table = .;
        *(.ex_table)
@@ -82,23 +82,21 @@ SECTIONS
        *(.data.read_mostly)
        *(.data.rel.ro)
        *(.data.rel.ro.*)
-  } :text
 
-  .data : {                    /* Data */
        . = ALIGN(PAGE_SIZE);
        *(.data.page_aligned)
        *(.data)
        *(.data.rel)
        *(.data.rel.*)
        CONSTRUCTORS
-  } :text
 
 #ifdef LOCK_PROFILE
-  . = ALIGN(32);
-  __lock_profile_start = .;
-  .lockprofile.data : { *(.lockprofile.data) } :text
-  __lock_profile_end = .;
+       . = ALIGN(32);
+       __lock_profile_start = .;
+       .lockprofile.data : { *(.lockprofile.data) } :text
+       __lock_profile_end = .;
 #endif
+  } :text
 
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
@@ -140,21 +138,18 @@ SECTIONS
        __ctors_start = .;
        *(.ctors)
        __ctors_end = .;
-  } :text
-  . = ALIGN(32);
-  .init.setup : {
+
+       . = ALIGN(32);
        __setup_start = .;
        *(.init.setup)
        __setup_end = .;
-  } :text
-  .initcall.init : {
+
        __initcall_start = .;
        *(.initcallpresmp.init)
        __presmp_initcall_end = .;
        *(.initcall1.init)
        __initcall_end = .;
-  } :text
-  .xsm_initcall.init : {
+
        __xsm_initcall_start = .;
        *(.xsm_initcall.init)
        __xsm_initcall_end = .;
-- 
1.7.10.4

Re: [PATCH 3/5] x86/link: Reduce the number of sections created

[Jan Beulich]

>>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
> There is no need for top level sections for each of these, so they are
> subsumed into more-generic sections.
> 
>   .data.read_mostly and .lockprofile.data are moved to .data
> 
>   .init.setup, .initcall.init, .xsm_initcall.init are moved to .init.data
> 
> This will simplify the later adjustments to use superpages.

I'm curious, as I wouldn't expect the section count to matter there
at all. Generally, especially when having to disassemble a xen-syms
I find it quite useful if e.g. not all data is lumped together into one
big .data section.

Jan

Re: [PATCH 3/5] x86/link: Reduce the number of sections created

[Andrew Cooper]

On 05/06/15 16:38, Jan Beulich wrote:
>>>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
>> There is no need for top level sections for each of these, so they are
>> subsumed into more-generic sections.
>>
>>   .data.read_mostly and .lockprofile.data are moved to .data
>>
>>   .init.setup, .initcall.init, .xsm_initcall.init are moved to .init.data
>>
>> This will simplify the later adjustments to use superpages.
> I'm curious, as I wouldn't expect the section count to matter there
> at all. Generally, especially when having to disassemble a xen-syms
> I find it quite useful if e.g. not all data is lumped together into one
> big .data section.

With hindsight since writing, I think I can solve the conceptual problem
I was having with extra commenting in the linker script.

Originally I attempting to coalesce all sections which would have
identical superpage permissions, but this isn't exclusive with having
these sections separately.

Any thoughts about the .reloc section, at least moving it into it to
between __init_begin and __init_end so it gets reclaimed on boot?

~Andrew

Re: [PATCH 3/5] x86/link: Reduce the number of sections created

[Jan Beulich]

>>> On 05.06.15 at 18:11, <andrew.cooper3@citrix.com> wrote:
> On 05/06/15 16:38, Jan Beulich wrote:
>>>>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
>>> There is no need for top level sections for each of these, so they are
>>> subsumed into more-generic sections.
>>>
>>>   .data.read_mostly and .lockprofile.data are moved to .data
>>>
>>>   .init.setup, .initcall.init, .xsm_initcall.init are moved to .init.data
>>>
>>> This will simplify the later adjustments to use superpages.
>> I'm curious, as I wouldn't expect the section count to matter there
>> at all. Generally, especially when having to disassemble a xen-syms
>> I find it quite useful if e.g. not all data is lumped together into one
>> big .data section.
> 
> With hindsight since writing, I think I can solve the conceptual problem
> I was having with extra commenting in the linker script.
> 
> Originally I attempting to coalesce all sections which would have
> identical superpage permissions, but this isn't exclusive with having
> these sections separately.

Good. I'll put this patch at the side then for the moment.

> Any thoughts about the .reloc section, at least moving it into it to
> between __init_begin and __init_end so it gets reclaimed on boot?

.reloc sits after _end (as it's of interest to only the EFI boot
loader code), and hence should already be getting reclaimed.

Jan

[PATCH 4/5] x86/setup: Move CPU0s stack out of the Xen text/data/bss virtual region

[Andrew Cooper]

Currently, the BSP's stack is the BSS symbol cpu0_stack.  In builds using
memguard_stack(), a page gets shot out of the mappings.

To avoid shattering the superpage which will eventually map the BSS, use the
directmap virtual address of cpu0_stack, while still using the same underlying
physical memory.  (Xen has an order 21 physical relocation requirement meaning
that the order 3 alignment requirement for cpu0_stack will be honoured even
via its diretmap mapping.)

In addition, fix two issues exposed by the changes.

 * do_invalid_op() should use is_active_kernel_text() rather than having its
   own, different, idea of when to search through the bugframes.
 * Setting of system_state to active needs to be deferred until after code has
   left .init.text, for bugframes/backtraces to function in reinit_bsp_stack().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/setup.c |   34 ++++++++++++++++++++++++++--------
 xen/arch/x86/traps.c |    3 +--
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index d3c1817..c32e49f 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -507,6 +507,10 @@ static void __init kexec_reserve_area(struct e820map *e820)
 
 static void noinline init_done(void)
 {
+    system_state = SYS_STATE_active;
+
+    domain_unpause_by_systemcontroller(hardware_domain);
+
     /* Free (or page-protect) the init areas. */
     memset(__init_begin, 0xcc, __init_end - __init_begin); /* int3 poison */
     free_xen_data(__init_begin, __init_end);
@@ -515,6 +519,23 @@ static void noinline init_done(void)
     startup_cpu_idle_loop();
 }
 
+/* Reinitalise all state referring to the old virtual address of the stack. */
+static void __init noreturn reinit_bsp_stack(void)
+{
+    unsigned long *stack = (void*)(get_stack_bottom() & ~(STACK_SIZE - 1));
+
+    /* Update TSS and ISTs */
+    load_system_tables();
+
+    /* Update SYSCALL trampolines */
+    percpu_traps_init();
+
+    stack_base[0] = stack;
+    memguard_guard_stack(stack);
+
+    reset_stack_and_jump(init_done);
+}
+
 static bool_t __init loader_is_grub2(const char *loader_name)
 {
     /* GRUB1="GNU GRUB 0.xx"; GRUB2="GRUB 1.xx" */
@@ -1210,9 +1231,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     tboot_probe();
 
-    /* Unmap the first page of CPU0's stack. */
-    memguard_guard_stack(cpu0_stack);
-
     open_softirq(NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ, new_tlbflush_clock_period);
 
     if ( opt_watchdog ) 
@@ -1454,11 +1472,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     setup_io_bitmap(dom0);
 
-    system_state = SYS_STATE_active;
-
-    domain_unpause_by_systemcontroller(dom0);
-
-    reset_stack_and_jump(init_done);
+    /* Jump to the 1:1 virtual mappings of cpu0_stack. */
+    asm volatile ("mov %[stk], %%rsp; jmp %c[fn]" ::
+                  [stk] "g" (__va(__pa(get_stack_bottom()))),
+                  [fn] "i" (reinit_bsp_stack) : "memory");
+    unreachable();
 }
 
 void arch_get_xen_caps(xen_capabilities_info_t *info)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 91701a2..080c0c1 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1072,8 +1072,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
         return;
     }
 
-    if ( (!is_kernel_text(eip) &&
-          (system_state > SYS_STATE_boot || !is_kernel_inittext(eip))) ||
+    if ( !is_active_kernel_text(regs->eip) ||
          __copy_from_user(bug_insn, eip, sizeof(bug_insn)) ||
          memcmp(bug_insn, "\xf\xb", sizeof(bug_insn)) )
         goto die;
-- 
1.7.10.4

[PATCH 5/5] x86/boot: Move/copy sections more efficiently

[Andrew Cooper]

Both the trampoline copy and BSS initialise can be performed more efficiently
by using 4-byte varients of the string operations.

The ALIGN(STACK_SIZE) actually belongs with .bss.stack_aligned, but __init_end
still needs page alignment because of the init sections being freed and
returned to the domheap after boot.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/boot/head.S |    9 +++++----
 xen/arch/x86/xen.lds.S   |    5 ++++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index cfd59dc..604c515 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -128,7 +128,8 @@ __start:
         mov     $sym_phys(__bss_end),%ecx
         sub     %edi,%ecx
         xor     %eax,%eax
-        rep     stosb
+        shr     $2,%ecx
+        rep     stosl
 
         /* Interrogate CPU extended features via CPUID. */
         mov     $0x80000000,%eax
@@ -198,8 +199,8 @@ __start:
 
         /* Copy bootstrap trampoline to low memory, below 1MB. */
         mov     $sym_phys(trampoline_start),%esi
-        mov     $trampoline_end - trampoline_start,%ecx
-        rep     movsb
+        mov     $((trampoline_end - trampoline_start) / 4),%ecx
+        rep     movsl
 
         /* Jump into the relocated trampoline. */
         lret
@@ -211,6 +212,6 @@ reloc:
 
 ENTRY(trampoline_start)
 #include "trampoline.S"
-GLOBAL(trampoline_end)
+ENTRY(trampoline_end)
 
 #include "x86_64.S"
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 8295ecf..b49c031 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -153,11 +153,13 @@ SECTIONS
        __xsm_initcall_start = .;
        *(.xsm_initcall.init)
        __xsm_initcall_end = .;
+
+       . = ALIGN(PAGE_SIZE);
   } :text
-  . = ALIGN(STACK_SIZE);
   __init_end = .;
 
   .bss : {                     /* BSS */
+       . = ALIGN(STACK_SIZE);
        __bss_start = .;
        *(.bss.stack_aligned)
        . = ALIGN(PAGE_SIZE);
@@ -170,6 +172,7 @@ SECTIONS
        *(.bss.percpu.read_mostly)
        . = ALIGN(SMP_CACHE_BYTES);
        __per_cpu_data_end = .;
+       . = ALIGN(8);
        __bss_end = .;
   } :text
   _end = . ;
-- 
1.7.10.4

Re: [PATCH 5/5] x86/boot: Move/copy sections more efficiently

[Jan Beulich]

>>> On 05.06.15 at 15:31, <andrew.cooper3@citrix.com> wrote:
> Both the trampoline copy and BSS initialise can be performed more efficiently
> by using 4-byte varients of the string operations.

On second thought I'm actually not convinced of this: With
X86_FEATURE_ERMS using byte MOVS/STOS is supposedly
preferred.

> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -153,11 +153,13 @@ SECTIONS
>         __xsm_initcall_start = .;
>         *(.xsm_initcall.init)
>         __xsm_initcall_end = .;
> +
> +       . = ALIGN(PAGE_SIZE);
>    } :text
> -  . = ALIGN(STACK_SIZE);
>    __init_end = .;

Perhaps worth also moving the symbol into the section?

Jan