thr3ads.net - Xen devel - [PATCH 0/7] xen: arm: memory mangement fixes / improvements [Sep 2013]

If this information is useful, please help other people find it:
Share via:

Ian Campbell

2013-Sep-12 12:42 UTC

[PATCH 0/7] xen: arm: memory mangement fixes / improvements

The following fix a bunch of issues discovered while running on Midway
plus some random bits and bobs.

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 1/7] xen/arm: ensure the xenheap is 32MB aligned

My patch 08693f5948d8 "xen: arm: reduce the size of the xen heap to max 1/8
RAM size" unintentionally violated the constraint that the xenheap must be
32MB aligned, since we only explicitly align the end of the heap and
xenheap_pages was not a multiple of 32 pages.

Round xenheap pages up to a 32MB boundary.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/setup.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 4b31623..1ba2eb3 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -323,7 +323,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t
dtb_size)
      * constraints.
      */
     heap_pages = (ram_size >> PAGE_SHIFT);
-    xenheap_pages = max(heap_pages/8, 128UL<<(20-PAGE_SHIFT));
+    xenheap_pages = (heap_pages/8 + 0x1fffUL) & ~0x1fffUL;
+    xenheap_pages = max(xenheap_pages, 128UL<<(20-PAGE_SHIFT));
 
     do
     {
-- 
1.7.10.4

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 2/7] xen/arm: DOMHEAP_SECOND_PAGES is arm32 specific

since 5263507b1b4a "xen: arm: Use a direct mapping of RAM on arm64"

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/include/asm-arm/config.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 604088e..624c73e 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -136,6 +136,9 @@
 
 #define DOMHEAP_ENTRIES        1024  /* 1024 2MB mapping slots */
 
+/* Number of domheap pagetable pages required at the second level (2MB
mappings) */
+#define DOMHEAP_SECOND_PAGES ((DOMHEAP_VIRT_END - DOMHEAP_VIRT_START + 1)
>> FIRST_SHIFT)
+
 #else /* ARM_64 */
 
 #define SLOT0_ENTRY_BITS  39
@@ -159,9 +162,6 @@
 
 #endif
 
-/* Number of domheap pagetable pages required at the second level (2MB
mappings) */
-#define DOMHEAP_SECOND_PAGES ((DOMHEAP_VIRT_END - DOMHEAP_VIRT_START + 1)
>> FIRST_SHIFT)
-
 /* Fixmap slots */
 #define FIXMAP_CONSOLE  0  /* The primary UART */
 #define FIXMAP_PT       1  /* Temporary mappings of pagetable pages */
-- 
1.7.10.4

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 3/7] xen/arm: Reserve FDT via early module mechanism

This will stop us putting any heaps or relocating Xen itself over the FDT.

The devicetree will be copied to allocated memory in setup_mm and the original
copy will be freed by discard_initial_modules.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/setup.c          |    3 ++-
 xen/common/device_tree.c      |    9 ++++++++-
 xen/include/xen/device_tree.h |   11 ++++++-----
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 1ba2eb3..ab3d9aa 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -510,9 +510,10 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     smp_clear_cpu_maps();
 
+    /* This is mapped by head.S */
     device_tree_flattened = (void *)BOOT_MISC_VIRT_START
         + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
-    fdt_size = device_tree_early_init(device_tree_flattened);
+    fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
 
     cpus = smp_get_max_cpus();
     cmdline_parse(device_tree_bootargs(device_tree_flattened));
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index c4f0f2c..9e0c224 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -510,14 +510,21 @@ static void __init early_print_info(void)
  *
  * Returns the size of the DTB.
  */
-size_t __init device_tree_early_init(const void *fdt)
+size_t __init device_tree_early_init(const void *fdt, paddr_t paddr)
 {
+    struct dt_mb_module *mod;
     int ret;
 
     ret = fdt_check_header(fdt);
     if ( ret < 0 )
         early_panic("No valid device tree\n");
 
+    mod = &early_info.modules.module[MOD_FDT];
+    mod->start = paddr & PAGE_MASK;
+    mod->size = (fdt_totalsize(fdt) + ~PAGE_MASK) & PAGE_MASK;
+
+    early_info.modules.nr_mods = max(MOD_FDT, early_info.modules.nr_mods);
+
     device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
     early_print_info();
 
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 5cc1905..3e50383 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -21,11 +21,12 @@
 #define NR_MEM_BANKS 8
 
 #define MOD_XEN 0
-#define MOD_KERNEL 1
-#define MOD_INITRD 2
-#define NR_MODULES 3
+#define MOD_FDT 1
+#define MOD_KERNEL 2
+#define MOD_INITRD 3
+#define NR_MODULES 4
 
-#define MOD_DISCARD_FIRST MOD_KERNEL
+#define MOD_DISCARD_FIRST MOD_FDT
 
 struct membank {
     paddr_t start;
@@ -166,7 +167,7 @@ typedef int (*device_tree_node_func)(const void *fdt,
 extern struct dt_early_info early_info;
 extern void *device_tree_flattened;
 
-size_t __init device_tree_early_init(const void *fdt);
+size_t __init device_tree_early_init(const void *fdt, paddr_t paddr);
 
 void __init device_tree_get_reg(const u32 **cell, u32 address_cells,
                                 u32 size_cells,
-- 
1.7.10.4

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 4/7] xen/arm: cope with modules outside of "visible" RAM

This can happen if modules are in a bank which we can''t cope with e.g.
due to
being non-contiguous.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/setup.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ab3d9aa..95f22a1 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -380,6 +380,12 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t
dtb_size)
             e = n = ram_end;
         }
 
+        /* Module in RAM which we cannot see here, due to not handling
+         * non-contiguous memory regions yes
+         */
+        if ( e > ram_end )
+            e = ram_end;
+
         /* Avoid the xenheap */
         if ( s < ((xenheap_mfn_start+xenheap_pages) << PAGE_SHIFT)
              && (xenheap_mfn_start << PAGE_SHIFT) < e )
-- 
1.7.10.4

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 5/7] xen: support RAM at addresses 0 and 4096

Currently the mapping from pages to zones causes the page at zero to go into
zone -1 and the page at 4096 to go into zone 0, which is the Xen zone
(confusing various assertions).

