* [PATCH 01/16] arm: Zero the BSS at start of day.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 9:56 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 02/16] Create a raw binary target Ian Campbell
` (16 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Avoids surprises e.g. when loading via the boot-wrapper.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/head.S | 20 +++++++++++++++++++-
xen/arch/arm/xen.lds.S | 1 +
2 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
index cdbe011..131cdf9 100644
--- a/xen/arch/arm/head.S
+++ b/xen/arch/arm/head.S
@@ -127,8 +127,26 @@ boot_cpu:
add pc, r0, r10 /* Call PA of function */
hyp:
- PRINT("- Setting up control registers -\r\n")
+ /* Zero BSS On the boot CPU to avoid nasty surprises */
+ teq r12, #0
+ bne skip_bss
+
+ PRINT("- Zero BSS -\r\n")
+ ldr r0, =__bss_start /* Load start & end of bss */
+ ldr r1, =__bss_end
+ add r0, r0, r10 /* Apply physical offset */
+ add r1, r1, r10
+
+ mov r2, #0
+1: str r2, [r0], #4
+ cmp r0, r1
+ blo 1b
+
+skip_bss:
+
+ PRINT("- Setting up control registers -\r\n")
+
/* Set up memory attribute type tables */
ldr r0, =MAIR0VAL
ldr r1, =MAIR1VAL
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 4a9d086..410d7db 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -119,6 +119,7 @@ SECTIONS
*(.bss.percpu.read_mostly)
. = ALIGN(SMP_CACHE_BYTES);
__per_cpu_data_end = .;
+ __bss_end = .;
} :text
_end = . ;
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 02/16] Create a raw binary target.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
2012-09-03 13:30 ` [PATCH 01/16] arm: Zero the BSS at start of day Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 10:01 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 03/16] arm: make virtual address defines unsigned Ian Campbell
` (15 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
This is suitable for direct loading by a bootloader.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/Makefile | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index bfac017..f296c2f 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -37,12 +37,16 @@ endif
ALL_OBJS := head.o $(ALL_OBJS)
-$(TARGET): $(TARGET)-syms
+$(TARGET): $(TARGET)-syms $(TARGET).bin
# XXX: VE model loads by VMA so instead of
# making a proper ELF we link with LMA == VMA and adjust crudely
$(OBJCOPY) --change-addresses +0x80000000 $< $@
$(STRIP) $@
+#
+$(TARGET).bin: $(TARGET)-syms
+ objcopy -O binary -R .comment -S $< $@
+
#$(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
# ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x100000 \
# `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 03/16] arm: make virtual address defines unsigned
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
2012-09-03 13:30 ` [PATCH 01/16] arm: Zero the BSS at start of day Ian Campbell
2012-09-03 13:30 ` [PATCH 02/16] Create a raw binary target Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 10:02 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 04/16] arm: handle xenheap which isn't at the start of RAM Ian Campbell
` (14 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
avoids confusion due to overflow etc.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/include/asm-arm/config.h | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 7d02cc7..2a05539 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -69,14 +69,14 @@
* - in setup_pagetables() when relocating Xen.
*/
-#define XEN_VIRT_START 0x00200000
-#define FIXMAP_ADDR(n) (0x00400000 + (n) * PAGE_SIZE)
-#define BOOT_MISC_VIRT_START 0x00600000
-#define FRAMETABLE_VIRT_START 0x02000000
-#define XENHEAP_VIRT_START 0x40000000
-#define DOMHEAP_VIRT_START 0x80000000
-
-#define HYPERVISOR_VIRT_START mk_unsigned_long(XEN_VIRT_START)
+#define XEN_VIRT_START mk_unsigned_long(0x00200000)
+#define FIXMAP_ADDR(n) (mk_unsigned_long(0x00400000) + (n) * PAGE_SIZE)
+#define BOOT_MISC_VIRT_START mk_unsigned_long(0x00600000)
+#define FRAMETABLE_VIRT_START mk_unsigned_long(0x02000000)
+#define XENHEAP_VIRT_START mk_unsigned_long(0x40000000)
+#define DOMHEAP_VIRT_START mk_unsigned_long(0x80000000)
+
+#define HYPERVISOR_VIRT_START XEN_VIRT_START
#define DOMHEAP_ENTRIES 1024 /* 1024 2MB mapping slots */
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 04/16] arm: handle xenheap which isn't at the start of RAM.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (2 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 03/16] arm: make virtual address defines unsigned Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 11:36 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 05/16] arm: move get_paddr_function to arch setup.c from device_tree.c Ian Campbell
` (13 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Also refactor page_to_virt somewhat in an attempt to make it clearer
what is happening.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/include/asm-arm/mm.h | 24 +++++++++++++++++++-----
1 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index b37bd35..6498322 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -214,17 +214,31 @@ static inline struct page_info *virt_to_page(const void *v)
ASSERT(va >= XENHEAP_VIRT_START);
ASSERT(va < xenheap_virt_end);
- return frame_table + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT);
+ return frame_table
+ + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT)
+ + xenheap_mfn_start
+ - frametable_base_mfn;
}
static inline void *page_to_virt(const struct page_info *pg)
{
+ unsigned long va;
+ const unsigned long offset =
+ (xenheap_mfn_start-frametable_base_mfn)*sizeof(*pg);
+
+ /*
+ * Dividing by this on both top and bottom factors out the largest
+ * common factor of 2 which helps the compiler to use smaller shifts.
+ */
+ const unsigned long lcd = (sizeof(*pg) & -sizeof(*pg));
+
ASSERT((unsigned long)pg - FRAMETABLE_VIRT_START < frametable_virt_end);
- return (void *)(XENHEAP_VIRT_START +
- ((unsigned long)pg - FRAMETABLE_VIRT_START) /
- (sizeof(*pg) / (sizeof(*pg) & -sizeof(*pg))) *
- (PAGE_SIZE / (sizeof(*pg) & -sizeof(*pg))));
+ va = (unsigned long)pg;
+ va = XENHEAP_VIRT_START +
+ ((va - FRAMETABLE_VIRT_START - offset) / (sizeof(*pg) / lcd)) *
+ (PAGE_SIZE / lcd);
+ return (void *)va;
}
struct domain *page_get_owner_and_reference(struct page_info *page);
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 04/16] arm: handle xenheap which isn't at the start of RAM.
2012-09-03 13:30 ` [PATCH 04/16] arm: handle xenheap which isn't at the start of RAM Ian Campbell
@ 2012-09-06 11:36 ` Tim Deegan
0 siblings, 0 replies; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 11:36 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679044), Ian Campbell wrote:
> Also refactor page_to_virt somewhat in an attempt to make it clearer
> what is happening.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/include/asm-arm/mm.h | 24 +++++++++++++++++++-----
> 1 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index b37bd35..6498322 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -214,17 +214,31 @@ static inline struct page_info *virt_to_page(const void *v)
> ASSERT(va >= XENHEAP_VIRT_START);
> ASSERT(va < xenheap_virt_end);
>
> - return frame_table + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT);
> + return frame_table
> + + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT)
> + + xenheap_mfn_start
> + - frametable_base_mfn;
> }
>
> static inline void *page_to_virt(const struct page_info *pg)
> {
> + unsigned long va;
> + const unsigned long offset =
> + (xenheap_mfn_start-frametable_base_mfn)*sizeof(*pg);
> +
> + /*
> + * Dividing by this on both top and bottom factors out the largest
> + * common factor of 2 which helps the compiler to use smaller shifts.
> + */
> + const unsigned long lcd = (sizeof(*pg) & -sizeof(*pg));
> +
> ASSERT((unsigned long)pg - FRAMETABLE_VIRT_START < frametable_virt_end);
> - return (void *)(XENHEAP_VIRT_START +
> - ((unsigned long)pg - FRAMETABLE_VIRT_START) /
> - (sizeof(*pg) / (sizeof(*pg) & -sizeof(*pg))) *
> - (PAGE_SIZE / (sizeof(*pg) & -sizeof(*pg))));
>
> + va = (unsigned long)pg;
> + va = XENHEAP_VIRT_START +
> + ((va - FRAMETABLE_VIRT_START - offset) / (sizeof(*pg) / lcd)) *
> + (PAGE_SIZE / lcd);
> + return (void *)va;
This function is getting a bit too confusing. I think we should just
use page_to_mfn and mfn_to_virt instead: the magic lcd trick eliminates
a shift but having to construct "offset" to match it adds one.
Tim.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 05/16] arm: move get_paddr_function to arch setup.c from device_tree.c
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (3 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 04/16] arm: handle xenheap which isn't at the start of RAM Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 11:40 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 06/16] arm: parse modules from DT during early boot Ian Campbell
` (12 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
It's not realy got any DT functionality in it and its only caller is
setup_pagetables.
Put it here because future patches want to incorporate of the module
layout in memory and I'd like to confine that to setup.c
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/mm.c | 5 +----
xen/arch/arm/setup.c | 35 ++++++++++++++++++++++++++++++++++-
xen/common/device_tree.c | 32 --------------------------------
xen/include/asm-arm/mm.h | 2 +-
xen/include/xen/device_tree.h | 1 -
5 files changed, 36 insertions(+), 39 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 08bc55b..52bb5c7 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -206,15 +206,12 @@ void unmap_domain_page(const void *va)
/* Boot-time pagetable setup.
* Changes here may need matching changes in head.S */
-void __init setup_pagetables(unsigned long boot_phys_offset)
+void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
{
- paddr_t xen_paddr;
unsigned long dest_va;
lpae_t pte, *p;
int i;
- xen_paddr = device_tree_get_xen_paddr();
-
/* Map the destination in the boot misc area. */
dest_va = BOOT_MISC_VIRT_START;
pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index c4ca270..b466875 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -37,6 +37,7 @@
#include <asm/current.h>
#include <asm/setup.h>
#include <asm/vfp.h>
+#include <asm/early_printk.h>
#include "gic.h"
static __attribute_used__ void init_done(void)
@@ -66,6 +67,38 @@ static void __init processor_id(void)
READ_CP32(ID_ISAR3), READ_CP32(ID_ISAR4), READ_CP32(ID_ISAR5));
}
+/**
+ * get_xen_paddr - get physical address to relocate Xen to
+ *
+ * Xen is relocated to the top of RAM and aligned to a XEN_PADDR_ALIGN
+ * boundary.
+ */
+static paddr_t __init get_xen_paddr(void)
+{
+ struct dt_mem_info *mi = &early_info.mem;
+ paddr_t min_size;
+ paddr_t paddr = 0, t;
+ int i;
+
+ min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
+
+ /* Find the highest bank with enough space. */
+ for ( i = 0; i < mi->nr_banks; i++ )
+ {
+ if ( mi->bank[i].size >= min_size )
+ {
+ t = mi->bank[i].start + mi->bank[i].size - min_size;
+ if ( t > paddr )
+ paddr = t;
+ }
+ }
+
+ if ( !paddr )
+ early_panic("Not enough memory to relocate Xen\n");
+
+ return paddr;
+}
+
static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
{
paddr_t ram_start;
@@ -155,7 +188,7 @@ void __init start_xen(unsigned long boot_phys_offset,
cmdline_parse(device_tree_bootargs(fdt));
- setup_pagetables(boot_phys_offset);
+ setup_pagetables(boot_phys_offset, get_xen_paddr());
#ifdef EARLY_UART_ADDRESS
/* Map the UART */
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 04619f4..3d1f0f4 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -273,38 +273,6 @@ size_t __init device_tree_early_init(const void *fdt)
return fdt_totalsize(fdt);
}
-/**
- * device_tree_get_xen_paddr - get physical address to relocate Xen to
- *
- * Xen is relocated to the top of RAM and aligned to a XEN_PADDR_ALIGN
- * boundary.
- */
-paddr_t __init device_tree_get_xen_paddr(void)
-{
- struct dt_mem_info *mi = &early_info.mem;
- paddr_t min_size;
- paddr_t paddr = 0, t;
- int i;
-
- min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
-
- /* Find the highest bank with enough space. */
- for ( i = 0; i < mi->nr_banks; i++ )
- {
- if ( mi->bank[i].size >= min_size )
- {
- t = mi->bank[i].start + mi->bank[i].size - min_size;
- if ( t > paddr )
- paddr = t;
- }
- }
-
- if ( !paddr )
- early_panic("Not enough memory to relocate Xen\n");
-
- return paddr;
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 6498322..b0e5a67 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -138,7 +138,7 @@ extern unsigned long max_page;
extern unsigned long total_pages;
/* Boot-time pagetable setup */
-extern void setup_pagetables(unsigned long boot_phys_offset);
+extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr);
/* MMU setup for seccondary CPUS (which already have paging enabled) */
extern void __cpuinit mmu_init_secondary_cpu(void);
/* Set up the xenheap: up to 1GB of contiguous, always-mapped memory.
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 8e1626c..4d010c0 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -39,7 +39,6 @@ extern struct dt_early_info early_info;
extern void *device_tree_flattened;
size_t device_tree_early_init(const void *fdt);
-paddr_t device_tree_get_xen_paddr(void);
void device_tree_get_reg(const u32 **cell, u32 address_cells, u32 size_cells,
u64 *start, u64 *size);
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 06/16] arm: parse modules from DT during early boot.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (4 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 05/16] arm: move get_paddr_function to arch setup.c from device_tree.c Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 11:47 ` Tim Deegan
2012-11-30 14:58 ` Stefano Stabellini
2012-09-03 13:30 ` [PATCH 07/16] arm: avoid placing Xen over any modules Ian Campbell
` (11 subsequent siblings)
17 siblings, 2 replies; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
The bootloader should populate /chosen/nr-modules with the number of
modules and then for each module 0..nr-modules-1 it should populate
/chosen/moduleN-{start,end} with the physical address of the module.
The hypervisor allows for 2 modules (kernel and initrd). Currently we
don't do anything with them.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/common/device_tree.c | 57 +++++++++++++++++++++++++++++++++++++++++
xen/include/xen/device_tree.h | 7 +++++
2 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 3d1f0f4..226e3f3 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -229,6 +229,61 @@ static void __init process_memory_node(const void *fdt, int node,
}
}
+static void __init process_chosen_node(const void *fdt, int node,
+ const char *name,
+ u32 address_cells, u32 size_cells)
+{
+ const struct fdt_property *prop;
+ const u32 *cell;
+ paddr_t size;
+ int i, nr_modules;
+
+ prop = fdt_get_property(fdt, node, "nr-modules", NULL);
+ if ( !prop )
+ {
+ early_info.modules.nr_mods = 0;
+ return;
+ }
+
+ cell = (const u32 *)prop->data;
+ get_val(&cell, 1, (u64*)&nr_modules);
+
+ if ( nr_modules > NR_MODULES )
+ {
+ early_panic("too many modules %d > %d\n", nr_modules, NR_MODULES);
+ }
+
+ for ( i = 0; i < nr_modules; i++ )
+ {
+ struct membank *mod = &early_info.modules.module[i];
+ /* longest prop name is "start", single digit numbers of modules */
+ char propname[strlen("moduleX-start") + 1];
+
+ BUILD_BUG_ON(NR_MODULES > 9);
+
+ snprintf(propname, sizeof(propname), "module%d-start", i+1);
+ prop = fdt_get_property(fdt, node, propname, NULL);
+ if ( !prop )
+ early_panic("no start for module %d\n", i);
+
+ cell = (const u32 *)prop->data;
+ device_tree_get_reg(&cell, address_cells, size_cells,
+ &mod->start, &size);
+
+ snprintf(propname, sizeof(propname), "module%d-end", i+1);
+ prop = fdt_get_property(fdt, node, propname, NULL);
+ if ( !prop )
+ early_panic("no end for module %d\n", i);
+
+ cell = (const u32 *)prop->data;
+ device_tree_get_reg(&cell, address_cells, size_cells,
+ &mod->size, &size);
+ mod->size -= mod->start;
+ }
+
+ early_info.modules.nr_mods = nr_modules;
+}
+
static int __init early_scan_node(const void *fdt,
int node, const char *name, int depth,
u32 address_cells, u32 size_cells,
@@ -236,6 +291,8 @@ static int __init early_scan_node(const void *fdt,
{
if ( device_tree_node_matches(fdt, node, "memory") )
process_memory_node(fdt, node, name, address_cells, size_cells);
+ else if ( device_tree_node_matches(fdt, node, "chosen") )
+ process_chosen_node(fdt, node, name, address_cells, size_cells);
return 0;
}
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 4d010c0..585fcc9 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -15,6 +15,7 @@
#define DEVICE_TREE_MAX_DEPTH 16
#define NR_MEM_BANKS 8
+#define NR_MODULES 2
struct membank {
paddr_t start;
@@ -26,8 +27,14 @@ struct dt_mem_info {
struct membank bank[NR_MEM_BANKS];
};
+struct dt_module_info {
+ int nr_mods;
+ struct membank module[NR_MODULES];
+};
+
struct dt_early_info {
struct dt_mem_info mem;
+ struct dt_module_info modules;
};
typedef int (*device_tree_node_func)(const void *fdt,
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 06/16] arm: parse modules from DT during early boot.
2012-09-03 13:30 ` [PATCH 06/16] arm: parse modules from DT during early boot Ian Campbell
@ 2012-09-06 11:47 ` Tim Deegan
2012-09-06 11:53 ` Ian Campbell
2012-11-30 14:58 ` Stefano Stabellini
1 sibling, 1 reply; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 11:47 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679046), Ian Campbell wrote:
> The bootloader should populate /chosen/nr-modules with the number of
> modules and then for each module 0..nr-modules-1 it should populate
> /chosen/moduleN-{start,end} with the physical address of the module.
The code below looks like it's expecting the modules to be numbered from
1, not from 0.
Tim.
> The hypervisor allows for 2 modules (kernel and initrd). Currently we
> don't do anything with them.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/common/device_tree.c | 57 +++++++++++++++++++++++++++++++++++++++++
> xen/include/xen/device_tree.h | 7 +++++
> 2 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 3d1f0f4..226e3f3 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -229,6 +229,61 @@ static void __init process_memory_node(const void *fdt, int node,
> }
> }
>
> +static void __init process_chosen_node(const void *fdt, int node,
> + const char *name,
> + u32 address_cells, u32 size_cells)
> +{
> + const struct fdt_property *prop;
> + const u32 *cell;
> + paddr_t size;
> + int i, nr_modules;
> +
> + prop = fdt_get_property(fdt, node, "nr-modules", NULL);
> + if ( !prop )
> + {
> + early_info.modules.nr_mods = 0;
> + return;
> + }
> +
> + cell = (const u32 *)prop->data;
> + get_val(&cell, 1, (u64*)&nr_modules);
> +
> + if ( nr_modules > NR_MODULES )
> + {
> + early_panic("too many modules %d > %d\n", nr_modules, NR_MODULES);
> + }
> +
> + for ( i = 0; i < nr_modules; i++ )
> + {
> + struct membank *mod = &early_info.modules.module[i];
> + /* longest prop name is "start", single digit numbers of modules */
> + char propname[strlen("moduleX-start") + 1];
> +
> + BUILD_BUG_ON(NR_MODULES > 9);
> +
> + snprintf(propname, sizeof(propname), "module%d-start", i+1);
> + prop = fdt_get_property(fdt, node, propname, NULL);
> + if ( !prop )
> + early_panic("no start for module %d\n", i);
> +
> + cell = (const u32 *)prop->data;
> + device_tree_get_reg(&cell, address_cells, size_cells,
> + &mod->start, &size);
> +
> + snprintf(propname, sizeof(propname), "module%d-end", i+1);
> + prop = fdt_get_property(fdt, node, propname, NULL);
> + if ( !prop )
> + early_panic("no end for module %d\n", i);
> +
> + cell = (const u32 *)prop->data;
> + device_tree_get_reg(&cell, address_cells, size_cells,
> + &mod->size, &size);
> + mod->size -= mod->start;
> + }
> +
> + early_info.modules.nr_mods = nr_modules;
> +}
> +
> static int __init early_scan_node(const void *fdt,
> int node, const char *name, int depth,
> u32 address_cells, u32 size_cells,
> @@ -236,6 +291,8 @@ static int __init early_scan_node(const void *fdt,
> {
> if ( device_tree_node_matches(fdt, node, "memory") )
> process_memory_node(fdt, node, name, address_cells, size_cells);
> + else if ( device_tree_node_matches(fdt, node, "chosen") )
> + process_chosen_node(fdt, node, name, address_cells, size_cells);
>
> return 0;
> }
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 4d010c0..585fcc9 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -15,6 +15,7 @@
> #define DEVICE_TREE_MAX_DEPTH 16
>
> #define NR_MEM_BANKS 8
> +#define NR_MODULES 2
>
> struct membank {
> paddr_t start;
> @@ -26,8 +27,14 @@ struct dt_mem_info {
> struct membank bank[NR_MEM_BANKS];
> };
>
> +struct dt_module_info {
> + int nr_mods;
> + struct membank module[NR_MODULES];
> +};
> +
> struct dt_early_info {
> struct dt_mem_info mem;
> + struct dt_module_info modules;
> };
>
> typedef int (*device_tree_node_func)(const void *fdt,
> --
> 1.7.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 06/16] arm: parse modules from DT during early boot.
2012-09-06 11:47 ` Tim Deegan
@ 2012-09-06 11:53 ` Ian Campbell
0 siblings, 0 replies; 47+ messages in thread
From: Ian Campbell @ 2012-09-06 11:53 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xen.org
On Thu, 2012-09-06 at 12:47 +0100, Tim Deegan wrote:
> At 13:30 +0000 on 03 Sep (1346679046), Ian Campbell wrote:
> > The bootloader should populate /chosen/nr-modules with the number of
> > modules and then for each module 0..nr-modules-1 it should populate
> > /chosen/moduleN-{start,end} with the physical address of the module.
>
> The code below looks like it's expecting the modules to be numbered from
> 1, not from 0.
Yes, looks like I'd forgotten that by the time I wrote the commit
message. Thanks.
Ian.
>
> Tim.
>
> > The hypervisor allows for 2 modules (kernel and initrd). Currently we
> > don't do anything with them.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > xen/common/device_tree.c | 57 +++++++++++++++++++++++++++++++++++++++++
> > xen/include/xen/device_tree.h | 7 +++++
> > 2 files changed, 64 insertions(+), 0 deletions(-)
> >
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index 3d1f0f4..226e3f3 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -229,6 +229,61 @@ static void __init process_memory_node(const void *fdt, int node,
> > }
> > }
> >
> > +static void __init process_chosen_node(const void *fdt, int node,
> > + const char *name,
> > + u32 address_cells, u32 size_cells)
> > +{
> > + const struct fdt_property *prop;
> > + const u32 *cell;
> > + paddr_t size;
> > + int i, nr_modules;
> > +
> > + prop = fdt_get_property(fdt, node, "nr-modules", NULL);
> > + if ( !prop )
> > + {
> > + early_info.modules.nr_mods = 0;
> > + return;
> > + }
> > +
> > + cell = (const u32 *)prop->data;
> > + get_val(&cell, 1, (u64*)&nr_modules);
> > +
> > + if ( nr_modules > NR_MODULES )
> > + {
> > + early_panic("too many modules %d > %d\n", nr_modules, NR_MODULES);
> > + }
> > +
> > + for ( i = 0; i < nr_modules; i++ )
> > + {
> > + struct membank *mod = &early_info.modules.module[i];
> > + /* longest prop name is "start", single digit numbers of modules */
> > + char propname[strlen("moduleX-start") + 1];
> > +
> > + BUILD_BUG_ON(NR_MODULES > 9);
> > +
> > + snprintf(propname, sizeof(propname), "module%d-start", i+1);
> > + prop = fdt_get_property(fdt, node, propname, NULL);
> > + if ( !prop )
> > + early_panic("no start for module %d\n", i);
> > +
> > + cell = (const u32 *)prop->data;
> > + device_tree_get_reg(&cell, address_cells, size_cells,
> > + &mod->start, &size);
> > +
> > + snprintf(propname, sizeof(propname), "module%d-end", i+1);
> > + prop = fdt_get_property(fdt, node, propname, NULL);
> > + if ( !prop )
> > + early_panic("no end for module %d\n", i);
> > +
> > + cell = (const u32 *)prop->data;
> > + device_tree_get_reg(&cell, address_cells, size_cells,
> > + &mod->size, &size);
> > + mod->size -= mod->start;
> > + }
> > +
> > + early_info.modules.nr_mods = nr_modules;
> > +}
> > +
> > static int __init early_scan_node(const void *fdt,
> > int node, const char *name, int depth,
> > u32 address_cells, u32 size_cells,
> > @@ -236,6 +291,8 @@ static int __init early_scan_node(const void *fdt,
> > {
> > if ( device_tree_node_matches(fdt, node, "memory") )
> > process_memory_node(fdt, node, name, address_cells, size_cells);
> > + else if ( device_tree_node_matches(fdt, node, "chosen") )
> > + process_chosen_node(fdt, node, name, address_cells, size_cells);
> >
> > return 0;
> > }
> > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> > index 4d010c0..585fcc9 100644
> > --- a/xen/include/xen/device_tree.h
> > +++ b/xen/include/xen/device_tree.h
> > @@ -15,6 +15,7 @@
> > #define DEVICE_TREE_MAX_DEPTH 16
> >
> > #define NR_MEM_BANKS 8
> > +#define NR_MODULES 2
> >
> > struct membank {
> > paddr_t start;
> > @@ -26,8 +27,14 @@ struct dt_mem_info {
> > struct membank bank[NR_MEM_BANKS];
> > };
> >
> > +struct dt_module_info {
> > + int nr_mods;
> > + struct membank module[NR_MODULES];
> > +};
> > +
> > struct dt_early_info {
> > struct dt_mem_info mem;
> > + struct dt_module_info modules;
> > };
> >
> > typedef int (*device_tree_node_func)(const void *fdt,
> > --
> > 1.7.9.1
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 06/16] arm: parse modules from DT during early boot.
2012-09-03 13:30 ` [PATCH 06/16] arm: parse modules from DT during early boot Ian Campbell
2012-09-06 11:47 ` Tim Deegan
@ 2012-11-30 14:58 ` Stefano Stabellini
1 sibling, 0 replies; 47+ messages in thread
From: Stefano Stabellini @ 2012-11-30 14:58 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On Mon, 3 Sep 2012, Ian Campbell wrote:
> The bootloader should populate /chosen/nr-modules with the number of
> modules and then for each module 0..nr-modules-1 it should populate
> /chosen/moduleN-{start,end} with the physical address of the module.
Considering that we can traverse the DT and enumerate all the children
of a particular node, we don't need a nr-modules property.
Also I would rather move all the modules under /chosen/modules than have
/chosen/module-N.
We could even go as far as having one single modules node, with every
module expressed as a reg range in memory.
Then you can document that for Xen the first module should be the linux
kernel and the second module should be the initrd.
chosen {
modules {
reg = < first_address first_size >, <second_address second_size>;
};
};
Besides being more concise and requiring simpler parsing, I think that
it is much more... aesthetic :)
> The hypervisor allows for 2 modules (kernel and initrd). Currently we
> don't do anything with them.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/common/device_tree.c | 57 +++++++++++++++++++++++++++++++++++++++++
> xen/include/xen/device_tree.h | 7 +++++
> 2 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 3d1f0f4..226e3f3 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -229,6 +229,61 @@ static void __init process_memory_node(const void *fdt, int node,
> }
> }
>
> +static void __init process_chosen_node(const void *fdt, int node,
> + const char *name,
> + u32 address_cells, u32 size_cells)
> +{
> + const struct fdt_property *prop;
> + const u32 *cell;
> + paddr_t size;
> + int i, nr_modules;
> +
> + prop = fdt_get_property(fdt, node, "nr-modules", NULL);
> + if ( !prop )
> + {
> + early_info.modules.nr_mods = 0;
> + return;
> + }
> +
> + cell = (const u32 *)prop->data;
> + get_val(&cell, 1, (u64*)&nr_modules);
> +
> + if ( nr_modules > NR_MODULES )
> + {
> + early_panic("too many modules %d > %d\n", nr_modules, NR_MODULES);
> + }
> +
> + for ( i = 0; i < nr_modules; i++ )
> + {
> + struct membank *mod = &early_info.modules.module[i];
> + /* longest prop name is "start", single digit numbers of modules */
> + char propname[strlen("moduleX-start") + 1];
> +
> + BUILD_BUG_ON(NR_MODULES > 9);
> +
> + snprintf(propname, sizeof(propname), "module%d-start", i+1);
> + prop = fdt_get_property(fdt, node, propname, NULL);
> + if ( !prop )
> + early_panic("no start for module %d\n", i);
> +
> + cell = (const u32 *)prop->data;
> + device_tree_get_reg(&cell, address_cells, size_cells,
> + &mod->start, &size);
I don't think you should be using device_tree_get_reg to parse a non reg
property
> + snprintf(propname, sizeof(propname), "module%d-end", i+1);
> + prop = fdt_get_property(fdt, node, propname, NULL);
> + if ( !prop )
> + early_panic("no end for module %d\n", i);
> +
> + cell = (const u32 *)prop->data;
> + device_tree_get_reg(&cell, address_cells, size_cells,
> + &mod->size, &size);
> + mod->size -= mod->start;
> + }
> +
> + early_info.modules.nr_mods = nr_modules;
> +}
> +
> static int __init early_scan_node(const void *fdt,
> int node, const char *name, int depth,
> u32 address_cells, u32 size_cells,
> @@ -236,6 +291,8 @@ static int __init early_scan_node(const void *fdt,
> {
> if ( device_tree_node_matches(fdt, node, "memory") )
> process_memory_node(fdt, node, name, address_cells, size_cells);
> + else if ( device_tree_node_matches(fdt, node, "chosen") )
> + process_chosen_node(fdt, node, name, address_cells, size_cells);
>
> return 0;
> }
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 4d010c0..585fcc9 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -15,6 +15,7 @@
> #define DEVICE_TREE_MAX_DEPTH 16
>
> #define NR_MEM_BANKS 8
> +#define NR_MODULES 2
>
> struct membank {
> paddr_t start;
> @@ -26,8 +27,14 @@ struct dt_mem_info {
> struct membank bank[NR_MEM_BANKS];
> };
>
> +struct dt_module_info {
> + int nr_mods;
> + struct membank module[NR_MODULES];
> +};
> +
> struct dt_early_info {
> struct dt_mem_info mem;
> + struct dt_module_info modules;
> };
>
> typedef int (*device_tree_node_func)(const void *fdt,
> --
> 1.7.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 07/16] arm: avoid placing Xen over any modules.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (5 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 06/16] arm: parse modules from DT during early boot Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 12:01 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 08/16] arm: really allocate boot frametable pages with 32M alignment Ian Campbell
` (10 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
This will still fail if the modules are such that Xen is pushed out of
the top 32M of RAM since it will then overlap with the domheap (or
possibly xenheap). This will be dealt with later.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/setup.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index b466875..5f8a3d7 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -67,17 +67,58 @@ static void __init processor_id(void)
READ_CP32(ID_ISAR3), READ_CP32(ID_ISAR4), READ_CP32(ID_ISAR5));
}
+/*
+ * Returns the end address of the highest region in the range s..e
+ * with required size and alignment that does not conflict with the
+ * modules from first_mod to nr_modules.
+ *
+ * For non-recursive callers first_mod should normally be 0.
+ */
+static paddr_t __init consider_modules(paddr_t s, paddr_t e,
+ uint32_t size, paddr_t align,
+ int first_mod)
+{
+ const struct dt_module_info *mi = &early_info.modules;
+ int i;
+
+ s = (s+align-1) & ~(align-1);
+ e = e & ~(align-1);
+
+ if ( s > e || e - s < size )
+ return 0;
+
+ for ( i = first_mod; i < mi->nr_mods; i++ )
+ {
+ paddr_t mod_s;
+ paddr_t mod_e;
+
+ mod_s = mi->module[i].start;
+ mod_e = mod_s + mi->module[i].size;
+
+ 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;
+}
+
/**
* get_xen_paddr - get physical address to relocate Xen to
*
- * Xen is relocated to the top of RAM and aligned to a XEN_PADDR_ALIGN
- * boundary.
+ * Xen is relocated to as near to the top of RAM as possible and
+ * aligned to a XEN_PADDR_ALIGN boundary.
*/
static paddr_t __init get_xen_paddr(void)
{
struct dt_mem_info *mi = &early_info.mem;
paddr_t min_size;
- paddr_t paddr = 0, t;
+ paddr_t paddr = 0;
int i;
min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
@@ -85,17 +126,28 @@ static paddr_t __init get_xen_paddr(void)
/* Find the highest bank with enough space. */
for ( i = 0; i < mi->nr_banks; i++ )
{
- if ( mi->bank[i].size >= min_size )
+ const struct membank *bank = &mi->bank[i];
+ paddr_t s, e;
+
+ if ( bank->size >= min_size )
{
- t = mi->bank[i].start + mi->bank[i].size - min_size;
- if ( t > paddr )
- paddr = t;
+ e = consider_modules(bank->start, bank->start + bank->size,
+ min_size, XEN_PADDR_ALIGN, 0);
+ if ( !e )continue;
+
+ s = e - min_size;
+
+ if ( s > paddr )
+ paddr = s;
}
}
if ( !paddr )
early_panic("Not enough memory to relocate Xen\n");
+ early_printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
+ paddr, paddr + min_size);
+
return paddr;
}
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 07/16] arm: avoid placing Xen over any modules.
2012-09-03 13:30 ` [PATCH 07/16] arm: avoid placing Xen over any modules Ian Campbell
@ 2012-09-06 12:01 ` Tim Deegan
0 siblings, 0 replies; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 12:01 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679047), Ian Campbell wrote:
> @@ -85,17 +126,28 @@ static paddr_t __init get_xen_paddr(void)
> /* Find the highest bank with enough space. */
> for ( i = 0; i < mi->nr_banks; i++ )
> {
> - if ( mi->bank[i].size >= min_size )
> + const struct membank *bank = &mi->bank[i];
> + paddr_t s, e;
> +
> + if ( bank->size >= min_size )
> {
> - t = mi->bank[i].start + mi->bank[i].size - min_size;
> - if ( t > paddr )
> - paddr = t;
> + e = consider_modules(bank->start, bank->start + bank->size,
> + min_size, XEN_PADDR_ALIGN, 0);
> + if ( !e )continue;
Missing newline. With that fixed,
Acked-by: Tim Deegan <tim@xen.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 08/16] arm: really allocate boot frametable pages with 32M alignment
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (6 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 07/16] arm: avoid placing Xen over any modules Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 12:04 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 09/16] arm: avoid allocating the heaps over modules or xen itself Ian Campbell
` (9 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
This argument to alloc_boot_pages is "pfn_align" and not an order.
We've been lucky until now that the area given to the boot allocator
happened to be properly aligned and this allocation was early enough
to benefit.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/mm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 52bb5c7..aafc48c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -353,7 +353,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
/* Round up to 32M boundary */
frametable_size = (frametable_size + 0x1ffffff) & ~0x1ffffff;
- base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 5);
+ base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
create_mappings(FRAMETABLE_VIRT_START, base_mfn, frametable_size >> PAGE_SHIFT);
memset(&frame_table[0], 0, nr_pages * sizeof(struct page_info));
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 09/16] arm: avoid allocating the heaps over modules or xen itself.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (7 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 08/16] arm: really allocate boot frametable pages with 32M alignment Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 12:08 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 10/16] arm: print a message if multiple banks of memory are present Ian Campbell
` (8 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/setup.c | 119 ++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 105 insertions(+), 14 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 5f8a3d7..369e164 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -40,6 +40,8 @@
#include <asm/early_printk.h>
#include "gic.h"
+static __initdata paddr_t xen_paddr;
+
static __attribute_used__ void init_done(void)
{
free_init_memory();
@@ -72,7 +74,8 @@ static void __init processor_id(void)
* with required size and alignment that does not conflict with the
* modules from first_mod to nr_modules.
*
- * For non-recursive callers first_mod should normally be 0.
+ * For non-recursive callers first_mod should normally be 0 (all
+ * modules) or -1 (all modules and Xen itself).
*/
static paddr_t __init consider_modules(paddr_t s, paddr_t e,
uint32_t size, paddr_t align,
@@ -92,8 +95,17 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
paddr_t mod_s;
paddr_t mod_e;
- mod_s = mi->module[i].start;
- mod_e = mod_s + mi->module[i].size;
+ /* module "-1" is Xen itself. */
+ if ( i == -1 )
+ {
+ mod_s = xen_paddr;
+ mod_e = mod_s + ((_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1));
+ }
+ else
+ {
+ mod_s = mi->module[i].start;
+ mod_e = mod_s + mi->module[i].size;
+ }
if ( s < mod_e && mod_s < e )
{
@@ -108,6 +120,46 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
return e;
}
+/*
+ * Return the end of the non-module region starting at s. In other
+ * words return s the start of the next modules after s.
+ *
+ * Also returns the end of that module in *n.
+ */
+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;
+
+ for ( i = -1; i < mi->nr_mods; i++ )
+ {
+ paddr_t mod_s;
+ paddr_t mod_e;
+
+ /* module "-1" is Xen itself. */
+ if ( i == -1 )
+ {
+ mod_s = xen_paddr;
+ mod_e = mod_s + ((_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1));
+ }
+ else
+ {
+ mod_s = mi->module[i].start;
+ mod_e = mod_s + mi->module[i].size;
+ }
+
+ if ( mod_s < s )
+ continue;
+ if ( mod_s > lowest )
+ continue;
+ lowest = mod_s;
+ *n = mod_e;
+ }
+ return lowest;
+}
+
+
/**
* get_xen_paddr - get physical address to relocate Xen to
*
@@ -156,6 +208,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
paddr_t ram_start;
paddr_t ram_end;
paddr_t ram_size;
+ paddr_t s, e;
unsigned long ram_pages;
unsigned long heap_pages, xenheap_pages, domheap_pages;
unsigned long dtb_pages;
@@ -171,22 +224,37 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
ram_pages = ram_size >> PAGE_SHIFT;
/*
- * Calculate the sizes for the heaps using these constraints:
+ * Locate the xenheap using these constraints:
*
- * - heaps must be 32 MiB aligned
- * - must not include Xen itself
- * - xen heap must be at most 1 GiB
+ * - must be 32 MiB aligned
+ * - must not include Xen itself or the boot modules
+ * - must be at most 1 GiB
+ * - must be at least 128M
*
- * XXX: needs a platform with at least 1GiB of RAM or the dom
- * heap will be empty and no domains can be created.
+ * We try to allocate the largest xenheap possible within these
+ * constraints.
*/
- heap_pages = (ram_size >> PAGE_SHIFT) - (32 << (20 - PAGE_SHIFT));
+ heap_pages = (ram_size >> PAGE_SHIFT);
xenheap_pages = min(1ul << (30 - PAGE_SHIFT), heap_pages);
+
+ do
+ {
+ e = consider_modules(ram_start, ram_end, xenheap_pages<<PAGE_SHIFT,
+ 32<<20, -1);
+ if ( e )
+ break;
+
+ xenheap_pages >>= 1;
+ } while ( xenheap_pages > 128<<(20-PAGE_SHIFT) );
+
+ if ( ! e )
+ panic("Not not enough space for xenheap\n");
+
domheap_pages = heap_pages - xenheap_pages;
printk("Xen heap: %lu pages Dom heap: %lu pages\n", xenheap_pages, domheap_pages);
- setup_xenheap_mappings(ram_start >> PAGE_SHIFT, xenheap_pages);
+ setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
/*
* Need a single mapped page for populating bootmem_region_list
@@ -210,8 +278,30 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
copy_from_paddr(device_tree_flattened, dtb_paddr, dtb_size, BUFFERABLE);
/* Add non-xenheap memory */
- init_boot_pages(pfn_to_paddr(xenheap_mfn_start + xenheap_pages),
- pfn_to_paddr(xenheap_mfn_start + xenheap_pages + domheap_pages));
+ s = ram_start;
+ while ( s < ram_end )
+ {
+ paddr_t n = ram_end;
+
+ e = next_module(s, &n);
+
+ if ( e == ~(paddr_t)0 )
+ {
+ e = n = ram_end;
+ }
+
+ /* Avoid the xenheap */
+ if ( s < ((xenheap_mfn_start+xenheap_pages) << PAGE_SHIFT)
+ && (xenheap_mfn_start << PAGE_SHIFT) < e )
+ {
+ e = pfn_to_paddr(xenheap_mfn_start);
+ n = pfn_to_paddr(xenheap_mfn_start+xenheap_pages);
+ }
+
+ init_boot_pages(s, e);
+
+ s = n;
+ }
setup_frametable_mappings(ram_start, ram_end);
max_page = PFN_DOWN(ram_end);
@@ -240,7 +330,8 @@ void __init start_xen(unsigned long boot_phys_offset,
cmdline_parse(device_tree_bootargs(fdt));
- setup_pagetables(boot_phys_offset, get_xen_paddr());
+ xen_paddr = get_xen_paddr();
+ setup_pagetables(boot_phys_offset, xen_paddr);
#ifdef EARLY_UART_ADDRESS
/* Map the UART */
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 10/16] arm: print a message if multiple banks of memory are present.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (8 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 09/16] arm: avoid allocating the heaps over modules or xen itself Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 12:31 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 11/16] arm: mark heap and frametable limits as read mostly Ian Campbell
` (7 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/setup.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 369e164..85487fe 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -218,6 +218,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
* TODO: only using the first RAM bank for now. The heaps and the
* frame table assume RAM is physically contiguous.
*/
+ if ( early_info.mem.nr_banks > 1 )
+ early_printk("WARNING: Only using first bank of memory\n");
ram_start = early_info.mem.bank[0].start;
ram_size = early_info.mem.bank[0].size;
ram_end = ram_start + ram_size;
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 11/16] arm: mark heap and frametable limits as read mostly
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (9 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 10/16] arm: print a message if multiple banks of memory are present Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 13:29 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 12/16] arm: const-correctness in virt_to_maddr Ian Campbell
` (6 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
These are used in virt_to_page and page_to_virt so I imagine there's
some small benefit to this (but I've not measured)
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/mm.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index aafc48c..368911b 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -47,11 +47,12 @@ uint64_t boot_httbr;
static paddr_t phys_offset;
/* Limits of the Xen heap */
-unsigned long xenheap_mfn_start, xenheap_mfn_end;
-unsigned long xenheap_virt_end;
+unsigned long xenheap_mfn_start __read_mostly;
+unsigned long xenheap_mfn_end __read_mostly;
+unsigned long xenheap_virt_end __read_mostly;
-unsigned long frametable_base_mfn;
-unsigned long frametable_virt_end;
+unsigned long frametable_base_mfn __read_mostly;
+unsigned long frametable_virt_end __read_mostly;
unsigned long max_page;
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 12/16] arm: const-correctness in virt_to_maddr
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (10 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 11/16] arm: mark heap and frametable limits as read mostly Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 13:33 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 13/16] device-tree: get_val cannot cope with cells > 2, add a BUG Ian Campbell
` (5 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/include/asm-arm/mm.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index b0e5a67..7440fe5 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -179,9 +179,9 @@ extern void clear_fixmap(unsigned map);
#define paddr_to_pdx(pa) pfn_to_pdx(paddr_to_pfn(pa))
-static inline paddr_t virt_to_maddr(void *va)
+static inline paddr_t virt_to_maddr(const void *va)
{
- uint64_t par = va_to_par((uint32_t)va);
+ uint64_t par = va_to_par((const uint32_t)va);
return (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
}
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 12/16] arm: const-correctness in virt_to_maddr
2012-09-03 13:30 ` [PATCH 12/16] arm: const-correctness in virt_to_maddr Ian Campbell
@ 2012-09-06 13:33 ` Tim Deegan
0 siblings, 0 replies; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 13:33 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679052), Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/include/asm-arm/mm.h | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index b0e5a67..7440fe5 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -179,9 +179,9 @@ extern void clear_fixmap(unsigned map);
> #define paddr_to_pdx(pa) pfn_to_pdx(paddr_to_pfn(pa))
>
>
> -static inline paddr_t virt_to_maddr(void *va)
> +static inline paddr_t virt_to_maddr(const void *va)
> {
> - uint64_t par = va_to_par((uint32_t)va);
> + uint64_t par = va_to_par((const uint32_t)va);
This second const is unnecessary (or else you also need one in the cast
on the next line). For the first one, though:
Acked-by: Tim Deegan <tim@xen.org>
> return (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
> }
>
> --
> 1.7.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 13/16] device-tree: get_val cannot cope with cells > 2, add a BUG
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (11 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 12/16] arm: const-correctness in virt_to_maddr Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 13:35 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 14/16] arm: load dom0 kernel from first boot module Ian Campbell
` (4 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/common/device_tree.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 226e3f3..60f3a03 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -45,6 +45,8 @@ static void __init get_val(const u32 **cell, u32 cells, u64 *val)
{
*val = 0;
+ BUG_ON( cells > 2 );
+
while ( cells-- )
{
*val <<= 32;
@@ -139,7 +141,7 @@ int device_tree_for_each_node(const void *fdt,
*/
const char *device_tree_bootargs(const void *fdt)
{
- int node;
+ int node;
const struct fdt_property *prop;
node = fdt_path_offset(fdt, "/chosen");
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 13/16] device-tree: get_val cannot cope with cells > 2, add a BUG
2012-09-03 13:30 ` [PATCH 13/16] device-tree: get_val cannot cope with cells > 2, add a BUG Ian Campbell
@ 2012-09-06 13:35 ` Tim Deegan
0 siblings, 0 replies; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 13:35 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679053), Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/common/device_tree.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 226e3f3..60f3a03 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -45,6 +45,8 @@ static void __init get_val(const u32 **cell, u32 cells, u64 *val)
> {
> *val = 0;
>
> + BUG_ON( cells > 2 );
> +
> while ( cells-- )
> {
> *val <<= 32;
> @@ -139,7 +141,7 @@ int device_tree_for_each_node(const void *fdt,
> */
> const char *device_tree_bootargs(const void *fdt)
> {
> - int node;
> + int node;
Unrelated change - maybe in a cset of its own, if that's not too ridiculous?
Tim.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 14/16] arm: load dom0 kernel from first boot module
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (12 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 13/16] device-tree: get_val cannot cope with cells > 2, add a BUG Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 13:44 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 15/16] arm: discard boot modules after building domain 0 Ian Campbell
` (3 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/kernel.c | 63 +++++++++++++++++++++++++++++++++++++-----------
xen/arch/arm/kernel.h | 10 +++++++
2 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 2d56130..d82c3e1 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -65,13 +65,13 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx)
static void kernel_zimage_load(struct kernel_info *info)
{
paddr_t load_addr = info->zimage.load_addr;
+ paddr_t paddr = info->zimage.kernel_addr;
paddr_t len = info->zimage.len;
- paddr_t flash = KERNEL_FLASH_ADDRESS;
void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
unsigned long offs;
- printk("Loading %"PRIpaddr" byte zImage from flash %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr": [",
- len, flash, load_addr, load_addr + len);
+ printk("Loading zImage from %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr": [",
+ paddr, load_addr, load_addr + len);
for ( offs = 0; offs < len; offs += PAGE_SIZE )
{
paddr_t ma = gvirt_to_maddr(load_addr + offs);
@@ -80,7 +80,7 @@ static void kernel_zimage_load(struct kernel_info *info)
if ( ( offs % (1<<20) ) == 0 )
printk(".");
- set_fixmap(FIXMAP_MISC, (flash+offs) >> PAGE_SHIFT, DEV_SHARED);
+ set_fixmap(FIXMAP_MISC, (paddr+offs) >> PAGE_SHIFT, DEV_SHARED);
memcpy(dst, src, PAGE_SIZE);
clear_fixmap(FIXMAP_MISC);
@@ -92,13 +92,16 @@ static void kernel_zimage_load(struct kernel_info *info)
/**
* Check the image is a zImage and return the load address and length
*/
-static int kernel_try_zimage_prepare(struct kernel_info *info)
+static int kernel_try_zimage_prepare(struct kernel_info *info,
+ paddr_t addr, paddr_t size)
{
uint32_t *zimage = (void *)FIXMAP_ADDR(FIXMAP_MISC);
uint32_t start, end;
struct minimal_dtb_header dtb_hdr;
- set_fixmap(FIXMAP_MISC, KERNEL_FLASH_ADDRESS >> PAGE_SHIFT, DEV_SHARED);
+ set_fixmap(FIXMAP_MISC, addr >> PAGE_SHIFT, DEV_SHARED);
+
+ zimage += addr & ~PAGE_MASK;
if (zimage[ZIMAGE_MAGIC_OFFSET/4] != ZIMAGE_MAGIC)
return -EINVAL;
@@ -106,16 +109,24 @@ static int kernel_try_zimage_prepare(struct kernel_info *info)
start = zimage[ZIMAGE_START_OFFSET/4];
end = zimage[ZIMAGE_END_OFFSET/4];
+ if ( end > addr + size )
+ return -EINVAL;
+
clear_fixmap(FIXMAP_MISC);
/*
* Check for an appended DTB.
*/
- copy_from_paddr(&dtb_hdr, KERNEL_FLASH_ADDRESS + end - start, sizeof(dtb_hdr), DEV_SHARED);
+ copy_from_paddr(&dtb_hdr, addr + end - start, sizeof(dtb_hdr), DEV_SHARED);
if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
end += be32_to_cpu(dtb_hdr.total_size);
+
+ if ( end > addr + size )
+ return -EINVAL;
}
+ info->zimage.kernel_addr = addr;
+
/*
* If start is zero, the zImage is position independent -- load it
* at 32k from start of RAM.
@@ -142,25 +153,26 @@ static void kernel_elf_load(struct kernel_info *info)
free_xenheap_pages(info->kernel_img, info->kernel_order);
}
-static int kernel_try_elf_prepare(struct kernel_info *info)
+static int kernel_try_elf_prepare(struct kernel_info *info,
+ paddr_t addr, paddr_t size)
{
int rc;
- info->kernel_order = get_order_from_bytes(KERNEL_FLASH_SIZE);
+ info->kernel_order = get_order_from_bytes(size);
info->kernel_img = alloc_xenheap_pages(info->kernel_order, 0);
if ( info->kernel_img == NULL )
panic("Cannot allocate temporary buffer for kernel.\n");
- copy_from_paddr(info->kernel_img, KERNEL_FLASH_ADDRESS, KERNEL_FLASH_SIZE, DEV_SHARED);
+ copy_from_paddr(info->kernel_img, addr, size, DEV_SHARED);
- if ( (rc = elf_init(&info->elf.elf, info->kernel_img, KERNEL_FLASH_SIZE )) != 0 )
- return rc;
+ if ( (rc = elf_init(&info->elf.elf, info->kernel_img, size )) != 0 )
+ goto err;
#ifdef VERBOSE
elf_set_verbose(&info->elf.elf);
#endif
elf_parse_binary(&info->elf.elf);
if ( (rc = elf_xen_parse(&info->elf.elf, &info->elf.parms)) != 0 )
- return rc;
+ goto err;
/*
* TODO: can the ELF header be used to find the physical address
@@ -170,15 +182,36 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
info->load = kernel_elf_load;
return 0;
+err:
+ free_xenheap_pages(info->kernel_img, info->kernel_order);
+ return rc;
}
int kernel_prepare(struct kernel_info *info)
{
int rc;
- rc = kernel_try_zimage_prepare(info);
+ paddr_t start, size;
+
+ if ( early_info.modules.nr_mods > 1 )
+ panic("Cannot handle dom0 initrd yet\n");
+
+ if ( early_info.modules.nr_mods < 1 )
+ {
+ printk("No boot modules found, trying flash\n");
+ start = KERNEL_FLASH_ADDRESS;
+ size = KERNEL_FLASH_SIZE;
+ }
+ else
+ {
+ printk("Loading kernel from boot module 1\n");
+ start = early_info.modules.module[0].start;
+ size = early_info.modules.module[0].size;
+ }
+
+ rc = kernel_try_zimage_prepare(info, start, size);
if (rc < 0)
- rc = kernel_try_elf_prepare(info);
+ rc = kernel_try_elf_prepare(info, start, size);
return rc;
}
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 4533568..2353e13 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -22,6 +22,7 @@ struct kernel_info {
union {
struct {
+ paddr_t kernel_addr;
paddr_t load_addr;
paddr_t len;
} zimage;
@@ -39,3 +40,12 @@ int kernel_prepare(struct kernel_info *info);
void kernel_load(struct kernel_info *info);
#endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 14/16] arm: load dom0 kernel from first boot module
2012-09-03 13:30 ` [PATCH 14/16] arm: load dom0 kernel from first boot module Ian Campbell
@ 2012-09-06 13:44 ` Tim Deegan
0 siblings, 0 replies; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 13:44 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679054), Ian Campbell wrote:
> -static int kernel_try_zimage_prepare(struct kernel_info *info)
> +static int kernel_try_zimage_prepare(struct kernel_info *info,
> + paddr_t addr, paddr_t size)
> {
> uint32_t *zimage = (void *)FIXMAP_ADDR(FIXMAP_MISC);
> uint32_t start, end;
> struct minimal_dtb_header dtb_hdr;
>
> - set_fixmap(FIXMAP_MISC, KERNEL_FLASH_ADDRESS >> PAGE_SHIFT, DEV_SHARED);
> + set_fixmap(FIXMAP_MISC, addr >> PAGE_SHIFT, DEV_SHARED);
> +
> + zimage += addr & ~PAGE_MASK;
>
> if (zimage[ZIMAGE_MAGIC_OFFSET/4] != ZIMAGE_MAGIC)
> return -EINVAL;
> @@ -106,16 +109,24 @@ static int kernel_try_zimage_prepare(struct kernel_info *info)
> start = zimage[ZIMAGE_START_OFFSET/4];
> end = zimage[ZIMAGE_END_OFFSET/4];
>
> + if ( end > addr + size )
> + return -EINVAL;
> +
> clear_fixmap(FIXMAP_MISC);
No clear_fixmap() on the error path? I see there isn't one on the
existing error path above, but I suspect that's not deliberate.
>
> /*
> * Check for an appended DTB.
> */
> - copy_from_paddr(&dtb_hdr, KERNEL_FLASH_ADDRESS + end - start, sizeof(dtb_hdr), DEV_SHARED);
> + copy_from_paddr(&dtb_hdr, addr + end - start, sizeof(dtb_hdr), DEV_SHARED);
> if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
> end += be32_to_cpu(dtb_hdr.total_size);
> +
> + if ( end > addr + size )
> + return -EINVAL;
There ought to be a bounds check before the copy_from_paddr as well
(though I suppose there's not much to do except fail more gracefully).
Tim.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 15/16] arm: discard boot modules after building domain 0.
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (13 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 14/16] arm: load dom0 kernel from first boot module Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 13:53 ` Tim Deegan
2012-09-03 13:30 ` [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line Ian Campbell
` (2 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/domain_build.c | 3 +++
xen/arch/arm/setup.c | 16 ++++++++++++++++
xen/include/asm-arm/setup.h | 2 ++
3 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a9e7f43..e96ed10 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -10,6 +10,7 @@
#include <xen/device_tree.h>
#include <xen/libfdt/libfdt.h>
#include <xen/guest_access.h>
+#include <asm/setup.h>
#include "gic.h"
#include "kernel.h"
@@ -308,6 +309,8 @@ int construct_dom0(struct domain *d)
dtb_load(&kinfo);
kernel_load(&kinfo);
+ discard_initial_modules();
+
clear_bit(_VPF_down, &v->pause_flags);
memset(regs, 0, sizeof(*regs));
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 85487fe..b23bdc1 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -69,6 +69,22 @@ static void __init processor_id(void)
READ_CP32(ID_ISAR3), READ_CP32(ID_ISAR4), READ_CP32(ID_ISAR5));
}
+void __init discard_initial_modules(void)
+{
+ struct dt_module_info *mi = &early_info.modules;
+ int i;
+
+ for ( i = 0; i < mi->nr_mods; i++ )
+ {
+ paddr_t s = mi->module[i].start;
+ paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
+
+ init_domheap_pages(s, e);
+ }
+
+ mi->nr_mods = 0;
+}
+
/*
* Returns the end address of the highest region in the range s..e
* with required size and alignment that does not conflict with the
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 8769f66..3267db0 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -9,6 +9,8 @@ void arch_get_xen_caps(xen_capabilities_info_t *info);
int construct_dom0(struct domain *d);
+void discard_initial_modules(void);
+
#endif
/*
* Local variables:
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 15/16] arm: discard boot modules after building domain 0.
2012-09-03 13:30 ` [PATCH 15/16] arm: discard boot modules after building domain 0 Ian Campbell
@ 2012-09-06 13:53 ` Tim Deegan
2012-09-06 13:57 ` Ian Campbell
0 siblings, 1 reply; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 13:53 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679055), Ian Campbell wrote:
> +void __init discard_initial_modules(void)
> +{
> + struct dt_module_info *mi = &early_info.modules;
> + int i;
> +
> + for ( i = 0; i < mi->nr_mods; i++ )
> + {
> + paddr_t s = mi->module[i].start;
> + paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
> +
> + init_domheap_pages(s, e);
Is there a risk of weirdness from adding non-superpage-aligned memory to
the domheap here, given that map_domain_page always uses 2MB mappings?
> + }
> +
> + mi->nr_mods = 0;
> +}
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 15/16] arm: discard boot modules after building domain 0.
2012-09-06 13:53 ` Tim Deegan
@ 2012-09-06 13:57 ` Ian Campbell
2012-09-06 14:03 ` Tim Deegan
0 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-06 13:57 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xen.org
On Thu, 2012-09-06 at 14:53 +0100, Tim Deegan wrote:
> At 13:30 +0000 on 03 Sep (1346679055), Ian Campbell wrote:
> > +void __init discard_initial_modules(void)
> > +{
> > + struct dt_module_info *mi = &early_info.modules;
> > + int i;
> > +
> > + for ( i = 0; i < mi->nr_mods; i++ )
> > + {
> > + paddr_t s = mi->module[i].start;
> > + paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
> > +
> > + init_domheap_pages(s, e);
>
> Is there a risk of weirdness from adding non-superpage-aligned memory to
> the domheap here, given that map_domain_page always uses 2MB mappings?
I hadn't thought about this.
These regions will mesh precisely with what setup_mm has added and
therefore the result is that the entire 2MB is on the heap, I think.
These non-aligned looking regions are actually within larger regions of
RAM so I don't think there is any danger of actually mapping some
non-RAM as part of such a 2MB mapping?
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 15/16] arm: discard boot modules after building domain 0.
2012-09-06 13:57 ` Ian Campbell
@ 2012-09-06 14:03 ` Tim Deegan
0 siblings, 0 replies; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 14:03 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
At 14:57 +0100 on 06 Sep (1346943456), Ian Campbell wrote:
> On Thu, 2012-09-06 at 14:53 +0100, Tim Deegan wrote:
> > At 13:30 +0000 on 03 Sep (1346679055), Ian Campbell wrote:
> > > +void __init discard_initial_modules(void)
> > > +{
> > > + struct dt_module_info *mi = &early_info.modules;
> > > + int i;
> > > +
> > > + for ( i = 0; i < mi->nr_mods; i++ )
> > > + {
> > > + paddr_t s = mi->module[i].start;
> > > + paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
> > > +
> > > + init_domheap_pages(s, e);
> >
> > Is there a risk of weirdness from adding non-superpage-aligned memory to
> > the domheap here, given that map_domain_page always uses 2MB mappings?
>
> I hadn't thought about this.
>
> These regions will mesh precisely with what setup_mm has added and
> therefore the result is that the entire 2MB is on the heap, I think.
>
> These non-aligned looking regions are actually within larger regions of
> RAM so I don't think there is any danger of actually mapping some
> non-RAM as part of such a 2MB mapping?
Righto. We also might care about accidental r/w mappings of r/o things,
but since Xen itself is 32MB-aligned that's OK. I think for now this is
all fine, so:
Acked-by: Tim Deegan <tim@xen.org>
though it will need adjustment if Xen becomes module 0, of course. :)
Cheers,
Tim.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (14 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 15/16] arm: discard boot modules after building domain 0 Ian Campbell
@ 2012-09-03 13:30 ` Ian Campbell
2012-09-06 13:50 ` Tim Deegan
2012-09-06 14:19 ` David Vrabel
2012-09-06 14:46 ` [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM David Vrabel
2012-10-11 14:57 ` Ian Campbell
17 siblings, 2 replies; 47+ messages in thread
From: Ian Campbell @ 2012-09-03 13:30 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
Ideally this would use module1-args iff the kernel came from
module1-{start,end} and the existing xen,dom0-bootargs if the kernel
came from flash, but this approach is simpler and has the sme effect
in practice.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/domain_build.c | 23 ++++++++++++++++++++---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e96ed10..2b65637 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -88,6 +88,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
{
int prop;
+ int had_mod1_args = 0;
+
for ( prop = fdt_first_property_offset(fdt, node);
prop >= 0;
prop = fdt_next_property_offset(fdt, prop) )
@@ -104,15 +106,30 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
prop_len = fdt32_to_cpu(p->len);
/*
- * In chosen node: replace bootargs with value from
- * xen,dom0-bootargs.
+ * In chosen node:
+ *
+ * * replace bootargs with value from module1-args, falling
+ * back to xen,dom0-bootargs if not present.
+ * * remove all other module*.
*/
if ( device_tree_node_matches(fdt, node, "chosen") )
{
if ( strcmp(prop_name, "bootargs") == 0 )
continue;
- if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
+ if ( strcmp(prop_name, "module1-args") == 0 )
+ {
prop_name = "bootargs";
+ had_mod1_args = 1;
+ }
+ if ( strncmp(prop_name, "module", strlen("module")) == 0 )
+ continue;
+ if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
+ {
+ if ( had_mod1_args )
+ continue;
+ else
+ prop_name = "bootargs";
+ }
}
/*
* In a memory node: adjust reg property.
--
1.7.9.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-03 13:30 ` [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line Ian Campbell
@ 2012-09-06 13:50 ` Tim Deegan
2012-09-06 13:55 ` Ian Campbell
2012-09-06 14:19 ` David Vrabel
1 sibling, 1 reply; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 13:50 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
At 13:30 +0000 on 03 Sep (1346679056), Ian Campbell wrote:
> Ideally this would use module1-args iff the kernel came from
> module1-{start,end} and the existing xen,dom0-bootargs if the kernel
> came from flash, but this approach is simpler and has the sme effect
> in practice.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/arch/arm/domain_build.c | 23 ++++++++++++++++++++---
> 1 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e96ed10..2b65637 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -88,6 +88,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> {
> int prop;
>
> + int had_mod1_args = 0;
> +
> for ( prop = fdt_first_property_offset(fdt, node);
> prop >= 0;
> prop = fdt_next_property_offset(fdt, prop) )
> @@ -104,15 +106,30 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> prop_len = fdt32_to_cpu(p->len);
>
> /*
> - * In chosen node: replace bootargs with value from
> - * xen,dom0-bootargs.
> + * In chosen node:
> + *
> + * * replace bootargs with value from module1-args, falling
> + * back to xen,dom0-bootargs if not present.
> + * * remove all other module*.
> */
> if ( device_tree_node_matches(fdt, node, "chosen") )
> {
> if ( strcmp(prop_name, "bootargs") == 0 )
> continue;
> - if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> + if ( strcmp(prop_name, "module1-args") == 0 )
> + {
> prop_name = "bootargs";
> + had_mod1_args = 1;
> + }
> + if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> + continue;
ITYM "else if" here, or the module1-args property gets dropped too,
doesn't it?
Tim.
> + if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> + {
> + if ( had_mod1_args )
> + continue;
> + else
> + prop_name = "bootargs";
> + }
> }
> /*
> * In a memory node: adjust reg property.
> --
> 1.7.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-06 13:50 ` Tim Deegan
@ 2012-09-06 13:55 ` Ian Campbell
2012-09-06 13:58 ` Tim Deegan
0 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-06 13:55 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xen.org
On Thu, 2012-09-06 at 14:50 +0100, Tim Deegan wrote:
> At 13:30 +0000 on 03 Sep (1346679056), Ian Campbell wrote:
> > Ideally this would use module1-args iff the kernel came from
> > module1-{start,end} and the existing xen,dom0-bootargs if the kernel
> > came from flash, but this approach is simpler and has the sme effect
> > in practice.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > xen/arch/arm/domain_build.c | 23 ++++++++++++++++++++---
> > 1 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index e96ed10..2b65637 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -88,6 +88,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> > {
> > int prop;
> >
> > + int had_mod1_args = 0;
> > +
> > for ( prop = fdt_first_property_offset(fdt, node);
> > prop >= 0;
> > prop = fdt_next_property_offset(fdt, prop) )
> > @@ -104,15 +106,30 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> > prop_len = fdt32_to_cpu(p->len);
> >
> > /*
> > - * In chosen node: replace bootargs with value from
> > - * xen,dom0-bootargs.
> > + * In chosen node:
> > + *
> > + * * replace bootargs with value from module1-args, falling
> > + * back to xen,dom0-bootargs if not present.
> > + * * remove all other module*.
> > */
> > if ( device_tree_node_matches(fdt, node, "chosen") )
> > {
> > if ( strcmp(prop_name, "bootargs") == 0 )
> > continue;
> > - if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> > + if ( strcmp(prop_name, "module1-args") == 0 )
> > + {
> > prop_name = "bootargs";
> > + had_mod1_args = 1;
> > + }
> > + if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> > + continue;
>
> ITYM "else if" here, or the module1-args property gets dropped too,
> doesn't it?
The intention was to filter "module*" from the DTB altogether, since
they were intended for Xen not dom0. All that should remain is
module1-args which has been renamed to bootargs which is what dom0
expects.
That isn't really mentioned in the commit log but I did at least comment
it above ;-)
>
> Tim.
>
> > + if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> > + {
> > + if ( had_mod1_args )
> > + continue;
> > + else
> > + prop_name = "bootargs";
> > + }
> > }
> > /*
> > * In a memory node: adjust reg property.
> > --
> > 1.7.9.1
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-06 13:55 ` Ian Campbell
@ 2012-09-06 13:58 ` Tim Deegan
2012-09-06 13:59 ` Ian Campbell
0 siblings, 1 reply; 47+ messages in thread
From: Tim Deegan @ 2012-09-06 13:58 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
At 14:55 +0100 on 06 Sep (1346943300), Ian Campbell wrote:
> > > if ( device_tree_node_matches(fdt, node, "chosen") )
> > > {
> > > if ( strcmp(prop_name, "bootargs") == 0 )
> > > continue;
> > > - if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> > > + if ( strcmp(prop_name, "module1-args") == 0 )
> > > + {
> > > prop_name = "bootargs";
> > > + had_mod1_args = 1;
> > > + }
> > > + if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> > > + continue;
> >
> > ITYM "else if" here, or the module1-args property gets dropped too,
> > doesn't it?
>
> The intention was to filter "module*" from the DTB altogether, since
> they were intended for Xen not dom0. All that should remain is
> module1-args which has been renamed to bootargs which is what dom0
> expects.
Argh, of course now prop_name's been clobbered it won't be filtered. :)
Sorry for the noise.
Acked-by: Tim Deegan <tim@xen.org>, though I imagine you'll want
David's ack rather than mine.
Tim.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-06 13:58 ` Tim Deegan
@ 2012-09-06 13:59 ` Ian Campbell
0 siblings, 0 replies; 47+ messages in thread
From: Ian Campbell @ 2012-09-06 13:59 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xen.org
On Thu, 2012-09-06 at 14:58 +0100, Tim Deegan wrote:
> At 14:55 +0100 on 06 Sep (1346943300), Ian Campbell wrote:
> > > > if ( device_tree_node_matches(fdt, node, "chosen") )
> > > > {
> > > > if ( strcmp(prop_name, "bootargs") == 0 )
> > > > continue;
> > > > - if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> > > > + if ( strcmp(prop_name, "module1-args") == 0 )
> > > > + {
> > > > prop_name = "bootargs";
> > > > + had_mod1_args = 1;
> > > > + }
> > > > + if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> > > > + continue;
> > >
> > > ITYM "else if" here, or the module1-args property gets dropped too,
> > > doesn't it?
> >
> > The intention was to filter "module*" from the DTB altogether, since
> > they were intended for Xen not dom0. All that should remain is
> > module1-args which has been renamed to bootargs which is what dom0
> > expects.
>
> Argh, of course now prop_name's been clobbered it won't be filtered. :)
> Sorry for the noise.
Actually I hadn't got quite what you were suggesting, and it is a bit
subtle now you put it that way. An else if would probably help the code
be self documenting.
>
> Acked-by: Tim Deegan <tim@xen.org>, though I imagine you'll want
> David's ack rather than mine.
>
> Tim.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-03 13:30 ` [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line Ian Campbell
2012-09-06 13:50 ` Tim Deegan
@ 2012-09-06 14:19 ` David Vrabel
2012-09-06 14:28 ` Ian Campbell
1 sibling, 1 reply; 47+ messages in thread
From: David Vrabel @ 2012-09-06 14:19 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
On 03/09/12 14:30, Ian Campbell wrote:
> Ideally this would use module1-args iff the kernel came from
> module1-{start,end} and the existing xen,dom0-bootargs if the kernel
> came from flash, but this approach is simpler and has the sme effect
> in practice.
Is module1-args defined in a spec somewhere?
If the DT has xen,dom0-bootargs followed by module1-args you will end up
with two bootargs nodes. Suggest preferring the first one found and
discard the other.
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/arch/arm/domain_build.c | 23 ++++++++++++++++++++---
> 1 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e96ed10..2b65637 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -88,6 +88,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> {
> int prop;
>
> + int had_mod1_args = 0;
> +
> for ( prop = fdt_first_property_offset(fdt, node);
> prop >= 0;
> prop = fdt_next_property_offset(fdt, prop) )
> @@ -104,15 +106,30 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> prop_len = fdt32_to_cpu(p->len);
>
> /*
> - * In chosen node: replace bootargs with value from
> - * xen,dom0-bootargs.
> + * In chosen node:
> + *
> + * * replace bootargs with value from module1-args, falling
> + * back to xen,dom0-bootargs if not present.
> + * * remove all other module*.
> */
> if ( device_tree_node_matches(fdt, node, "chosen") )
> {
> if ( strcmp(prop_name, "bootargs") == 0 )
> continue;
> - if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> + if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> + continue;
Clearer to write this as:
if ( strncmp(prop_name, "module", strlen("module")) == 0 )
if ( strcmp(prop_name, "module1-args") == 0 )
{
prop_name = "bootargs";
had_mod1_args = 1;
}
else
continue;
}
> + if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> + {
> + if ( had_mod1_args )
> + continue;
> + else
> + prop_name = "bootargs";
> + }
> }
> /*
> * In a memory node: adjust reg property.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line
2012-09-06 14:19 ` David Vrabel
@ 2012-09-06 14:28 ` Ian Campbell
0 siblings, 0 replies; 47+ messages in thread
From: Ian Campbell @ 2012-09-06 14:28 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel@lists.xen.org
On Thu, 2012-09-06 at 15:19 +0100, David Vrabel wrote:
> On 03/09/12 14:30, Ian Campbell wrote:
> > Ideally this would use module1-args iff the kernel came from
> > module1-{start,end} and the existing xen,dom0-bootargs if the kernel
> > came from flash, but this approach is simpler and has the sme effect
> > in practice.
>
> Is module1-args defined in a spec somewhere?
Nope, I made it up as a PoC. I described it a little bit in the 0/16
mail.
> If the DT has xen,dom0-bootargs followed by module1-args you will end up
> with two bootargs nodes. Suggest preferring the first one found and
> discard the other.
Yes.
>
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > xen/arch/arm/domain_build.c | 23 ++++++++++++++++++++---
> > 1 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index e96ed10..2b65637 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -88,6 +88,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> > {
> > int prop;
> >
> > + int had_mod1_args = 0;
> > +
> > for ( prop = fdt_first_property_offset(fdt, node);
> > prop >= 0;
> > prop = fdt_next_property_offset(fdt, prop) )
> > @@ -104,15 +106,30 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> > prop_len = fdt32_to_cpu(p->len);
> >
> > /*
> > - * In chosen node: replace bootargs with value from
> > - * xen,dom0-bootargs.
> > + * In chosen node:
> > + *
> > + * * replace bootargs with value from module1-args, falling
> > + * back to xen,dom0-bootargs if not present.
> > + * * remove all other module*.
> > */
> > if ( device_tree_node_matches(fdt, node, "chosen") )
> > {
> > if ( strcmp(prop_name, "bootargs") == 0 )
> > continue;
> > - if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> > + if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> > + continue;
>
> Clearer to write this as:
>
> if ( strncmp(prop_name, "module", strlen("module")) == 0 )
> if ( strcmp(prop_name, "module1-args") == 0 )
> {
> prop_name = "bootargs";
> had_mod1_args = 1;
> }
> else
> continue;
> }
>
> > + if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> > + {
> > + if ( had_mod1_args )
> > + continue;
> > + else
> > + prop_name = "bootargs";
> > + }
> > }
> > /*
> > * In a memory node: adjust reg property.
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (15 preceding siblings ...)
2012-09-03 13:30 ` [PATCH 16/16] arm: use /chosen/module1-args for domain 0 command line Ian Campbell
@ 2012-09-06 14:46 ` David Vrabel
2012-09-10 16:12 ` Ian Campbell
2012-10-11 14:57 ` Ian Campbell
17 siblings, 1 reply; 47+ messages in thread
From: David Vrabel @ 2012-09-06 14:46 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
On 03/09/12 14:28, Ian Campbell wrote:
> The following series implements support for initial images and DTB in
> RAM, as opposed to in flash (dom0 kernel) or compiled into the
> hypervisor (DTB). It arranges to not clobber these with either the h/v
> text on relocation or with the heaps and frees them as appropriate.
>
> Most of this is independent of the specific bootloader protocol which is
> used to tell Xen where these modules actually are, but I have included a
> simple PoC bootloader protocol based around device tree which is similar
> to the protocol used by Linux to find its initrd
> (where /chosen/linux,initrd-{start,end} indicate the physical address).
>
> In the PoC the modules are listed in the chosen node starting
> with /chosen/nr-modules which contains the count and then /chosen/module
> %d-{start,end} which gives the physical address of the module
> and /chosen/module%d-args which give its command line.
Until there is an agreement on this protocol I would prepend a "xen,"
prefix to the node names (xen,nr-modules etc.).
bootargs instead of args would be more consistent perhaps. So,
module1-args becomes xen,module1-bootargs.
The proposed protocol is functional and useful using nodes for each
module seems to be more device-tree-ish. I think in the longer term,
perhaps something like the following would be better?
chosen {
module@1 {
compatible = "multiboot-module";
regs = <0x12345678 0x01000>;
bootargs = "frob";
};
module@2 {
compatible = "multiboot-module";
regs = <0x12345678 0x01000>;
}
}
David
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM
2012-09-06 14:46 ` [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM David Vrabel
@ 2012-09-10 16:12 ` Ian Campbell
2012-09-17 11:39 ` Stefano Stabellini
0 siblings, 1 reply; 47+ messages in thread
From: Ian Campbell @ 2012-09-10 16:12 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel
On Thu, 2012-09-06 at 15:46 +0100, David Vrabel wrote:
> On 03/09/12 14:28, Ian Campbell wrote:
> > The following series implements support for initial images and DTB in
> > RAM, as opposed to in flash (dom0 kernel) or compiled into the
> > hypervisor (DTB). It arranges to not clobber these with either the h/v
> > text on relocation or with the heaps and frees them as appropriate.
> >
> > Most of this is independent of the specific bootloader protocol which is
> > used to tell Xen where these modules actually are, but I have included a
> > simple PoC bootloader protocol based around device tree which is similar
> > to the protocol used by Linux to find its initrd
> > (where /chosen/linux,initrd-{start,end} indicate the physical address).
> >
> > In the PoC the modules are listed in the chosen node starting
> > with /chosen/nr-modules which contains the count and then /chosen/module
> > %d-{start,end} which gives the physical address of the module
> > and /chosen/module%d-args which give its command line.
>
> Until there is an agreement on this protocol I would prepend a "xen,"
> prefix to the node names (xen,nr-modules etc.).
OK.
> bootargs instead of args would be more consistent perhaps. So,
> module1-args becomes xen,module1-bootargs.
>
> The proposed protocol is functional and useful using nodes for each
> module seems to be more device-tree-ish. I think in the longer term,
> perhaps something like the following would be better?
I rather suspect I'm going to end up porting multiboot to ARM. But you
are right that this looks like a better DTish way than mine.
>
> chosen {
> module@1 {
Are these normally 1 based or 0 based in DT?
> compatible = "multiboot-module";
> regs = <0x12345678 0x01000>;
Is this start and length? This relates somehow to #address-cells or
something doesn't it?
> bootargs = "frob";
> };
> module@2 {
> compatible = "multiboot-module";
> regs = <0x12345678 0x01000>;
> }
> }
>
> David
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM
2012-09-10 16:12 ` Ian Campbell
@ 2012-09-17 11:39 ` Stefano Stabellini
0 siblings, 0 replies; 47+ messages in thread
From: Stefano Stabellini @ 2012-09-17 11:39 UTC (permalink / raw)
To: Ian Campbell; +Cc: David Vrabel, xen-devel
On Mon, 10 Sep 2012, Ian Campbell wrote:
> On Thu, 2012-09-06 at 15:46 +0100, David Vrabel wrote:
> > On 03/09/12 14:28, Ian Campbell wrote:
> > > The following series implements support for initial images and DTB in
> > > RAM, as opposed to in flash (dom0 kernel) or compiled into the
> > > hypervisor (DTB). It arranges to not clobber these with either the h/v
> > > text on relocation or with the heaps and frees them as appropriate.
> > >
> > > Most of this is independent of the specific bootloader protocol which is
> > > used to tell Xen where these modules actually are, but I have included a
> > > simple PoC bootloader protocol based around device tree which is similar
> > > to the protocol used by Linux to find its initrd
> > > (where /chosen/linux,initrd-{start,end} indicate the physical address).
> > >
> > > In the PoC the modules are listed in the chosen node starting
> > > with /chosen/nr-modules which contains the count and then /chosen/module
> > > %d-{start,end} which gives the physical address of the module
> > > and /chosen/module%d-args which give its command line.
> >
> > Until there is an agreement on this protocol I would prepend a "xen,"
> > prefix to the node names (xen,nr-modules etc.).
>
> OK.
>
> > bootargs instead of args would be more consistent perhaps. So,
> > module1-args becomes xen,module1-bootargs.
> >
> > The proposed protocol is functional and useful using nodes for each
> > module seems to be more device-tree-ish. I think in the longer term,
> > perhaps something like the following would be better?
>
> I rather suspect I'm going to end up porting multiboot to ARM. But you
> are right that this looks like a better DTish way than mine.
I think that the grub folks have already extended the original spec to
make it easier to port to other archs.
Give a look at include/multiboot2.h, it seems to support mips as well as
x86.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM
2012-09-03 13:28 [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM Ian Campbell
` (16 preceding siblings ...)
2012-09-06 14:46 ` [PATCH 0/16] arm: support for initial modules (e.g. dom0) and DTB supplied in RAM David Vrabel
@ 2012-10-11 14:57 ` Ian Campbell
17 siblings, 0 replies; 47+ messages in thread
From: Ian Campbell @ 2012-10-11 14:57 UTC (permalink / raw)
To: xen-devel
On Mon, 2012-09-03 at 14:28 +0100, Ian Campbell wrote:
> The following series implements support for initial images and DTB in
> RAM
A subset of these patches have been acked so I have applied them. They
are generally cleanups rather than the main boot-wrapper stuff.
arm: Zero the BSS at start of day.
arm: make virtual address defines unsigned
arm: move get_paddr_function to arch setup.c from device_tree.c
arm: really allocate boot frametable pages with 32M alignment
arm: print a message if multiple banks of memory are present.
arm: mark heap and frametable limits as read mostly
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread