All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Map Xen code/data/bss with superpages
@ 2016-02-24 19:07 Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/lockprof: Move .lockprofile.data into .rodata Andrew Cooper
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper

And make use of NX and RO attributes wherever possible.

After this series, the pagetable layout looks like:

(XEN) *** Dumping Xen text/data/bss mappings from ffff82d080000000
(XEN) l2_xenmap: ffff82d080814000, pa 00000000ac014000
(XEN)  L4[261] = 00000000ac017063 X S RW P
(XEN)   L3[322] = 00000000ac014063 X S RW P
(XEN)    L2[000] = 00000000ab8001e1 X Gl + S RO P   <- .text
(XEN)    L2[001] = 00000000aba001a1 X Gl + S RO P   <- .text
(XEN)    L2[002] = 80000000abc001a1 NX Gl + S RO P  <- .rodata
                                                    <- discarded .init
(XEN)    L2[004] = 80000000ac0001e3 NX Gl + S RW P  <- .data and .bss
(XEN)    L2[511] = 000000084da6f063 X S RW P        <- stubs

Andrew Cooper (8):
  xen/lockprof: Move .lockprofile.data into .rodata
  xen/x86: Improvements to build-time pagetable generation
  xen/x86: Construct the {l2,l3}_bootmap at compile time
  xen/memguard: Drop memguard_init() entirely
  xen/x86: Disable CR0.WP while applying alternatives
  xen/x86: Reorder .data and .init when linking
  xen/x86: Use 2M superpages for text/data/bss mappings
  xen/x86: Unilaterally remove .init mappings

 xen/arch/arm/xen.lds.S       |  15 ++++---
 xen/arch/x86/alternative.c   |   7 +++
 xen/arch/x86/boot/head.S     |  18 +++-----
 xen/arch/x86/boot/x86_64.S   |  44 +++++++++++++++---
 xen/arch/x86/mm.c            |  16 -------
 xen/arch/x86/setup.c         |  64 +++++++++++++++++++--------
 xen/arch/x86/x86_64/mm.c     |   4 --
 xen/arch/x86/xen.lds.S       | 103 ++++++++++++++++++++++++++++---------------
 xen/include/asm-arm/config.h |   1 +
 xen/include/asm-arm/mm.h     |   1 -
 xen/include/asm-x86/config.h |   1 +
 xen/include/asm-x86/mm.h     |   2 -
 xen/include/asm-x86/setup.h  |   5 +++
 xen/include/xen/spinlock.h   |   2 +-
 14 files changed, 178 insertions(+), 105 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] xen/lockprof: Move .lockprofile.data into .rodata
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-25 11:02   ` Stefano Stabellini
  2016-02-24 19:07 ` [PATCH] xen/x86: Improvements to build-time pagetable generation Andrew Cooper
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Stefano Stabellini, Ian Campbell, Jan Beulich

The entire contents of .lockprofile.data are unchanging pointers to
lock_profile structure in .data.  Annotate the type as such, and link the
section in .rodata.  As these are just pointers, 32byte alignment is
unnecessary.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>

v2:
 * New
v3:
 * Explicitly introduce POINTER_ALIGN to avoid forcing 8 byte alignment on
   arm32
---
 xen/arch/arm/xen.lds.S       | 15 ++++++++-------
 xen/arch/x86/xen.lds.S       | 14 +++++++-------
 xen/include/asm-arm/config.h |  1 +
 xen/include/asm-x86/config.h |  1 +
 xen/include/xen/spinlock.h   |  2 +-
 5 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index f501a2f..1b5e516 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -50,6 +50,14 @@ SECTIONS
        __stop_bug_frames_2 = .;
        *(.rodata)
        *(.rodata.*)
+
+#ifdef LOCK_PROFILE
+       . = ALIGN(POINTER_ALIGN);
+       __lock_profile_start = .;
+       *(.lockprofile.data)
+       __lock_profile_end = .;
+#endif
+
         _erodata = .;          /* End of read-only data */
   } :text
 
@@ -83,13 +91,6 @@ SECTIONS
        *(.data.rel.ro.*)
   } :text
 
-#ifdef LOCK_PROFILE
-  . = ALIGN(32);
-  __lock_profile_start = .;
-  .lockprofile.data : { *(.lockprofile.data) } :text
-  __lock_profile_end = .;
-#endif
-
   . = ALIGN(8);
   .arch.info : {
       _splatform = .;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 9fde1db..8390ec2 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -65,6 +65,13 @@ SECTIONS
 
        *(.rodata)
        *(.rodata.*)
+
+#ifdef LOCK_PROFILE
+       . = ALIGN(POINTER_ALIGN);
+       __lock_profile_start = .;
+       *(.lockprofile.data)
+       __lock_profile_end = .;
+#endif
   } :text
 
   . = ALIGN(SMP_CACHE_BYTES);
@@ -97,13 +104,6 @@ SECTIONS
        CONSTRUCTORS
   } :text
 