Arrange instead for the mapping to be such that zone 0 is always reserved for
Xen and all other pages map to a zone >= 1.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: keir@xen.org
---
 xen/common/page_alloc.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 41251b2..e2cd139 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -257,11 +257,11 @@ unsigned long __init alloc_boot_pages(
  */
 
 #define MEMZONE_XEN 0
-#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT)
+#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT + 1 + 1)
 
-#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 0 : ((b) - PAGE_SHIFT -
1))
+#define bits_to_zone(b) (((b) < PAGE_SHIFT) ? 1 : ((b) - PAGE_SHIFT + 1 +
1))
 #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
-                          (fls(page_to_mfn(pg)) - 1))
+                          (fls(page_to_mfn(pg)) + 1))
 
 typedef struct page_list_head heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1];
 static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];
-- 
1.7.10.4

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 6/7] xen/arm: Support dtb /memreserve/ regions

This requires a mapping of the DTB during setup_mm. Previosly this was in the
BOOT_MISC slot, which is clobbered by setup_pagetables. Split it out into its
own slot which can be preserved.

Also handle this regions are part of consider_modules() and next_modules() to
ensure we do not locate any part of Xen or the heaps over them.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/arm32/head.S    |    2 +-
 xen/arch/arm/arm32/traps.c   |    3 +++
 xen/arch/arm/arm64/head.S    |    2 +-
 xen/arch/arm/mm.c            |   10 +++++++-
 xen/arch/arm/setup.c         |   58 ++++++++++++++++++++++++++++++++++++++++--
 xen/arch/arm/traps.c         |    3 ++-
 xen/common/device_tree.c     |   13 +++++++++-
 xen/include/asm-arm/config.h |    7 ++---
 xen/include/asm-arm/mm.h     |    2 ++
 9 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 79e95b6..5072e2a 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -301,7 +301,7 @@ cpu_init_done:
         orr   r2, r2, #PT_UPPER(MEM)
         orr   r2, r2, #PT_LOWER(MEM) /* r2:r3 := 2MB RAM incl. DTB */
         add   r4, r4, #8
-        strd  r2, r3, [r1, r4]       /* Map it in the early boot slot */
+        strd  r2, r3, [r1, r4]       /* Map it in the early fdt slot */
 
 pt_ready:
         PRINT("- Turning on paging -\r\n")
diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index ff0b945..e8dd9f5 100644
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -22,6 +22,7 @@
 #include <public/xen.h>
 
 #include <asm/processor.h>
+#include <asm/early_printk.h>
 
 asmlinkage void do_trap_undefined_instruction(struct cpu_user_regs *regs)
 {
@@ -40,6 +41,8 @@ asmlinkage void do_trap_prefetch_abort(struct cpu_user_regs
*regs)
 
 asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
 {
+    early_printk("Data Abort at %"PRIvaddr" DFAR
%"PRIx32"\n",
+                 regs->pc, READ_CP32(DFAR));
     do_unexpected_trap("Data Abort", regs);
 }
 
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 21b7e4d..33ff489 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -255,7 +255,7 @@ skip_bss:
         mov   x3, #PT_MEM            /* x2 := 2MB RAM incl. DTB */
         orr   x2, x2, x3
         add   x4, x4, #8
-        str   x2, [x1, x4]           /* Map it in the early boot slot */
+        str   x2, [x1, x4]           /* Map it in the early fdt slot */
 
 pt_ready:
         PRINT("- Turning on paging -\r\n")
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 69c157a..86e3207 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -361,6 +361,13 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
     return mfn_to_xen_entry(mfn);
 }
 
+void __init remove_early_mappings(void)
+{
+    lpae_t pte = {0};
+    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
+    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, SECOND_SIZE);
+}
+
 /* Boot-time pagetable setup.
  * Changes here may need matching changes in head.S */
 void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
@@ -401,7 +408,8 @@ void __init setup_pagetables(unsigned long boot_phys_offset,
paddr_t xen_paddr)
         p[second_linear_offset(va)].bits = 0;
     }
     for ( i = 0; i < 4 * LPAE_ENTRIES; i++)
-        if ( p[i].pt.valid )
+        /* The FDT is not relocated */
+        if ( p[i].pt.valid && i !=
second_linear_offset(BOOT_FDT_VIRT_START) )
             p[i].pt.base += (phys_offset - boot_phys_offset) >>
PAGE_SHIFT;
 
     /* Change pagetables to the copy in the relocated Xen */
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 95f22a1..d88f121 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -35,6 +35,7 @@
 #include <xen/cpu.h>
 #include <xen/pfn.h>
 #include <xen/vmap.h>
+#include <xen/libfdt/libfdt.h>
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/setup.h>
@@ -161,6 +162,8 @@ void __init discard_initial_modules(void)
     }
 
     mi->nr_mods = 0;
+
+    remove_early_mappings();
 }
 
 /*
@@ -177,6 +180,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
 {
     const struct dt_module_info *mi = &early_info.modules;
     int i;
+    int nr_rsvd;
 
     s = (s+align-1) & ~(align-1);
     e = e & ~(align-1);
@@ -184,6 +188,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
     if ( s > e ||  e - s < size )
         return 0;
 
+    /* First check the boot modules */
     for ( i = first_mod; i <= mi->nr_mods; i++ )
     {
         paddr_t mod_s = mi->module[i].start;
@@ -199,6 +204,32 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t
e,
         }
     }
 
+    /* Now check any fdt reserved areas. */
+
+    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
+
+    for ( ; i < mi->nr_mods + nr_rsvd; i++ )
+    {
+        paddr_t mod_s, mod_e;
+
+        if ( fdt_get_mem_rsv(device_tree_flattened,
+                             i - mi->nr_mods,
+                             &mod_s, &mod_e ) < 0 )
+            /* If we can''t read it, pretend it doesn''t
exist... */
+            continue;
+
+        /* fdt_get_mem_rsv returns length */
+        mod_e += mod_s;
+
+        if ( s < mod_e && mod_s < e )
+        {
+            mod_e = consider_modules(mod_e, e, size, align, i+1);
+            if ( mod_e )
+                return mod_e;
+
+            return consider_modules(s, mod_s, size, align, i+1);
+        }
+    }
     return e;
 }
 
@@ -212,7 +243,29 @@ static paddr_t __init next_module(paddr_t s, paddr_t *n)
 {
     struct dt_module_info *mi = &early_info.modules;
     paddr_t lowest = ~(paddr_t)0;
-    int i;
+    int i, nr_rsvd;
+
+    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
+
+    for ( i=0 ; i < nr_rsvd; i++ )
+    {
+        paddr_t mod_s, mod_e;
+
+        if ( fdt_get_mem_rsv(device_tree_flattened, i,
+                             &mod_s, &mod_e ) < 0 )
+            /* If we can''t read it, pretend it doesn''t
exist... */
+            continue;
+
+        /* fdt_get_mem_rsv returns length */
+        mod_e += mod_s;
+
+        if ( mod_s < s )
+            continue;
+        if ( mod_s > lowest )
+            continue;
+        lowest = mod_s;
+        *n = mod_e;
+    }
 
     for ( i = 0; i <= mi->nr_mods; i++ )
     {
@@ -226,6 +279,7 @@ static paddr_t __init next_module(paddr_t s, paddr_t *n)
         lowest = mod_s;
         *n = mod_e;
     }
+
     return lowest;
 }
 
@@ -517,7 +571,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     smp_clear_cpu_maps();
 
     /* This is mapped by head.S */
-    device_tree_flattened = (void *)BOOT_MISC_VIRT_START
+    device_tree_flattened = (void *)BOOT_FDT_VIRT_START
         + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
     fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 0e9a141..a78c1e0 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -35,6 +35,7 @@
 #include <asm/regs.h>
 #include <asm/cpregs.h>
 #include <asm/psci.h>
+#include <asm/early_printk.h>
 
 #include "decode.h"
 #include "io.h"
@@ -810,7 +811,7 @@ void vcpu_show_execution_state(struct vcpu *v)
 
 void do_unexpected_trap(const char *msg, struct cpu_user_regs *regs)
 {
-    printk("CPU%d: Unexpected Trap: %s\n", smp_processor_id(), msg);
+    early_printk("CPU%d: Unexpected Trap: %s\n", smp_processor_id(),
msg);
     show_execution_state(regs);
     while(1);
 }
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 9e0c224..132a2bd 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -489,7 +489,7 @@ static void __init early_print_info(void)
 {
     struct dt_mem_info *mi = &early_info.mem;
     struct dt_module_info *mods = &early_info.modules;
-    int i;
+    int i, nr_rsvd;
 
     for ( i = 0; i < mi->nr_banks; i++ )
         early_printk("RAM: %"PRIpaddr" -
%"PRIpaddr"\n",
@@ -502,6 +502,17 @@ static void __init early_print_info(void)
                      mods->module[i].start,
                      mods->module[i].start + mods->module[i].size,
                      mods->module[i].cmdline);
+    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
+    for ( i = 0; i < nr_rsvd; i++ )
+    {
+        paddr_t s, e;
+        if ( fdt_get_mem_rsv(device_tree_flattened, i, &s, &e) < 0 )
+            continue;
+        /* fdt_get_mem_rsv returns length */
+        e += s;
+        early_printk(" RESVD[%d]: %"PRIpaddr" -
%"PRIpaddr"\n",
+                     i, s, e);
+    }
 }
 
 /**
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 624c73e..efeb952 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -80,10 +80,10 @@
  *   0  -   2M   Unmapped
  *   2M -   4M   Xen text, data, bss
  *   4M -   6M   Fixmap: special-purpose 4K mapping slots
- *   6M -   8M   Early boot misc (see below)
+ *   6M -   8M   Early boot mapping of FDT
+ *   8M -  10M   Early boot misc (see below)
  *
  * The early boot misc area is used:
- *   - in head.S for the DTB for device_tree_early_init().
  *   - in setup_pagetables() when relocating Xen.
  *
  * ARM32 layout:
@@ -116,7 +116,8 @@
 
 #define XEN_VIRT_START         _AT(vaddr_t,0x00200000)
 #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
-#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00600000)
+#define BOOT_FDT_VIRT_START    _AT(vaddr_t,0x00600000)
+#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00800000)
 
 #define HYPERVISOR_VIRT_START  XEN_VIRT_START
 
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 97c2ee0..467687a 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -155,6 +155,8 @@ extern unsigned long total_pages;
 
 /* Boot-time pagetable setup */
 extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t
xen_paddr);
+/* Remove early mappings */
+extern void remove_early_mappings(void);
 /* Allocate and initialise pagetables for a secondary CPU */
 extern int __cpuinit init_secondary_pagetables(int cpu);
 /* Switch secondary CPUS to its own pagetables and finalise MMU setup */
-- 
1.7.10.4

Ian Campbell

2013-Sep-12 12:42 UTC

head link

[PATCH 7/7] xen/arm: rename boot misc region to boot reloc now it has a single purpose

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/mm.c            |    4 ++--
 xen/include/asm-arm/config.h |    7 ++-----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 86e3207..06670e0 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -128,7 +128,7 @@ static inline void
check_memory_layout_alignment_constraints(void) {
     /* 2MB aligned regions */
     BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
     BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
-    BUILD_BUG_ON(BOOT_MISC_VIRT_START & ~SECOND_MASK);
+    BUILD_BUG_ON(BOOT_RELOC_VIRT_START & ~SECOND_MASK);
     /* 1GB aligned regions */
     BUILD_BUG_ON(XENHEAP_VIRT_START & ~FIRST_MASK);
 #ifdef CONFIG_DOMAIN_PAGE
@@ -377,7 +377,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset,
paddr_t xen_paddr)
     int i;
 
     /* Map the destination in the boot misc area. */
-    dest_va = BOOT_MISC_VIRT_START;
+    dest_va = BOOT_RELOC_VIRT_START;
     pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT);
     write_pte(xen_second + second_table_offset(dest_va), pte);
     flush_xen_data_tlb_range_va(dest_va, SECOND_SIZE);
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index efeb952..9e395c2 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -81,10 +81,7 @@
  *   2M -   4M   Xen text, data, bss
  *   4M -   6M   Fixmap: special-purpose 4K mapping slots
  *   6M -   8M   Early boot mapping of FDT
- *   8M -  10M   Early boot misc (see below)
- *
- * The early boot misc area is used:
- *   - in setup_pagetables() when relocating Xen.
+ *   8M -  10M   Early relocation address (used when relocating Xen)
  *
  * ARM32 layout:
  *   0  -   8M   <COMMON>