-#ifdef LOCK_PROFILE
-  . = ALIGN(32);
-  __lock_profile_start = .;
-  .lockprofile.data : { *(.lockprofile.data) } :text
-  __lock_profile_end = .;
-#endif
-
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
   .init.text : {
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index c3a2c30..c0ad469 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -15,6 +15,7 @@
 
 #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
 #define BITS_PER_LONG (BYTES_PER_LONG << 3)
+#define POINTER_ALIGN BYTES_PER_LONG
 
 /* xen_ulong_t is always 64 bits */
 #define BITS_PER_XEN_ULONG 64
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 07f3c1f..7291b59 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -13,6 +13,7 @@
 #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
 #define BITS_PER_LONG (BYTES_PER_LONG << 3)
 #define BITS_PER_BYTE 8
+#define POINTER_ALIGN BYTES_PER_LONG
 
 #define BITS_PER_XEN_ULONG BITS_PER_LONG
 
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 77ab7d5..88b53f9 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -79,7 +79,7 @@ struct lock_profile_qhead {
 
 #define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
 #define _LOCK_PROFILE_PTR(name)                                               \
-    static struct lock_profile *__lock_profile_##name                         \
+    static struct lock_profile * const __lock_profile_##name                  \
     __used_section(".lockprofile.data") =                                     \
     &__lock_profile_data_##name
 #define _SPIN_LOCK_UNLOCKED(x) { { 0 }, SPINLOCK_NO_CPU, 0, _LOCK_DEBUG, x }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Improvements to build-time pagetable generation
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/lockprof: Move .lockprofile.data into .rodata Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Construct the {l2, l3}_bootmap at compile time Andrew Cooper
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

 * Additional comments, including size and runtime use.
 * Consistent use of .quad, rather than a mix including .long.

No change in runtime behaviour.

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

v2:
 * New
v3:
 * Drop the use of _PAGE_GLOBAL for intermediate levels.  While it is safe
   (i.e. ignored) on Intel, it is reserved for non-leaf entries on AMD.
---
 xen/arch/x86/boot/x86_64.S | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 94cf089..9df3902 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -86,32 +86,40 @@ 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.
+ * Uses 1x 4k page.
  */
 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
+        .quad (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR_NOCACHE | MAP_SMALL_PAGES
         .else
-        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR | MAP_SMALL_PAGES
+        .quad (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. */
+/*
+ * Space for mapping the first 4GB of memory, with the first 16 megabytes
+ * actualy mapped (mostly using superpages).  Uses 4x 4k pages.
+ */
 GLOBAL(l2_identmap)
         .quad sym_phys(l1_identmap) + __PAGE_HYPERVISOR
-        pfn = 0
+        idx = 1
         .rept 7
-        pfn = pfn + (1 << PAGETABLE_ORDER)
-        .quad (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR | _PAGE_PSE
+        .quad (idx << L2_PAGETABLE_SHIFT) | PAGE_HYPERVISOR | _PAGE_PSE
+        idx = idx + 1
         .endr
         .fill 4 * L2_PAGETABLE_ENTRIES - 8, 8, 0
         .size l2_identmap, . - l2_identmap
 
+/*
+ * L2 mapping the 1GB Xen text/data/bss region.  At boot it maps 16MB from
+ * __image_base__, and is modified when Xen relocates itself.  Uses 1x 4k
+ * page.
+ */
 GLOBAL(l2_xenmap)
         idx = 0
         .rept 8
@@ -121,6 +129,7 @@ GLOBAL(l2_xenmap)
         .fill L2_PAGETABLE_ENTRIES - 8, 8, 0
         .size l2_xenmap, . - l2_xenmap
 
+/* L2 mapping the fixmap.  Uses 1x 4k page. */
 l2_fixmap:
         idx = 0
         .rept L2_PAGETABLE_ENTRIES
@@ -133,6 +142,7 @@ l2_fixmap:
         .endr
         .size l2_fixmap, . - l2_fixmap
 
+/* Identity map, covering the 4 l2_identmap tables.  Uses 1x 4k page. */
 GLOBAL(l3_identmap)
         idx = 0
         .rept 4
@@ -142,6 +152,7 @@ GLOBAL(l3_identmap)
         .fill L3_PAGETABLE_ENTRIES - 4, 8, 0
         .size l3_identmap, . - l3_identmap
 
+/* L3 mapping the fixmap.  Uses 1x 4k page. */
 l3_xenmap:
         idx = 0
         .rept L3_PAGETABLE_ENTRIES
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Construct the {l2, l3}_bootmap at compile time
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/lockprof: Move .lockprofile.data into .rodata Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Improvements to build-time pagetable generation Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/memguard: Drop memguard_init() entirely Andrew Cooper
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

... rather than at runtime.

The bootmaps are discarded in zap_low_mappings(), so the tables themselves can
live in .init.data and be reclaimed after boot.

Hooking the l1_identmap into l2_xenmap stays for safety, along with a longer
comment explaining why.

This does not change the EFI construction of {l2,l3}_bootmap.  EFI already
constructs them cleanly in their relocated form.

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

v2:
 * New
v3:
 * Expand commit message to indicate that EFI is unaffected.
---
 xen/arch/x86/boot/head.S   | 18 +++++-------------
 xen/arch/x86/boot/x86_64.S | 19 +++++++++++++++++++
 xen/arch/x86/x86_64/mm.c   |  4 ----
 3 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ac4962b..f3501fd 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -150,21 +150,13 @@ __start:
         mov     %eax,sym_phys(boot_tsc_stamp)
         mov     %edx,sym_phys(boot_tsc_stamp+4)
 
-        /* Initialise L2 boot-map page table entries (16MB). */
-        mov     $sym_phys(l2_bootmap),%edx
-        mov     $PAGE_HYPERVISOR|_PAGE_PSE,%eax
-        mov     $8,%ecx
-1:      mov     %eax,(%edx)
-        add     $8,%edx
-        add     $(1<<L2_PAGETABLE_SHIFT),%eax
-        loop    1b
-        /* Initialise L3 boot-map page directory entry. */
-        mov     $sym_phys(l2_bootmap)+__PAGE_HYPERVISOR,%eax
-        mov     %eax,sym_phys(l3_bootmap) + 0*8
-        /* Hook 4kB mappings of first 2MB of memory into L2. */
+        /*
+         * 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)
-        mov     %edi,sym_phys(l2_bootmap)
 
         /* Apply relocations to bootstrap trampoline. */
         mov     sym_phys(trampoline_phys),%edx
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 9df3902..9ab9231 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -184,3 +184,22 @@ GLOBAL(idle_pg_table)
         .size idle_pg_table, . - idle_pg_table
 
 GLOBAL(__page_tables_end)
+
+/* Init pagetables.  Enough page directories to map into the bottom 1GB. */
+        .section .init.data, "a", @progbits
+        .align PAGE_SIZE, 0
+
+GLOBAL(l2_bootmap)
+        .quad sym_phys(l1_identmap) + __PAGE_HYPERVISOR
+        idx = 1
+        .rept 7
+        .quad (idx << L2_PAGETABLE_SHIFT) | __PAGE_HYPERVISOR | _PAGE_PSE
+        idx = idx + 1
+        .endr
+        .fill L2_PAGETABLE_ENTRIES - 8, 8, 0
+        .size l2_bootmap, . - l2_bootmap
+
+GLOBAL(l3_bootmap)
+        .quad sym_phys(l2_bootmap) + __PAGE_HYPERVISOR
+        .fill L3_PAGETABLE_ENTRIES - 1, 8, 0
+        .size l3_bootmap, . - l3_bootmap
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 2228898..e07e69e 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -42,10 +42,6 @@ asm(".file \"" __FILE__ "\"");
 
 unsigned int __read_mostly m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
 
-/* Enough page directories to map into the bottom 1GB. */
-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;
 
 void *do_page_walk(struct vcpu *v, unsigned long addr)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/memguard: Drop memguard_init() entirely
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
                   ` (2 preceding siblings ...)
  2016-02-24 19:07 ` [PATCH] xen/x86: Construct the {l2, l3}_bootmap at compile time Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Disable CR0.WP while applying alternatives Andrew Cooper
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper

The use of MAP_SMALL_PAGES causes shattering of the superpages making up the
Xen virtual region, and is counter to the purpose of this series.
Furthermore, it is not required for the memguard infrastructure to function
(which itself uses map_pages_to_xen() for creating holes).

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@citrix.com>
---

v2: Reword the commmit message
---
 xen/arch/x86/mm.c        | 16 ----------------
 xen/arch/x86/setup.c     |  2 --
 xen/include/asm-arm/mm.h |  1 -
 xen/include/asm-x86/mm.h |  2 --
 4 files changed, 21 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d6aaed8..ed8ab02 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -6346,22 +6346,6 @@ void free_perdomain_mappings(struct domain *d)
 
 #ifdef MEMORY_GUARD
 
-void memguard_init(void)
-{
-    unsigned long start = max_t(unsigned long, xen_phys_start, 1UL << 20);
-    map_pages_to_xen(
-        (unsigned long)__va(start),
-        start >> PAGE_SHIFT,
-        (__pa(&_end) + PAGE_SIZE - 1 - start) >> PAGE_SHIFT,
-        __PAGE_HYPERVISOR_RW|MAP_SMALL_PAGES);
-    BUG_ON(start != xen_phys_start);
-    map_pages_to_xen(
-        XEN_VIRT_START,
-        start >> PAGE_SHIFT,
-        (__pa(&_end) + PAGE_SIZE - 1 - start) >> PAGE_SHIFT,
-        __PAGE_HYPERVISOR|MAP_SMALL_PAGES);
-}
-
 static void __memguard_change_range(void *p, unsigned long l, int guard)
 {
     unsigned long _p = (unsigned long)p;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b8a28d7..cddf954 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1146,8 +1146,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
                    ~((1UL << L2_PAGETABLE_SHIFT) - 1);
     destroy_xen_mappings(xen_virt_end, XEN_VIRT_START + BOOTSTRAP_MAP_BASE);
 
-    memguard_init();
-
     nr_pages = 0;
     for ( i = 0; i < e820.nr_map; i++ )
         if ( e820.map[i].type == E820_RAM )
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 2e9d0b2..68cf203 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -331,7 +331,6 @@ unsigned long domain_get_maximum_gpfn(struct domain *d);
 
 extern struct domain *dom_xen, *dom_io, *dom_cow;
 
-#define memguard_init(_s)              (_s)
 #define memguard_guard_stack(_p)       ((void)0)
 #define memguard_guard_range(_p,_l)    ((void)0)
 #define memguard_unguard_range(_p,_l)  ((void)0)
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index a097382..23a4092 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -479,11 +479,9 @@ extern struct rangeset *mmio_ro_ranges;
 #define compat_cr3_to_pfn(cr3) (((unsigned)(cr3) >> 12) | ((unsigned)(cr3) << 20))
 
 #ifdef MEMORY_GUARD
-void memguard_init(void);
 void memguard_guard_range(void *p, unsigned long l);
 void memguard_unguard_range(void *p, unsigned long l);
 #else
-#define memguard_init()                ((void)0)
 #define memguard_guard_range(_p,_l)    ((void)0)
 #define memguard_unguard_range(_p,_l)  ((void)0)
 #endif
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Disable CR0.WP while applying alternatives
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
                   ` (3 preceding siblings ...)
  2016-02-24 19:07 ` [PATCH] xen/memguard: Drop memguard_init() entirely Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Reorder .data and .init when linking Andrew Cooper
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

In preparation for marking .text as read-only, care needs to be taken not to
fault while applying alternatives.

Swapping back to RW mappings is a possibility, but would require additional
TLB management.  A temporary disabling of CR0.WP is cleaner.

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

New in v2.  (The one downside of my very-quick-to-reboot test box is that it
is sufficiently old to not have any alternatives needing patching.)
---
 xen/arch/x86/alternative.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 46ac0fd..d123fa7 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -147,11 +147,15 @@ static void __init apply_alternatives(struct alt_instr *start, struct alt_instr
     struct alt_instr *a;
     u8 *instr, *replacement;
     u8 insnbuf[MAX_PATCH_LEN];
+    unsigned long cr0 = read_cr0();
 
     ASSERT(!local_irq_is_enabled());
 
     printk(KERN_INFO "alt table %p -> %p\n", start, end);
 
+    /* Disable WP to allow application of alternatives to read-only pages. */
+    write_cr0(cr0 & ~X86_CR0_WP);
+
     /*
      * The scan order should be from start to end. A later scanned
      * alternative code can overwrite a previous scanned alternative code.
@@ -181,6 +185,9 @@ static void __init apply_alternatives(struct alt_instr *start, struct alt_instr
                  a->instrlen - a->replacementlen);
         text_poke_early(instr, insnbuf, a->instrlen);
     }
+
+    /* Reinstate WP. */
+    write_cr0(cr0);
 }
 
 void __init alternative_instructions(void)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Reorder .data and .init when linking
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
                   ` (4 preceding siblings ...)
  2016-02-24 19:07 ` [PATCH] xen/x86: Disable CR0.WP while applying alternatives Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Unilaterally remove .init mappings Andrew Cooper
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

In preparation for using superpage mappings, .data and .bss will both want to
be mapped as read-write.  By making them adjacent, they can share the same
superpage and will not require superpage alignment between themselves.

While making this change, fix a latent alignment bug whereby the alignment for
.bss.stack_aligned was in .init.  __init_end only needs page alignment (due to
being reclaimed after boot), while .bss.stack_aligned really does needs
STACK_SIZE alignment.

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

v2:
 * New
v3:
 * Swap __bss_start and its alignment.
---
 xen/arch/x86/xen.lds.S | 63 +++++++++++++++++++++++++-------------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 8390ec2..f78309d 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -74,36 +74,6 @@ SECTIONS
 #endif
   } :text
 
-  . = ALIGN(SMP_CACHE_BYTES);
-  .data.read_mostly : {
-       /* Exception table */
-       __start___ex_table = .;
-       *(.ex_table)
-       __stop___ex_table = .;
-
-       /* Pre-exception table */
-       __start___pre_ex_table = .;
-       *(.ex_table.pre)
-       __stop___pre_ex_table = .;
-
-       *(.data.read_mostly)
-       . = ALIGN(8);
-       __start_schedulers_array = .;
-       *(.data.schedulers)
-       __end_schedulers_array = .;
-       *(.data.rel.ro)
-       *(.data.rel.ro.*)
-  } :text
-
-  .data : {                    /* Data */
-       . = ALIGN(PAGE_SIZE);
-       *(.data.page_aligned)
-       *(.data)
-       *(.data.rel)
-       *(.data.rel.*)
-       CONSTRUCTORS
-  } :text
-
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
   .init.text : {
@@ -163,10 +133,41 @@ SECTIONS
        *(.xsm_initcall.init)
        __xsm_initcall_end = .;
   } :text
-  . = ALIGN(STACK_SIZE);
+  . = ALIGN(PAGE_SIZE);
   __init_end = .;
 
+  . = ALIGN(SMP_CACHE_BYTES);
+  .data.read_mostly : {
+       /* Exception table */
+       __start___ex_table = .;
+       *(.ex_table)
+       __stop___ex_table = .;
+
+       /* Pre-exception table */
+       __start___pre_ex_table = .;
+       *(.ex_table.pre)
+       __stop___pre_ex_table = .;
+
+       *(.data.read_mostly)
+       . = ALIGN(8);
+       __start_schedulers_array = .;
+       *(.data.schedulers)
+       __end_schedulers_array = .;
+       *(.data.rel.ro)
+       *(.data.rel.ro.*)
+  } :text
+
+  .data : {                    /* Data */
+       . = ALIGN(PAGE_SIZE);
+       *(.data.page_aligned)
+       *(.data)
+       *(.data.rel)
+       *(.data.rel.*)
+       CONSTRUCTORS
+  } :text
+
   .bss : {                     /* BSS */
+       . = ALIGN(STACK_SIZE);
        __bss_start = .;
        *(.bss.stack_aligned)
        . = ALIGN(PAGE_SIZE);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
                   ` (5 preceding siblings ...)
  2016-02-24 19:07 ` [PATCH] xen/x86: Reorder .data and .init when linking Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  2016-02-24 19:07 ` [PATCH] xen/x86: Unilaterally remove .init mappings Andrew Cooper
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

This balloons the size of Xen in memory from 4.4MB to 8MB, because of the
required alignment adjustments.

However
 * All mappings are 2M superpages.
 * .text (and .init at boot) are the only sections marked executable.
 * .text and .rodata are marked read-only.

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

v2:
 * .data and .bss are adjcent (from earlier patch), so don't require 2M
   alignment
v3:
 * Move externs into an x86 location.
---
 xen/arch/x86/setup.c        | 38 +++++++++++++++++++++++++++++++++++---
 xen/arch/x86/xen.lds.S      | 27 +++++++++++++++++++++++++++
 xen/include/asm-x86/setup.h |  5 +++++
 3 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index cddf954..806fa95 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -920,14 +920,46 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
             /* The only data mappings to be relocated are in the Xen area. */
             pl2e = __va(__pa(l2_xenmap));
+            /*
+             * Undo the temporary-hooking of the l1_identmap.  __2M_text_start
+             * is contained in this PTE.
+             */
             *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
-                                   PAGE_HYPERVISOR_RWX | _PAGE_PSE);
+                                   PAGE_HYPERVISOR_RX | _PAGE_PSE);
             for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
             {
+                unsigned int flags;
+
                 if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
                     continue;
-                *pl2e = l2e_from_intpte(l2e_get_intpte(*pl2e) +
-                                        xen_phys_start);
+
+                if ( i < l2_table_offset((unsigned long)&__2M_text_end) )
+                {
+                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
+                }
+                else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) &&
+                          i <  l2_table_offset((unsigned long)&__2M_rodata_end) )
+                {
+                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
+                }
+                else if ( i >= l2_table_offset((unsigned long)&__2M_init_start) &&
+                          i <  l2_table_offset((unsigned long)&__2M_init_end) )
+                {
+                    flags = PAGE_HYPERVISOR_RWX | _PAGE_PSE;
+                }
+                else if ( (i >= l2_table_offset((unsigned long)&__2M_rwdata_start) &&
+                           i <  l2_table_offset((unsigned long)&__2M_rwdata_end)) )
+                {
+                    flags = PAGE_HYPERVISOR_RW | _PAGE_PSE;
+                }
+                else
+                {
+                    *pl2e = l2e_empty();
+                    continue;
+                }
+
+                *pl2e = l2e_from_paddr(
+                    l2e_get_paddr(*pl2e) + xen_phys_start, flags);
             }
 
             /* Re-sync the stack and then switch to relocated pagetables. */
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index f78309d..77d0ed0 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -38,6 +38,9 @@ SECTIONS
   . = __XEN_VIRT_START;
   __image_base__ = .;
 #endif
+
+  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
+
   . = __XEN_VIRT_START + MB(1);
   _start = .;
   .text : {
@@ -50,6 +53,10 @@ SECTIONS
        _etext = .;             /* End of text section */
   } :text = 0x9090
 
+  . = ALIGN(MB(2));
+  __2M_text_end = .;
+
+  __2M_rodata_start = .;       /* Start of 2M superpages, mapped RO. */
   .rodata : {
        /* Bug frames table */
        . = ALIGN(4);
@@ -74,6 +81,10 @@ SECTIONS
 #endif
   } :text
 
+  . = ALIGN(MB(2));
+  __2M_rodata_end = .;
+
+  __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
   .init.text : {
@@ -136,6 +147,10 @@ SECTIONS
   . = ALIGN(PAGE_SIZE);
   __init_end = .;
 
+  . = ALIGN(MB(2));
+  __2M_init_end = .;
+
+  __2M_rwdata_start = .;       /* Start of 2M superpages, mapped RW. */
   . = ALIGN(SMP_CACHE_BYTES);
   .data.read_mostly : {
        /* Exception table */
@@ -184,6 +199,9 @@ SECTIONS
   } :text
   _end = . ;
 
+  . = ALIGN(MB(2));
+  __2M_rwdata_end = .;
+
 #ifdef EFI
   . = ALIGN(4);
   .reloc : {
@@ -230,4 +248,13 @@ ASSERT(__image_base__ > XEN_VIRT_START ||
 ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
 #endif
 
+ASSERT(IS_ALIGNED(__2M_text_start,   MB(2)), "__2M_text_start misaligned")
+ASSERT(IS_ALIGNED(__2M_text_end,     MB(2)), "__2M_text_end misaligned")
+ASSERT(IS_ALIGNED(__2M_rodata_start, MB(2)), "__2M_rodata_start misaligned")
+ASSERT(IS_ALIGNED(__2M_rodata_end,   MB(2)), "__2M_rodata_end misaligned")
+ASSERT(IS_ALIGNED(__2M_init_start,   MB(2)), "__2M_init_start misaligned")
+ASSERT(IS_ALIGNED(__2M_init_end,     MB(2)), "__2M_init_end misaligned")
+ASSERT(IS_ALIGNED(__2M_rwdata_start, MB(2)), "__2M_rwdata_start misaligned")
+ASSERT(IS_ALIGNED(__2M_rwdata_end,   MB(2)), "__2M_rwdata_end misaligned")
+
 ASSERT(IS_ALIGNED(cpu0_stack, STACK_SIZE), "cpu0_stack misaligned")
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 381d9f8..c65b79c 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -4,6 +4,11 @@
 #include <xen/multiboot.h>
 #include <asm/numa.h>
 
+extern const char __2M_text_start[], __2M_text_end[];
+extern const char __2M_rodata_start[], __2M_rodata_end[];
+extern char __2M_init_start[], __2M_init_end[];
+extern char __2M_rwdata_start[], __2M_rwdata_end[];
+
 extern unsigned long xenheap_initial_phys_start;
 
 void early_cpu_init(void);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Unilaterally remove .init mappings
  2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
                   ` (6 preceding siblings ...)
  2016-02-24 19:07 ` [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings Andrew Cooper
@ 2016-02-24 19:07 ` Andrew Cooper
  7 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-02-24 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper

Because of the new 2M alignment of .init and .bss, the existing memory
guarding infrastructure causes a shattered 2M superpage with non-present
entries for .init, and present entries for the alignment space.

Do away with the difference in behaviour between debug and non-debug builds;
always destroy the .init mappings, and reuse the space for xenheap.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/setup.c   | 24 +++++++++++-------------
 xen/arch/x86/xen.lds.S |  3 +++
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 806fa95..8431f06 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -176,16 +176,6 @@ void __init discard_initial_images(void)
     initial_images = NULL;
 }
 
-static void free_xen_data(char *s, char *e)
-{
-#ifndef MEMORY_GUARD
-    init_xenheap_pages(__pa(s), __pa(e));
-#endif
-    memguard_guard_range(s, e-s);
-    /* Also zap the mapping in the 1:1 area. */
-    memguard_guard_range(__va(__pa(s)), e-s);
-}
-
 extern char __init_begin[], __init_end[], __bss_start[], __bss_end[];
 
 static void __init init_idle_domain(void)
@@ -509,13 +499,21 @@ static void __init kexec_reserve_area(struct e820map *e820)
 
 static void noinline init_done(void)
 {
+    void *va;
+
     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);
+    /* Zero the .init code and data. */
+    for ( va = __init_begin; va < _p(__init_end); va += PAGE_SIZE )
+        clear_page(va);
+
+    /* Destroy Xen's mappings, and reuse the pages. */
+    destroy_xen_mappings((unsigned long)&__2M_init_start,
+                         (unsigned long)&__2M_init_end);
+    init_xenheap_pages(__pa(__2M_init_start), __pa(__2M_init_end));
+
     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
 
     startup_cpu_idle_loop();
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 77d0ed0..04f6b78 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -258,3 +258,6 @@ ASSERT(IS_ALIGNED(__2M_rwdata_start, MB(2)), "__2M_rwdata_start misaligned")
 ASSERT(IS_ALIGNED(__2M_rwdata_end,   MB(2)), "__2M_rwdata_end misaligned")
 
 ASSERT(IS_ALIGNED(cpu0_stack, STACK_SIZE), "cpu0_stack misaligned")
+
+ASSERT(IS_ALIGNED(__init_begin, PAGE_SIZE), "__init_begin misaligned")
+ASSERT(IS_ALIGNED(__init_end,   PAGE_SIZE), "__init_end misaligned")
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/lockprof: Move .lockprofile.data into .rodata
  2016-02-24 19:07 ` [PATCH] xen/lockprof: Move .lockprofile.data into .rodata Andrew Cooper
@ 2016-02-25 11:02   ` Stefano Stabellini
  2016-02-25 11:12     ` Stefano Stabellini
  0 siblings, 1 reply; 17+ messages in thread
From: Stefano Stabellini @ 2016-02-25 11:02 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Stefano Stabellini, Ian Campbell, Jan Beulich, Xen-devel

On Wed, 24 Feb 2016, Andrew Cooper wrote:
> The entire contents of .lockprofile.data are unchanging pointers to
> lock_profile structure in .data.  Annotate the type as such, and link the
> section in .rodata.  As these are just pointers, 32byte alignment is
> unnecessary.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> 
> v2:
>  * New
> v3:
>  * Explicitly introduce POINTER_ALIGN to avoid forcing 8 byte alignment on
>    arm32

Actually the way it is done in this patch, POINTER_ALIGN is 8 on arm64
and 4 on arm32 (see the definition of LONG_BYTEORDER in config.h). 

But I think it should still be OK.

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  xen/arch/arm/xen.lds.S       | 15 ++++++++-------
>  xen/arch/x86/xen.lds.S       | 14 +++++++-------
>  xen/include/asm-arm/config.h |  1 +
>  xen/include/asm-x86/config.h |  1 +
>  xen/include/xen/spinlock.h   |  2 +-
>  5 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index f501a2f..1b5e516 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -50,6 +50,14 @@ SECTIONS
>         __stop_bug_frames_2 = .;
>         *(.rodata)
>         *(.rodata.*)
> +
> +#ifdef LOCK_PROFILE
> +       . = ALIGN(POINTER_ALIGN);
> +       __lock_profile_start = .;
> +       *(.lockprofile.data)
> +       __lock_profile_end = .;
> +#endif
> +
>          _erodata = .;          /* End of read-only data */
>    } :text
>  
> @@ -83,13 +91,6 @@ SECTIONS
>         *(.data.rel.ro.*)
>    } :text
>  
> -#ifdef LOCK_PROFILE
> -  . = ALIGN(32);
> -  __lock_profile_start = .;
> -  .lockprofile.data : { *(.lockprofile.data) } :text
> -  __lock_profile_end = .;
> -#endif
> -
>    . = ALIGN(8);
>    .arch.info : {
>        _splatform = .;
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 9fde1db..8390ec2 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -65,6 +65,13 @@ SECTIONS
>  
>         *(.rodata)
>         *(.rodata.*)
> +
> +#ifdef LOCK_PROFILE
> +       . = ALIGN(POINTER_ALIGN);
> +       __lock_profile_start = .;
> +       *(.lockprofile.data)
> +       __lock_profile_end = .;
> +#endif
>    } :text
>  
>    . = ALIGN(SMP_CACHE_BYTES);
> @@ -97,13 +104,6 @@ SECTIONS
>         CONSTRUCTORS
>    } :text
>  
> -#ifdef LOCK_PROFILE
> -  . = ALIGN(32);
> -  __lock_profile_start = .;
> -  .lockprofile.data : { *(.lockprofile.data) } :text
> -  __lock_profile_end = .;
> -#endif
> -
>    . = ALIGN(PAGE_SIZE);             /* Init code and data */
>    __init_begin = .;
>    .init.text : {
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index c3a2c30..c0ad469 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -15,6 +15,7 @@
>  
>  #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
>  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
> +#define POINTER_ALIGN BYTES_PER_LONG
>  
>  /* xen_ulong_t is always 64 bits */
>  #define BITS_PER_XEN_ULONG 64
> diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
> index 07f3c1f..7291b59 100644
> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -13,6 +13,7 @@
>  #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
>  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
>  #define BITS_PER_BYTE 8
> +#define POINTER_ALIGN BYTES_PER_LONG
>  
>  #define BITS_PER_XEN_ULONG BITS_PER_LONG
>  
> diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
> index 77ab7d5..88b53f9 100644
> --- a/xen/include/xen/spinlock.h
> +++ b/xen/include/xen/spinlock.h
> @@ -79,7 +79,7 @@ struct lock_profile_qhead {
>  
>  #define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
>  #define _LOCK_PROFILE_PTR(name)                                               \
> -    static struct lock_profile *__lock_profile_##name                         \
> +    static struct lock_profile * const __lock_profile_##name                  \
>      __used_section(".lockprofile.data") =                                     \
>      &__lock_profile_data_##name
>  #define _SPIN_LOCK_UNLOCKED(x) { { 0 }, SPINLOCK_NO_CPU, 0, _LOCK_DEBUG, x }
> -- 
> 2.1.4
> 

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/lockprof: Move .lockprofile.data into .rodata
  2016-02-25 11:02   ` Stefano Stabellini
@ 2016-02-25 11:12     ` Stefano Stabellini
  0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2016-02-25 11:12 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Andrew Cooper, Stefano Stabellini, Ian Campbell, Jan Beulich, Xen-devel

On Thu, 25 Feb 2016, Stefano Stabellini wrote:
> On Wed, 24 Feb 2016, Andrew Cooper wrote:
> > The entire contents of .lockprofile.data are unchanging pointers to
> > lock_profile structure in .data.  Annotate the type as such, and link the
> > section in .rodata.  As these are just pointers, 32byte alignment is
> > unnecessary.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> > 
> > v2:
> >  * New
> > v3:
> >  * Explicitly introduce POINTER_ALIGN to avoid forcing 8 byte alignment on
> >    arm32
> 
> Actually the way it is done in this patch, POINTER_ALIGN is 8 on arm64
> and 4 on arm32 (see the definition of LONG_BYTEORDER in config.h). 

I misread the sentence in the commit message, everything good then :-)


> But I think it should still be OK.
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> 
> >  xen/arch/arm/xen.lds.S       | 15 ++++++++-------
> >  xen/arch/x86/xen.lds.S       | 14 +++++++-------
> >  xen/include/asm-arm/config.h |  1 +
> >  xen/include/asm-x86/config.h |  1 +
> >  xen/include/xen/spinlock.h   |  2 +-
> >  5 files changed, 18 insertions(+), 15 deletions(-)
> > 
> > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> > index f501a2f..1b5e516 100644
> > --- a/xen/arch/arm/xen.lds.S
> > +++ b/xen/arch/arm/xen.lds.S
> > @@ -50,6 +50,14 @@ SECTIONS
> >         __stop_bug_frames_2 = .;
> >         *(.rodata)
> >         *(.rodata.*)
> > +
> > +#ifdef LOCK_PROFILE
> > +       . = ALIGN(POINTER_ALIGN);
> > +       __lock_profile_start = .;
> > +       *(.lockprofile.data)
> > +       __lock_profile_end = .;
> > +#endif
> > +
> >          _erodata = .;          /* End of read-only data */
> >    } :text
> >  
> > @@ -83,13 +91,6 @@ SECTIONS
> >         *(.data.rel.ro.*)
> >    } :text
> >  
> > -#ifdef LOCK_PROFILE
> > -  . = ALIGN(32);
> > -  __lock_profile_start = .;
> > -  .lockprofile.data : { *(.lockprofile.data) } :text
> > -  __lock_profile_end = .;
> > -#endif
> > -
> >    . = ALIGN(8);
> >    .arch.info : {
> >        _splatform = .;
> > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> > index 9fde1db..8390ec2 100644
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -65,6 +65,13 @@ SECTIONS
> >  
> >         *(.rodata)
> >         *(.rodata.*)
> > +
> > +#ifdef LOCK_PROFILE
> > +       . = ALIGN(POINTER_ALIGN);
> > +       __lock_profile_start = .;
> > +       *(.lockprofile.data)
> > +       __lock_profile_end = .;
> > +#endif
> >    } :text
> >  
> >    . = ALIGN(SMP_CACHE_BYTES);
> > @@ -97,13 +104,6 @@ SECTIONS
> >         CONSTRUCTORS
> >    } :text
> >  
> > -#ifdef LOCK_PROFILE
> > -  . = ALIGN(32);
> > -  __lock_profile_start = .;
> > -  .lockprofile.data : { *(.lockprofile.data) } :text
> > -  __lock_profile_end = .;
> > -#endif
> > -
> >    . = ALIGN(PAGE_SIZE);             /* Init code and data */
> >    __init_begin = .;
> >    .init.text : {
> > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> > index c3a2c30..c0ad469 100644
> > --- a/xen/include/asm-arm/config.h
> > +++ b/xen/include/asm-arm/config.h
> > @@ -15,6 +15,7 @@
> >  
> >  #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
> >  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
> > +#define POINTER_ALIGN BYTES_PER_LONG
> >  
> >  /* xen_ulong_t is always 64 bits */
> >  #define BITS_PER_XEN_ULONG 64
> > diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
> > index 07f3c1f..7291b59 100644
> > --- a/xen/include/asm-x86/config.h
> > +++ b/xen/include/asm-x86/config.h
> > @@ -13,6 +13,7 @@
> >  #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
> >  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
> >  #define BITS_PER_BYTE 8
> > +#define POINTER_ALIGN BYTES_PER_LONG
> >  
> >  #define BITS_PER_XEN_ULONG BITS_PER_LONG
> >  
> > diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
> > index 77ab7d5..88b53f9 100644
> > --- a/xen/include/xen/spinlock.h
> > +++ b/xen/include/xen/spinlock.h
> > @@ -79,7 +79,7 @@ struct lock_profile_qhead {
> >  
> >  #define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
> >  #define _LOCK_PROFILE_PTR(name)                                               \
> > -    static struct lock_profile *__lock_profile_##name                         \
> > +    static struct lock_profile * const __lock_profile_##name                  \
> >      __used_section(".lockprofile.data") =                                     \
> >      &__lock_profile_data_##name
> >  #define _SPIN_LOCK_UNLOCKED(x) { { 0 }, SPINLOCK_NO_CPU, 0, _LOCK_DEBUG, x }
> > -- 
> > 2.1.4
> > 
> 

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-22 10:24         ` Andrew Cooper
@ 2016-02-22 10:43           ` Jan Beulich
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2016-02-22 10:43 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 22.02.16 at 11:24, <andrew.cooper3@citrix.com> wrote:
> On 22/02/16 09:55, Jan Beulich wrote:
>>>>> --- a/xen/arch/x86/xen.lds.S
>>>>> +++ b/xen/arch/x86/xen.lds.S
>>>>> @@ -38,6 +38,9 @@ SECTIONS
>>>>>    . = __XEN_VIRT_START;
>>>>>    __image_base__ = .;
>>>>>  #endif
>>>>> +
>>>>> +  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
>>>> Is the reason for aforementioned build problem perhaps the fact
>>>> that this label (and the others too) lives outside of any section?
>>> I am not sure.  It is only this symbol which is a problem.  All others
>>> are fine.
>>>
>>> I actually intended this to be an RFC patch, to see if anyone had
>>> suggestions.
>> Since you now imply this to be at the image base, I don't see
>> why using e.g. __XEN_VIRT_START (in its place, or via #define)
>> wouldn't be as good an option.
> 
> I don't understand what you mean here.
> 
> If you are suggesting #define __2M_text_start __XEN_VIRT_START then I
> don't see how that is going to help with the relocation.

Why not? __XEN_VIRT_START is a constant, i.e. won't result in
any relocations to get emitted.

Jan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-22  9:55       ` Jan Beulich
@ 2016-02-22 10:24         ` Andrew Cooper
  2016-02-22 10:43           ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-02-22 10:24 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Xen-devel

On 22/02/16 09:55, Jan Beulich wrote:
>
>>>> +                {
>>>> +                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
>>>> +                }
>>>> +                else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) &&
>>>> +                          i <  l2_table_offset((unsigned long)&__2M_rodata_end) )
>>>> +                {
>>>> +                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
>>>> +                }
>>>> +                else if ( (i >= l2_table_offset((unsigned long)&__2M_data_start) &&
>>>> +                           i <  l2_table_offset((unsigned long)&__2M_data_end)) ||
>>>> +                          (i >= l2_table_offset((unsigned long)&__2M_bss_start) &&
>>>> +                           i <  l2_table_offset((unsigned long)&__2M_bss_end)) )
>>> This is odd - why can't .data and .bss share a (multiple of) 2M
>>> region, at once presumably getting the whole image down to 10M
>>> again?
>> .init is between .data and .bss, to allow .bss to be the final section
>> and not included in the result of mkelf32
> But I don't think it needs to remain there? Should be possible to be
> put between .text and .rodata, or between .rodata and .data ...

Actually yes - shifting .init to between .rodata and .data should work fine.

>
>>>> --- a/xen/arch/x86/xen.lds.S
>>>> +++ b/xen/arch/x86/xen.lds.S
>>>> @@ -38,6 +38,9 @@ SECTIONS
>>>>    . = __XEN_VIRT_START;
>>>>    __image_base__ = .;
>>>>  #endif
>>>> +
>>>> +  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
>>> Is the reason for aforementioned build problem perhaps the fact
>>> that this label (and the others too) lives outside of any section?
>> I am not sure.  It is only this symbol which is a problem.  All others
>> are fine.
>>
>> I actually intended this to be an RFC patch, to see if anyone had
>> suggestions.
> Since you now imply this to be at the image base, I don't see
> why using e.g. __XEN_VIRT_START (in its place, or via #define)
> wouldn't be as good an option.

I don't understand what you mean here.

If you are suggesting #define __2M_text_start __XEN_VIRT_START then I
don't see how that is going to help with the relocation.

~Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-19 15:51     ` Andrew Cooper
@ 2016-02-22  9:55       ` Jan Beulich
  2016-02-22 10:24         ` Andrew Cooper
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2016-02-22  9:55 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 19.02.16 at 16:51, <andrew.cooper3@citrix.com> wrote:
> On 19/02/16 14:58, Jan Beulich wrote:
>>>>> On 18.02.16 at 19:03, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -921,13 +921,51 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>              /* The only data mappings to be relocated are in the Xen area. */
>>>              pl2e = __va(__pa(l2_xenmap));
>>>              *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
>>> -                                   PAGE_HYPERVISOR_RWX | _PAGE_PSE);
>>> +                                   PAGE_HYPERVISOR_RX | _PAGE_PSE);
>>>              for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
>>>              {
>>> +                unsigned int flags;
>>> +
>>>                  if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
>>>                      continue;
>>> -                *pl2e = l2e_from_intpte(l2e_get_intpte(*pl2e) +
>>> -                                        xen_phys_start);
>>> +
>>> +                if ( /*
>>> +                      * Should be:
>>> +                      *
>>> +                      * i >= l2_table_offset((unsigned long)&__2M_text_start) &&
>>> +                      *
>>> +                      * but the EFI build can't manage the relocation.  It
>>> +                      * evaluates to 0, so just use the upper bound.
>>> +                      */
>>> +                     i < l2_table_offset((unsigned long)&__2M_text_end) )
>> I'll need some more detail about this, not the least because
>> excusing what looks like a hack with EFI, under which we won't
>> ever get here, is suspicious.
> 
> Specifically, the EFI uses i386pep and it objects to a 64bit relocation
> of 0xfffffffffffffffc.
> 
> I can't explain why this symbol ends up with that relocation.

Interesting.

>>> +                {
>>> +                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
>>> +                }
>>> +                else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) &&
>>> +                          i <  l2_table_offset((unsigned long)&__2M_rodata_end) )
>>> +                {
>>> +                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
>>> +                }
>>> +                else if ( (i >= l2_table_offset((unsigned long)&__2M_data_start) &&
>>> +                           i <  l2_table_offset((unsigned long)&__2M_data_end)) ||
>>> +                          (i >= l2_table_offset((unsigned long)&__2M_bss_start) &&
>>> +                           i <  l2_table_offset((unsigned long)&__2M_bss_end)) )
>> This is odd - why can't .data and .bss share a (multiple of) 2M
>> region, at once presumably getting the whole image down to 10M
>> again?
> 
> .init is between .data and .bss, to allow .bss to be the final section
> and not included in the result of mkelf32

But I don't think it needs to remain there? Should be possible to be
put between .text and .rodata, or between .rodata and .data ...

>>> --- a/xen/arch/x86/xen.lds.S
>>> +++ b/xen/arch/x86/xen.lds.S
>>> @@ -38,6 +38,9 @@ SECTIONS
>>>    . = __XEN_VIRT_START;
>>>    __image_base__ = .;
>>>  #endif
>>> +
>>> +  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
>> Is the reason for aforementioned build problem perhaps the fact
>> that this label (and the others too) lives outside of any section?
> 
> I am not sure.  It is only this symbol which is a problem.  All others
> are fine.
> 
> I actually intended this to be an RFC patch, to see if anyone had
> suggestions.

Since you now imply this to be at the image base, I don't see
why using e.g. __XEN_VIRT_START (in its place, or via #define)
wouldn't be as good an option.

>>> --- a/xen/include/xen/kernel.h
>>> +++ b/xen/include/xen/kernel.h
>>> @@ -65,6 +65,12 @@
>>>  	1;                                      \
>>>  })
>>>  
>>> +extern unsigned long __2M_text_start[], __2M_text_end[];
>>> +extern unsigned long __2M_rodata_start[], __2M_rodata_end[];
>>> +extern unsigned long __2M_data_start[], __2M_data_end[];
>>> +extern unsigned long __2M_init_start[], __2M_init_end[];
>>> +extern unsigned long __2M_bss_start[], __2M_bss_end[];
>> I'd really like to see at least the ones which are reference to r/o
>> sections marked const.
> 
> Ok.  I should probably also make them char rather than unsigned long, so
> pointer arithmetic works sensibly for the length.

Good idea indeed.

Jan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-19 14:58   ` Jan Beulich
@ 2016-02-19 15:51     ` Andrew Cooper
  2016-02-22  9:55       ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-02-19 15:51 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Xen-devel

On 19/02/16 14:58, Jan Beulich wrote:
>>>> On 18.02.16 at 19:03, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -921,13 +921,51 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>              /* The only data mappings to be relocated are in the Xen area. */
>>              pl2e = __va(__pa(l2_xenmap));
>>              *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
>> -                                   PAGE_HYPERVISOR_RWX | _PAGE_PSE);
>> +                                   PAGE_HYPERVISOR_RX | _PAGE_PSE);
>>              for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
>>              {
>> +                unsigned int flags;
>> +
>>                  if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
>>                      continue;
>> -                *pl2e = l2e_from_intpte(l2e_get_intpte(*pl2e) +
>> -                                        xen_phys_start);
>> +
>> +                if ( /*
>> +                      * Should be:
>> +                      *
>> +                      * i >= l2_table_offset((unsigned long)&__2M_text_start) &&
>> +                      *
>> +                      * but the EFI build can't manage the relocation.  It
>> +                      * evaluates to 0, so just use the upper bound.
>> +                      */
>> +                     i < l2_table_offset((unsigned long)&__2M_text_end) )
> I'll need some more detail about this, not the least because
> excusing what looks like a hack with EFI, under which we won't
> ever get here, is suspicious.

Specifically, the EFI uses i386pep and it objects to a 64bit relocation
of 0xfffffffffffffffc.

I can't explain why this symbol ends up with that relocation.

>
>> +                {
>> +                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
>> +                }
>> +                else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) &&
>> +                          i <  l2_table_offset((unsigned long)&__2M_rodata_end) )
>> +                {
>> +                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
>> +                }
>> +                else if ( (i >= l2_table_offset((unsigned long)&__2M_data_start) &&
>> +                           i <  l2_table_offset((unsigned long)&__2M_data_end)) ||
>> +                          (i >= l2_table_offset((unsigned long)&__2M_bss_start) &&
>> +                           i <  l2_table_offset((unsigned long)&__2M_bss_end)) )
> This is odd - why can't .data and .bss share a (multiple of) 2M
> region, at once presumably getting the whole image down to 10M
> again?