@@ -117,7 +114,7 @@
 #define XEN_VIRT_START         _AT(vaddr_t,0x00200000)
 #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
 #define BOOT_FDT_VIRT_START    _AT(vaddr_t,0x00600000)
-#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00800000)
+#define BOOT_RELOC_VIRT_START  _AT(vaddr_t,0x00800000)
 
 #define HYPERVISOR_VIRT_START  XEN_VIRT_START
 
-- 
1.7.10.4

Julien Grall

2013-Sep-12 13:03 UTC

head link

Re: [PATCH 6/7] xen/arm: Support dtb /memreserve/ regions

On 09/12/2013 01:42 PM, Ian Campbell wrote:> This requires a mapping of the DTB during setup_mm. Previosly this was in
the
> BOOT_MISC slot, which is clobbered by setup_pagetables. Split it out into
its
> own slot which can be preserved.
> 
> Also handle this regions are part of consider_modules() and next_modules()
to
> ensure we do not locate any part of Xen or the heaps over them.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/arm32/head.S    |    2 +-
>  xen/arch/arm/arm32/traps.c   |    3 +++
>  xen/arch/arm/arm64/head.S    |    2 +-
>  xen/arch/arm/mm.c            |   10 +++++++-
>  xen/arch/arm/setup.c         |   58
++++++++++++++++++++++++++++++++++++++++--
>  xen/arch/arm/traps.c         |    3 ++-
>  xen/common/device_tree.c     |   13 +++++++++-
>  xen/include/asm-arm/config.h |    7 ++---
>  xen/include/asm-arm/mm.h     |    2 ++
>  9 files changed, 90 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 79e95b6..5072e2a 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -301,7 +301,7 @@ cpu_init_done:
>          orr   r2, r2, #PT_UPPER(MEM)
>          orr   r2, r2, #PT_LOWER(MEM) /* r2:r3 := 2MB RAM incl. DTB */
>          add   r4, r4, #8
> -        strd  r2, r3, [r1, r4]       /* Map it in the early boot slot */
> +        strd  r2, r3, [r1, r4]       /* Map it in the early fdt slot */
>  
>  pt_ready:
>          PRINT("- Turning on paging -\r\n")
> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> index ff0b945..e8dd9f5 100644
> --- a/xen/arch/arm/arm32/traps.c
> +++ b/xen/arch/arm/arm32/traps.c
> @@ -22,6 +22,7 @@
>  #include <public/xen.h>
>  
>  #include <asm/processor.h>
> +#include <asm/early_printk.h>
>  
>  asmlinkage void do_trap_undefined_instruction(struct cpu_user_regs *regs)
>  {
> @@ -40,6 +41,8 @@ asmlinkage void do_trap_prefetch_abort(struct
cpu_user_regs *regs)
>  
>  asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
>  {
> +    early_printk("Data Abort at %"PRIvaddr" DFAR
%"PRIx32"\n",
> +                 regs->pc, READ_CP32(DFAR));
early_printk belongs to __init and will be discarded by
free_init_memory. Any data abort after this call will result to a
"double" hang.
>      do_unexpected_trap("Data Abort", regs);
>  }
>  
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 21b7e4d..33ff489 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -255,7 +255,7 @@ skip_bss:
>          mov   x3, #PT_MEM            /* x2 := 2MB RAM incl. DTB */
>          orr   x2, x2, x3
>          add   x4, x4, #8
> -        str   x2, [x1, x4]           /* Map it in the early boot slot */
> +        str   x2, [x1, x4]           /* Map it in the early fdt slot */
>  
>  pt_ready:
>          PRINT("- Turning on paging -\r\n")
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 69c157a..86e3207 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -361,6 +361,13 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
>      return mfn_to_xen_entry(mfn);
>  }
>  
> +void __init remove_early_mappings(void)
> +{
> +    lpae_t pte = {0};
> +    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
> +    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, SECOND_SIZE);
> +}
> +
>  /* Boot-time pagetable setup.
>   * Changes here may need matching changes in head.S */
>  void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t
xen_paddr)
> @@ -401,7 +408,8 @@ void __init setup_pagetables(unsigned long
boot_phys_offset, paddr_t xen_paddr)
>          p[second_linear_offset(va)].bits = 0;
>      }
>      for ( i = 0; i < 4 * LPAE_ENTRIES; i++)
> -        if ( p[i].pt.valid )
> +        /* The FDT is not relocated */
> +        if ( p[i].pt.valid && i !=
second_linear_offset(BOOT_FDT_VIRT_START) )
>              p[i].pt.base += (phys_offset - boot_phys_offset) >>
PAGE_SHIFT;
>  
>      /* Change pagetables to the copy in the relocated Xen */
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 95f22a1..d88f121 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -35,6 +35,7 @@
>  #include <xen/cpu.h>
>  #include <xen/pfn.h>
>  #include <xen/vmap.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <asm/page.h>
>  #include <asm/current.h>
>  #include <asm/setup.h>
> @@ -161,6 +162,8 @@ void __init discard_initial_modules(void)
>      }
>  
>      mi->nr_mods = 0;
> +
> +    remove_early_mappings();
>  }
>  
>  /*
> @@ -177,6 +180,7 @@ static paddr_t __init consider_modules(paddr_t s,
paddr_t e,
>  {
>      const struct dt_module_info *mi = &early_info.modules;
>      int i;
> +    int nr_rsvd;
>  
>      s = (s+align-1) & ~(align-1);
>      e = e & ~(align-1);
> @@ -184,6 +188,7 @@ static paddr_t __init consider_modules(paddr_t s,
paddr_t e,
>      if ( s > e ||  e - s < size )
>          return 0;
>  
> +    /* First check the boot modules */
>      for ( i = first_mod; i <= mi->nr_mods; i++ )
>      {
>          paddr_t mod_s = mi->module[i].start;
> @@ -199,6 +204,32 @@ static paddr_t __init consider_modules(paddr_t s,
paddr_t e,
>          }
>      }
>  
> +    /* Now check any fdt reserved areas. */
> +
> +    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> +
> +    for ( ; i < mi->nr_mods + nr_rsvd; i++ )
> +    {
> +        paddr_t mod_s, mod_e;
> +
> +        if ( fdt_get_mem_rsv(device_tree_flattened,
> +                             i - mi->nr_mods,
> +                             &mod_s, &mod_e ) < 0 )
> +            /* If we can''t read it, pretend it doesn''t
exist... */
> +            continue;
> +
> +        /* fdt_get_mem_rsv returns length */
> +        mod_e += mod_s;
> +
> +        if ( s < mod_e && mod_s < e )
> +        {
> +            mod_e = consider_modules(mod_e, e, size, align, i+1);
> +            if ( mod_e )
> +                return mod_e;
> +
> +            return consider_modules(s, mod_s, size, align, i+1);
> +        }
> +    }
>      return e;
>  }
>  
> @@ -212,7 +243,29 @@ static paddr_t __init next_module(paddr_t s, paddr_t
*n)
>  {
>      struct dt_module_info *mi = &early_info.modules;
>      paddr_t lowest = ~(paddr_t)0;
> -    int i;
> +    int i, nr_rsvd;
> +
> +    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> +
> +    for ( i=0 ; i < nr_rsvd; i++ )
> +    {
> +        paddr_t mod_s, mod_e;
> +
> +        if ( fdt_get_mem_rsv(device_tree_flattened, i,
> +                             &mod_s, &mod_e ) < 0 )
> +            /* If we can''t read it, pretend it doesn''t
exist... */
> +            continue;
> +
> +        /* fdt_get_mem_rsv returns length */
> +        mod_e += mod_s;
> +
> +        if ( mod_s < s )
> +            continue;
> +        if ( mod_s > lowest )
> +            continue;
> +        lowest = mod_s;
> +        *n = mod_e;
> +    }
You use this same loop for ... fdt_get ... on different place. Can  you
add a function fdt_for_each_memreserve?>  
>      for ( i = 0; i <= mi->nr_mods; i++ )
>      {
> @@ -226,6 +279,7 @@ static paddr_t __init next_module(paddr_t s, paddr_t
*n)
>          lowest = mod_s;
>          *n = mod_e;
>      }
> +
Spurious line?
>      return lowest;
>  }
>  
> @@ -517,7 +571,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>      smp_clear_cpu_maps();
>  
>      /* This is mapped by head.S */
> -    device_tree_flattened = (void *)BOOT_MISC_VIRT_START
> +    device_tree_flattened = (void *)BOOT_FDT_VIRT_START
>          + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
>      fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 0e9a141..a78c1e0 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -35,6 +35,7 @@
>  #include <asm/regs.h>
>  #include <asm/cpregs.h>
>  #include <asm/psci.h>
> +#include <asm/early_printk.h>
>  
>  #include "decode.h"
>  #include "io.h"
> @@ -810,7 +811,7 @@ void vcpu_show_execution_state(struct vcpu *v)
>  
>  void do_unexpected_trap(const char *msg, struct cpu_user_regs *regs)
>  {
> -    printk("CPU%d: Unexpected Trap: %s\n", smp_processor_id(),
msg);
> +    early_printk("CPU%d: Unexpected Trap: %s\n",
smp_processor_id(), msg);
Same issue as do data_abort_trap
>      show_execution_state(regs);
>      while(1);
>  }
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 9e0c224..132a2bd 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -489,7 +489,7 @@ static void __init early_print_info(void)
>  {
>      struct dt_mem_info *mi = &early_info.mem;
>      struct dt_module_info *mods = &early_info.modules;
> -    int i;
> +    int i, nr_rsvd;
>  
>      for ( i = 0; i < mi->nr_banks; i++ )
>          early_printk("RAM: %"PRIpaddr" -
%"PRIpaddr"\n",
> @@ -502,6 +502,17 @@ static void __init early_print_info(void)
>                       mods->module[i].start,
>                       mods->module[i].start + mods->module[i].size,
>                       mods->module[i].cmdline);
> +    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> +    for ( i = 0; i < nr_rsvd; i++ )
> +    {
> +        paddr_t s, e;
> +        if ( fdt_get_mem_rsv(device_tree_flattened, i, &s, &e)
< 0 )
> +            continue;
> +        /* fdt_get_mem_rsv returns length */
> +        e += s;
> +        early_printk(" RESVD[%d]: %"PRIpaddr" -
%"PRIpaddr"\n",
> +                     i, s, e);
> +    }
>  }
>  
>  /**
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 624c73e..efeb952 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -80,10 +80,10 @@
>   *   0  -   2M   Unmapped
>   *   2M -   4M   Xen text, data, bss
>   *   4M -   6M   Fixmap: special-purpose 4K mapping slots
> - *   6M -   8M   Early boot misc (see below)
> + *   6M -   8M   Early boot mapping of FDT
> + *   8M -  10M   Early boot misc (see below)
>   *
>   * The early boot misc area is used:
> - *   - in head.S for the DTB for device_tree_early_init().
>   *   - in setup_pagetables() when relocating Xen.
>   *
>   * ARM32 layout:
> @@ -116,7 +116,8 @@
>  
>  #define XEN_VIRT_START         _AT(vaddr_t,0x00200000)
>  #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
> -#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00600000)
> +#define BOOT_FDT_VIRT_START    _AT(vaddr_t,0x00600000)
> +#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00800000)
>  
>  #define HYPERVISOR_VIRT_START  XEN_VIRT_START
>  
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index 97c2ee0..467687a 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -155,6 +155,8 @@ extern unsigned long total_pages;
>  
>  /* Boot-time pagetable setup */
>  extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t
xen_paddr);
> +/* Remove early mappings */
> +extern void remove_early_mappings(void);
>  /* Allocate and initialise pagetables for a secondary CPU */
>  extern int __cpuinit init_secondary_pagetables(int cpu);
>  /* Switch secondary CPUS to its own pagetables and finalise MMU setup */
> 

-- 
Julien Grall

Jan Beulich

2013-Sep-12 13:25 UTC

head link

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

>>> On 12.09.13 at 14:42, Ian Campbell <ian.campbell@citrix.com>
wrote:
> Currently the mapping from pages to zones causes the page at zero to go
into
> zone -1 and the page at 4096 to go into zone 0, which is the Xen zone
> (confusing various assertions).
So that''s a problem on ARM only, right? Because x86 avoids passing
the low first Mb to the allocator. I wonder whether ARM shouldn''t at
least avoid making the page at 0 available for allocation too, which
would address half of the problem. Avoiding MFN 1 would be less
natural, I agree.
> Arrange instead for the mapping to be such that zone 0 is always reserved
for
> Xen and all other pages map to a zone >= 1.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: keir@xen.org 
> ---
>  xen/common/page_alloc.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 41251b2..e2cd139 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -257,11 +257,11 @@ unsigned long __init alloc_boot_pages(
>   */
>  
>  #define MEMZONE_XEN 0
> -#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT)
> +#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT + 1 + 1)
So we''ve got a total delta of two between old and new handling
here...
> -#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 0 : ((b) -
PAGE_SHIFT - 1))
> +#define bits_to_zone(b) (((b) < PAGE_SHIFT) ? 1 : ((b) - PAGE_SHIFT + 1
+ 1))
... but a delta of 3 on the right side of the colon here (the delta being
just one on the left side is the actual meat of your fix as I understand
it). Or, taking a slightly different perspective, using < and <= in the
condition should not alter the result (but does if I''m not mistaken).
>  #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
> -                          (fls(page_to_mfn(pg)) - 1))
> +                          (fls(page_to_mfn(pg)) + 1))
And the total delta is 2 here again.