.init is between .data and .bss, to allow .bss to be the final section
and not included in the result of mkelf32

>
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -38,6 +38,9 @@ SECTIONS
>>    . = __XEN_VIRT_START;
>>    __image_base__ = .;
>>  #endif
>> +
>> +  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
> Is the reason for aforementioned build problem perhaps the fact
> that this label (and the others too) lives outside of any section?

I am not sure.  It is only this symbol which is a problem.  All others
are fine.

I actually intended this to be an RFC patch, to see if anyone had
suggestions.

>
>> --- a/xen/include/xen/kernel.h
>> +++ b/xen/include/xen/kernel.h
>> @@ -65,6 +65,12 @@
>>  	1;                                      \
>>  })
>>  
>> +extern unsigned long __2M_text_start[], __2M_text_end[];
>> +extern unsigned long __2M_rodata_start[], __2M_rodata_end[];
>> +extern unsigned long __2M_data_start[], __2M_data_end[];
>> +extern unsigned long __2M_init_start[], __2M_init_end[];
>> +extern unsigned long __2M_bss_start[], __2M_bss_end[];
> I'd really like to see at least the ones which are reference to r/o
> sections marked const.

Ok.  I should probably also make them char rather than unsigned long, so
pointer arithmetic works sensibly for the length.

~Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-18 18:03 ` [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings Andrew Cooper
@ 2016-02-19 14:58   ` Jan Beulich
  2016-02-19 15:51     ` Andrew Cooper
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2016-02-19 14:58 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 18.02.16 at 19:03, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -921,13 +921,51 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>              /* The only data mappings to be relocated are in the Xen area. */
>              pl2e = __va(__pa(l2_xenmap));
>              *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
> -                                   PAGE_HYPERVISOR_RWX | _PAGE_PSE);
> +                                   PAGE_HYPERVISOR_RX | _PAGE_PSE);
>              for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
>              {
> +                unsigned int flags;
> +
>                  if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
>                      continue;
> -                *pl2e = l2e_from_intpte(l2e_get_intpte(*pl2e) +
> -                                        xen_phys_start);
> +
> +                if ( /*
> +                      * Should be:
> +                      *
> +                      * i >= l2_table_offset((unsigned long)&__2M_text_start) &&
> +                      *
> +                      * but the EFI build can't manage the relocation.  It
> +                      * evaluates to 0, so just use the upper bound.
> +                      */
> +                     i < l2_table_offset((unsigned long)&__2M_text_end) )

I'll need some more detail about this, not the least because
excusing what looks like a hack with EFI, under which we won't
ever get here, is suspicious.

> +                {
> +                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
> +                }
> +                else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) &&
> +                          i <  l2_table_offset((unsigned long)&__2M_rodata_end) )
> +                {
> +                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
> +                }
> +                else if ( (i >= l2_table_offset((unsigned long)&__2M_data_start) &&
> +                           i <  l2_table_offset((unsigned long)&__2M_data_end)) ||
> +                          (i >= l2_table_offset((unsigned long)&__2M_bss_start) &&
> +                           i <  l2_table_offset((unsigned long)&__2M_bss_end)) )

This is odd - why can't .data and .bss share a (multiple of) 2M
region, at once presumably getting the whole image down to 10M
again?

> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -38,6 +38,9 @@ SECTIONS
>    . = __XEN_VIRT_START;
>    __image_base__ = .;
>  #endif
> +
> +  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */

Is the reason for aforementioned build problem perhaps the fact
that this label (and the others too) lives outside of any section?

> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -65,6 +65,12 @@
>  	1;                                      \
>  })
>  
> +extern unsigned long __2M_text_start[], __2M_text_end[];
> +extern unsigned long __2M_rodata_start[], __2M_rodata_end[];
> +extern unsigned long __2M_data_start[], __2M_data_end[];
> +extern unsigned long __2M_init_start[], __2M_init_end[];
> +extern unsigned long __2M_bss_start[], __2M_bss_end[];

I'd really like to see at least the ones which are reference to r/o
sections marked const.

Jan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings
  2016-02-18 18:03 [PATCH] xen/x86: Map Xen code/data/bss with superpages Andrew Cooper
@ 2016-02-18 18:03 ` Andrew Cooper
  2016-02-19 14:58   ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-02-18 18:03 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