Overall I''m really uncertain whether it wouldn''t be better for
ARM to
play by the x86 rules in this respect, or alternatively to further
generalize what you try to do here by allowing x86 to specify a bias
for the shift to skip all zones currently covering the low Mb, which on
ARM would end up being 1.

Jan

Julien Grall

2013-Sep-12 13:39 UTC

head link

Re: [PATCH 3/7] xen/arm: Reserve FDT via early module mechanism

On 09/12/2013 01:42 PM, Ian Campbell wrote:> This will stop us putting any heaps or relocating Xen itself over the FDT.
> 
> The devicetree will be copied to allocated memory in setup_mm and the
original
> copy will be freed by discard_initial_modules.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/setup.c          |    3 ++-
>  xen/common/device_tree.c      |    9 ++++++++-
>  xen/include/xen/device_tree.h |   11 ++++++-----
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 1ba2eb3..ab3d9aa 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -510,9 +510,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      smp_clear_cpu_maps();
>  
> +    /* This is mapped by head.S */
>      device_tree_flattened = (void *)BOOT_MISC_VIRT_START
>          + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
> -    fdt_size = device_tree_early_init(device_tree_flattened);
> +    fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
>  
>      cpus = smp_get_max_cpus();
>      cmdline_parse(device_tree_bootargs(device_tree_flattened));
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index c4f0f2c..9e0c224 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -510,14 +510,21 @@ static void __init early_print_info(void)
>   *
>   * Returns the size of the DTB.
>   */
> -size_t __init device_tree_early_init(const void *fdt)
> +size_t __init device_tree_early_init(const void *fdt, paddr_t paddr)
>  {
> +    struct dt_mb_module *mod;
>      int ret;
>  
>      ret = fdt_check_header(fdt);
>      if ( ret < 0 )
>          early_panic("No valid device tree\n");
>  
> +    mod = &early_info.modules.module[MOD_FDT];
> +    mod->start = paddr & PAGE_MASK;
> +    mod->size = (fdt_totalsize(fdt) + ~PAGE_MASK) & PAGE_MASK;
I''m not sure, s/~PAGE_MASK/PAGE_SIZE ?
> +
> +    early_info.modules.nr_mods = max(MOD_FDT, early_info.modules.nr_mods);
> +
>      device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
>      early_print_info();
>  
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 5cc1905..3e50383 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -21,11 +21,12 @@
>  #define NR_MEM_BANKS 8
>  
>  #define MOD_XEN 0
> -#define MOD_KERNEL 1
> -#define MOD_INITRD 2
> -#define NR_MODULES 3
> +#define MOD_FDT 1
> +#define MOD_KERNEL 2
> +#define MOD_INITRD 3
> +#define NR_MODULES 4
>  
> -#define MOD_DISCARD_FIRST MOD_KERNEL
> +#define MOD_DISCARD_FIRST MOD_FDT
>  
>  struct membank {
>      paddr_t start;
> @@ -166,7 +167,7 @@ typedef int (*device_tree_node_func)(const void *fdt,
>  extern struct dt_early_info early_info;
>  extern void *device_tree_flattened;
>  
> -size_t __init device_tree_early_init(const void *fdt);
> +size_t __init device_tree_early_init(const void *fdt, paddr_t paddr);
>  
>  void __init device_tree_get_reg(const u32 **cell, u32 address_cells,
>                                  u32 size_cells,
> 

-- 
Julien Grall

Ian Campbell

2013-Sep-12 13:47 UTC

head link

Re: [PATCH 6/7] xen/arm: Support dtb /memreserve/ regions

On Thu, 2013-09-12 at 14:03 +0100, Julien Grall wrote:> On 09/12/2013 01:42 PM, Ian Campbell wrote:
> > This requires a mapping of the DTB during setup_mm. Previosly this was
in the
> > BOOT_MISC slot, which is clobbered by setup_pagetables. Split it out
into its
> > own slot which can be preserved.
> > 
> > Also handle this regions are part of consider_modules() and
next_modules() to
> > ensure we do not locate any part of Xen or the heaps over them.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/arm32/head.S    |    2 +-
> >  xen/arch/arm/arm32/traps.c   |    3 +++
> >  xen/arch/arm/arm64/head.S    |    2 +-
> >  xen/arch/arm/mm.c            |   10 +++++++-
> >  xen/arch/arm/setup.c         |   58
++++++++++++++++++++++++++++++++++++++++--
> >  xen/arch/arm/traps.c         |    3 ++-
> >  xen/common/device_tree.c     |   13 +++++++++-
> >  xen/include/asm-arm/config.h |    7 ++---
> >  xen/include/asm-arm/mm.h     |    2 ++
> >  9 files changed, 90 insertions(+), 10 deletions(-)
> > 
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index 79e95b6..5072e2a 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -301,7 +301,7 @@ cpu_init_done:
> >          orr   r2, r2, #PT_UPPER(MEM)
> >          orr   r2, r2, #PT_LOWER(MEM) /* r2:r3 := 2MB RAM incl. DTB */
> >          add   r4, r4, #8
> > -        strd  r2, r3, [r1, r4]       /* Map it in the early boot slot
*/
> > +        strd  r2, r3, [r1, r4]       /* Map it in the early fdt slot
*/
> >  
> >  pt_ready:
> >          PRINT("- Turning on paging -\r\n")
> > diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> > index ff0b945..e8dd9f5 100644
> > --- a/xen/arch/arm/arm32/traps.c
> > +++ b/xen/arch/arm/arm32/traps.c
> > @@ -22,6 +22,7 @@
> >  #include <public/xen.h>
> >  
> >  #include <asm/processor.h>
> > +#include <asm/early_printk.h>
> >  
> >  asmlinkage void do_trap_undefined_instruction(struct cpu_user_regs
*regs)
> >  {
> > @@ -40,6 +41,8 @@ asmlinkage void do_trap_prefetch_abort(struct
cpu_user_regs *regs)
> >  
> >  asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
> >  {
> > +    early_printk("Data Abort at %"PRIvaddr" DFAR
%"PRIx32"\n",
> > +                 regs->pc, READ_CP32(DFAR));
> 
> early_printk belongs to __init and will be discarded by
> free_init_memory. Any data abort after this call will result to a
> "double" hang.
Yeah, this was some debug code of mine which snuck in, not supposed to
be here at all!
> >  
> >  void do_unexpected_trap(const char *msg, struct cpu_user_regs *regs)
> >  {
> > -    printk("CPU%d: Unexpected Trap: %s\n",
smp_processor_id(), msg);
> > +    early_printk("CPU%d: Unexpected Trap: %s\n",
smp_processor_id(), msg);
> 
> Same issue as do data_abort_trap
Yup!

Ian Campbell

2013-Sep-12 13:54 UTC

head link

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

On Thu, 2013-09-12 at 14:25 +0100, Jan Beulich wrote:> >>> On 12.09.13 at 14:42, Ian Campbell
<ian.campbell@citrix.com> wrote:
> > Currently the mapping from pages to zones causes the page at zero to
go into
> > zone -1 and the page at 4096 to go into zone 0, which is the Xen zone
> > (confusing various assertions).
> 
> So that''s a problem on ARM only, right? Because x86 avoids passing
> the low first Mb to the allocator. I wonder whether ARM shouldn''t
at
> least avoid making the page at 0 available for allocation too, which
> would address half of the problem. Avoiding MFN 1 would be less
> natural, I agree.
I went back and forth on this for a bit.
> 
> > Arrange instead for the mapping to be such that zone 0 is always
reserved for
> > Xen and all other pages map to a zone >= 1.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: keir@xen.org 
> > ---
> >  xen/common/page_alloc.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> > index 41251b2..e2cd139 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -257,11 +257,11 @@ unsigned long __init alloc_boot_pages(
> >   */
> >  
> >  #define MEMZONE_XEN 0
> > -#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT)
> > +#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT + 1 + 1)
> 
> So we''ve got a total delta of two between old and new handling
> here...
> 
> > -#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 0 : ((b) -
PAGE_SHIFT - 1))
> > +#define bits_to_zone(b) (((b) < PAGE_SHIFT) ? 1 : ((b) -
PAGE_SHIFT + 1 + 1))
> 
> ... but a delta of 3 on the right side of the colon here (the delta being
> just one on the left side is the actual meat of your fix as I understand
> it). Or, taking a slightly different perspective, using < and <= in
the
> condition should not alter the result (but does if I''m not
mistaken).
Hrm, you are right. I had a debug patch which printed out the
bits_to_zone and page_to_zone values for a bunch of supposed-to-be
consistent values, and they looked correct.

I can check again thouse, since something is clearly not quite hanging
together.> 
> >  #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
> > -                          (fls(page_to_mfn(pg)) - 1))
> > +                          (fls(page_to_mfn(pg)) + 1))
> 
> And the total delta is 2 here again.
> 
> Overall I''m really uncertain whether it wouldn''t be
better for ARM to
> play by the x86 rules in this respect,
Leaving page 0 free would make this a bit less confusing I suspect, by
removing the "off by two".
>  or alternatively to further
> generalize what you try to do here by allowing x86 to specify a bias
> for the shift to skip all zones currently covering the low Mb, which on
> ARM would end up being 1.
That could be a good approach.

Ian.

Ian Campbell

2013-Sep-12 13:56 UTC

head link

Re: [PATCH 3/7] xen/arm: Reserve FDT via early module mechanism

On Thu, 2013-09-12 at 14:39 +0100, Julien Grall wrote:> On 09/12/2013 01:42 PM, Ian Campbell wrote:
> > This will stop us putting any heaps or relocating Xen itself over the
FDT.
> > 
> > The devicetree will be copied to allocated memory in setup_mm and the
original
> > copy will be freed by discard_initial_modules.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/setup.c          |    3 ++-
> >  xen/common/device_tree.c      |    9 ++++++++-
> >  xen/include/xen/device_tree.h |   11 ++++++-----
> >  3 files changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 1ba2eb3..ab3d9aa 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -510,9 +510,10 @@ void __init start_xen(unsigned long
boot_phys_offset,
> >  
> >      smp_clear_cpu_maps();
> >  
> > +    /* This is mapped by head.S */
> >      device_tree_flattened = (void *)BOOT_MISC_VIRT_START
> >          + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
> > -    fdt_size = device_tree_early_init(device_tree_flattened);
> > +    fdt_size = device_tree_early_init(device_tree_flattened,
fdt_paddr);
> >  
> >      cpus = smp_get_max_cpus();
> >      cmdline_parse(device_tree_bootargs(device_tree_flattened));
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index c4f0f2c..9e0c224 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -510,14 +510,21 @@ static void __init early_print_info(void)
> >   *
> >   * Returns the size of the DTB.
> >   */
> > -size_t __init device_tree_early_init(const void *fdt)
> > +size_t __init device_tree_early_init(const void *fdt, paddr_t paddr)
> >  {
> > +    struct dt_mb_module *mod;
> >      int ret;
> >  
> >      ret = fdt_check_header(fdt);
> >      if ( ret < 0 )
> >          early_panic("No valid device tree\n");
> >  
> > +    mod = &early_info.modules.module[MOD_FDT];
> > +    mod->start = paddr & PAGE_MASK;
> > +    mod->size = (fdt_totalsize(fdt) + ~PAGE_MASK) & PAGE_MASK;
> 
> I''m not sure, s/~PAGE_MASK/PAGE_SIZE ?
~PAGE_MASK == 0xfff while PAGE_SIZE == 0x1000. PAGE_SIZE - 1 maybe, but
what''s there works, unless I''ve gotten myself confused
somewhere.

Ian.

Ian Campbell

2013-Sep-13 11:20 UTC

head link

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

On Thu, 2013-09-12 at 14:25 +0100, Jan Beulich wrote:> >>> On 12.09.13 at 14:42, Ian Campbell
<ian.campbell@citrix.com> wrote:
> > Currently the mapping from pages to zones causes the page at zero to
go into
> > zone -1 and the page at 4096 to go into zone 0, which is the Xen zone
> > (confusing various assertions).
> 
> So that''s a problem on ARM only, right? Because x86 avoids passing
> the low first Mb to the allocator. I wonder whether ARM shouldn''t
at
> least avoid making the page at 0 available for allocation too, which
> would address half of the problem. Avoiding MFN 1 would be less
> natural, I agree.
[...]> Overall I''m really uncertain whether it wouldn''t be
better for ARM to
> play by the x86 rules in this respect, or alternatively to further
> generalize what you try to do here by allowing x86 to specify a bias
> for the shift to skip all zones currently covering the low Mb, which on
> ARM would end up being 1.
Actually, if I don''t make a mess of my arithmetic then I don''t
think
this is needed, at least not for correctness.

page_to_zone() is still wrong for page 0, but that was true with the
previous version too, hence the checks to avoid adding page 0 to any
heap.

The difference is that it now ends up in zone 0 (Xen, bad) instead of
zone -1 (even worse!). Even that could be solved with this extra hunk
(which would mean we could drop all the checks in init_*_pages from
below too):

@@ -268,7 +267,7 @@ unsigned long __init alloc_boot_pages(
 
 #define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 1 : ((b) - PAGE_SHIFT))
 #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
-                          (fls(page_to_mfn(pg))))
+                          (fls(page_to_mfn(pg)) ? : 1))
 
 typedef struct page_list_head heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1];
 static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];

What do you think?

8<--------------------------------------------------------

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 41251b2..0e3055c 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -159,6 +160,8 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
 
     ps = round_pgup(ps);
     pe = round_pgdown(pe);
+    if ( ps < PAGE_SIZE )
+        ps = PAGE_SIZE; /* Always leave page 0 free */
     if ( pe <= ps )
         return;
 
@@ -257,11 +263,11 @@ unsigned long __init alloc_boot_pages(
  */
 
 #define MEMZONE_XEN 0
-#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT)
+#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT + 1)
 
-#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 0 : ((b) - PAGE_SHIFT -
1))
+#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 1 : ((b) - PAGE_SHIFT))
 #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