This balloons the size of Xen in memory from 3.4MB to 12MB, because of the
required alignment adjustments.

However
 * All mappings are 2M superpages.
 * .text (and .init at boot) are the only sections marked executable.
 * .text and .rodata are marked read-only.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/setup.c     | 44 +++++++++++++++++++++++++++++++++++++++++---
 xen/arch/x86/xen.lds.S   | 33 +++++++++++++++++++++++++++++++++
 xen/include/xen/kernel.h |  6 ++++++
 3 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index cddf954..c360fbc 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -921,13 +921,51 @@ void __init noreturn __start_xen(unsigned long mbi_p)
             /* The only data mappings to be relocated are in the Xen area. */
             pl2e = __va(__pa(l2_xenmap));
             *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
-                                   PAGE_HYPERVISOR_RWX | _PAGE_PSE);
+                                   PAGE_HYPERVISOR_RX | _PAGE_PSE);
             for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
             {
+                unsigned int flags;
+
                 if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
                     continue;
-                *pl2e = l2e_from_intpte(l2e_get_intpte(*pl2e) +
-                                        xen_phys_start);
+
+                if ( /*
+                      * Should be:
+                      *
+                      * i >= l2_table_offset((unsigned long)&__2M_text_start) &&
+                      *
+                      * but the EFI build can't manage the relocation.  It
+                      * evaluates to 0, so just use the upper bound.
+                      */
+                     i < l2_table_offset((unsigned long)&__2M_text_end) )
+                {
+                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
+                }
+                else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) &&
+                          i <  l2_table_offset((unsigned long)&__2M_rodata_end) )
+                {
+                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
+                }
+                else if ( (i >= l2_table_offset((unsigned long)&__2M_data_start) &&
+                           i <  l2_table_offset((unsigned long)&__2M_data_end)) ||
+                          (i >= l2_table_offset((unsigned long)&__2M_bss_start) &&
+                           i <  l2_table_offset((unsigned long)&__2M_bss_end)) )
+                {
+                    flags = PAGE_HYPERVISOR_RW | _PAGE_PSE;
+                }
+                else if ( i >= l2_table_offset((unsigned long)&__2M_init_start) &&
+                          i <  l2_table_offset((unsigned long)&__2M_init_end) )
+                {
+                    flags = PAGE_HYPERVISOR_RWX | _PAGE_PSE;
+                }
+                else
+                {
+                    *pl2e = l2e_empty();
+                    continue;
+                }
+
+                *pl2e = l2e_from_paddr(
+                    l2e_get_paddr(*pl2e) + xen_phys_start, flags);
             }
 
             /* Re-sync the stack and then switch to relocated pagetables. */
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 9fde1db..6c23c02 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -38,6 +38,9 @@ SECTIONS
   . = __XEN_VIRT_START;
   __image_base__ = .;
 #endif