-                          (fls(page_to_mfn(pg)) - 1))
+                          (fls(page_to_mfn(pg))))
 
 typedef struct page_list_head heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1];
 static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];
@@ -1311,6 +1332,8 @@ void init_xenheap_pages(paddr_t ps, paddr_t pe)
 {
     ps = round_pgup(ps);
     pe = round_pgdown(pe);
+    if ( ps < PAGE_SIZE )
+        ps = PAGE_SIZE; /* Always leave page 0 free */
     if ( pe <= ps )
         return;
 
@@ -1429,6 +1451,8 @@ void init_domheap_pages(paddr_t ps, paddr_t pe)
     smfn = round_pgup(ps) >> PAGE_SHIFT;
     emfn = round_pgdown(pe) >> PAGE_SHIFT;
 
+    if ( smfn == 0 )
+        smfn = PAGE_SIZE; /* Always leave page 0 free */
     init_heap_pages(mfn_to_page(smfn), emfn - smfn);
 }

Keir Fraser

2013-Sep-13 11:28 UTC

head link

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

On 13/09/2013 04:20, "Ian Campbell" <Ian.Campbell@citrix.com>
wrote:
> Actually, if I don''t make a mess of my arithmetic then I
don''t think
> this is needed, at least not for correctness.
> 
> page_to_zone() is still wrong for page 0, but that was true with the
> previous version too, hence the checks to avoid adding page 0 to any
> heap.
> 
> The difference is that it now ends up in zone 0 (Xen, bad) instead of
> zone -1 (even worse!). Even that could be solved with this extra hunk
> (which would mean we could drop all the checks in init_*_pages from
> below too):
> 
> @@ -268,7 +267,7 @@ unsigned long __init alloc_boot_pages(
>  
>  #define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 1 : ((b) -
PAGE_SHIFT))
>  #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
> -                          (fls(page_to_mfn(pg))))
> +                          (fls(page_to_mfn(pg)) ? : 1))
>  
>  typedef struct page_list_head
> heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1];
>  static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];
> 
> What do you think?
Yeah, looks good!

 -- Keir

Xen devel - Sep 2013 - [PATCH 0/7] xen: arm: memory mangement fixes / improvements

[PATCH 0/7] xen: arm: memory mangement fixes / improvements

[PATCH 1/7] xen/arm: ensure the xenheap is 32MB aligned

[PATCH 2/7] xen/arm: DOMHEAP_SECOND_PAGES is arm32 specific

[PATCH 3/7] xen/arm: Reserve FDT via early module mechanism

[PATCH 4/7] xen/arm: cope with modules outside of "visible" RAM

[PATCH 5/7] xen: support RAM at addresses 0 and 4096

[PATCH 6/7] xen/arm: Support dtb /memreserve/ regions

[PATCH 7/7] xen/arm: rename boot misc region to boot reloc now it has a single purpose

Re: [PATCH 6/7] xen/arm: Support dtb /memreserve/ regions

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

Re: [PATCH 3/7] xen/arm: Reserve FDT via early module mechanism

Re: [PATCH 6/7] xen/arm: Support dtb /memreserve/ regions

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

Re: [PATCH 3/7] xen/arm: Reserve FDT via early module mechanism

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096

Re: [PATCH 5/7] xen: support RAM at addresses 0 and 4096