+
+  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
+
   . = __XEN_VIRT_START + MB(1);
   _start = .;
   .text : {
@@ -50,6 +53,10 @@ SECTIONS
        _etext = .;             /* End of text section */
   } :text = 0x9090
 
+  . = ALIGN(MB(2));
+  __2M_text_end = .;
+
+  __2M_rodata_start = .;       /* Start of 2M superpages, mapped RO. */
   .rodata : {
        /* Bug frames table */
        . = ALIGN(4);
@@ -67,6 +74,10 @@ SECTIONS
        *(.rodata.*)
   } :text
 
+  . = ALIGN(MB(2));
+  __2M_rodata_end = .;
+
+  __2M_data_start = .;         /* Start of 2M superpages, mapped RW. */
   . = ALIGN(SMP_CACHE_BYTES);
   .data.read_mostly : {
        /* Exception table */
@@ -104,6 +115,10 @@ SECTIONS
   __lock_profile_end = .;
 #endif
 
+  . = ALIGN(MB(2));
+  __2M_data_end = .;
+
+  __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
   .init.text : {
@@ -166,6 +181,10 @@ SECTIONS
   . = ALIGN(STACK_SIZE);
   __init_end = .;
 
+  . = ALIGN(MB(2));
+  __2M_init_end = .;
+
+  __2M_bss_start = .;          /* Start of 2M superpages, mapped RW. */
   .bss : {                     /* BSS */
        __bss_start = .;
        *(.bss.stack_aligned)
@@ -183,6 +202,9 @@ SECTIONS
   } :text
   _end = . ;
 
+  . = ALIGN(MB(2));
+  __2M_bss_end = .;
+
 #ifdef EFI
   . = ALIGN(4);
   .reloc : {
@@ -229,4 +251,15 @@ ASSERT(__image_base__ > XEN_VIRT_START ||
 ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
 #endif
 
+ASSERT(IS_ALIGNED(__2M_text_start,   MB(2)), "__2M_text_start misaligned")
+ASSERT(IS_ALIGNED(__2M_text_end,     MB(2)), "__2M_text_end misaligned")
+ASSERT(IS_ALIGNED(__2M_rodata_start, MB(2)), "__2M_rodata_start misaligned")
+ASSERT(IS_ALIGNED(__2M_rodata_end,   MB(2)), "__2M_rodata_end misaligned")
+ASSERT(IS_ALIGNED(__2M_data_start,   MB(2)), "__2M_data_start misaligned")
+ASSERT(IS_ALIGNED(__2M_data_end,     MB(2)), "__2M_data_end misaligned")
+ASSERT(IS_ALIGNED(__2M_init_start,   MB(2)), "__2M_init_start misaligned")
+ASSERT(IS_ALIGNED(__2M_init_end,     MB(2)), "__2M_init_end misaligned")
+ASSERT(IS_ALIGNED(__2M_bss_start,    MB(2)), "__2M_bss_start misaligned")
+ASSERT(IS_ALIGNED(__2M_bss_end,      MB(2)), "__2M_bss_end misaligned")
+
 ASSERT(IS_ALIGNED(cpu0_stack, STACK_SIZE), "cpu0_stack misaligned")
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64d..8d08eca 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -65,6 +65,12 @@
 	1;                                      \
 })
 
+extern unsigned long __2M_text_start[], __2M_text_end[];
+extern unsigned long __2M_rodata_start[], __2M_rodata_end[];
+extern unsigned long __2M_data_start[], __2M_data_end[];
+extern unsigned long __2M_init_start[], __2M_init_end[];
+extern unsigned long __2M_bss_start[], __2M_bss_end[];
+
 extern char _start[], _end[], start[];
 #define is_kernel(p) ({                         \
     char *__p = (char *)(unsigned long)(p);     \
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-02-25 11:12 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-24 19:07 [PATCH] Map Xen code/data/bss with superpages Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/lockprof: Move .lockprofile.data into .rodata Andrew Cooper
2016-02-25 11:02   ` Stefano Stabellini
2016-02-25 11:12     ` Stefano Stabellini
2016-02-24 19:07 ` [PATCH] xen/x86: Improvements to build-time pagetable generation Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/x86: Construct the {l2, l3}_bootmap at compile time Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/memguard: Drop memguard_init() entirely Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/x86: Disable CR0.WP while applying alternatives Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/x86: Reorder .data and .init when linking Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings Andrew Cooper
2016-02-24 19:07 ` [PATCH] xen/x86: Unilaterally remove .init mappings Andrew Cooper
  -- strict thread matches above, loose matches on Subject: below --
2016-02-18 18:03 [PATCH] xen/x86: Map Xen code/data/bss with superpages Andrew Cooper
2016-02-18 18:03 ` [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings Andrew Cooper
2016-02-19 14:58   ` Jan Beulich
2016-02-19 15:51     ` Andrew Cooper
2016-02-22  9:55       ` Jan Beulich
2016-02-22 10:24         ` Andrew Cooper
2016-02-22 10:43           ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.