Hi, This is the second version of this small patch series. It adds support to load initrd in dom0 for ARM. All the changes can be found in each patch. Cheers, Julien Grall (4): xen/arm: Add macro MB xen/arm: Add macro ALIGN xen/arm: Add support to load initrd in dom0 xen/dts: Support Linux initrd DT bindings xen/arch/arm/domain_build.c | 101 ++++++++++++++++++++++++++++++++++++------ xen/arch/arm/kernel.c | 20 +++++---- xen/arch/arm/kernel.h | 2 + xen/common/device_tree.c | 27 ++++++++++- xen/include/asm-arm/config.h | 5 +++ 5 files changed, 132 insertions(+), 23 deletions(-) -- 1.7.10.4
Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/include/asm-arm/config.h | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index 604088e..2cea1ba 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -119,6 +119,7 @@ #define BOOT_MISC_VIRT_START _AT(vaddr_t,0x00600000) #define HYPERVISOR_VIRT_START XEN_VIRT_START +#define MB(_mb) (_AC(_mb, UL) << 20) #ifdef CONFIG_ARM_32 -- 1.7.10.4
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/common/device_tree.c | 2 --
xen/include/asm-arm/config.h | 4 ++++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 9a16650..32cbb1e 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -65,8 +65,6 @@ static LIST_HEAD(aliases_lookup);
printk(fmt, ## __VA_ARGS__); \
} while (0)
-#define ALIGN(x, a) ((x + (a) - 1) & ~((a) - 1));
-
// #define DEBUG_DT
#ifdef DEBUG_DT
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 2cea1ba..35f3b29 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -186,6 +186,10 @@
#define STACK_SIZE (PAGE_SIZE << STACK_ORDER)
#ifndef __ASSEMBLY__
+#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
+#endif
+
+#ifndef __ASSEMBLY__
extern unsigned long xen_phys_start;
extern unsigned long xenheap_phys_end;
extern unsigned long frametable_virt_end;
--
1.7.10.4
Julien Grall
2013-Sep-26 12:56 UTC
[PATCH v2 3/4] xen/arm: Add support to load initrd in dom0
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Changes in v2:
- Align the initrd and DTB length to 2MB
---
xen/arch/arm/domain_build.c | 101 +++++++++++++++++++++++++++++++++++++------
xen/arch/arm/kernel.c | 20 +++++----
xen/arch/arm/kernel.h | 2 +
3 files changed, 102 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b8c8a5d..7fc597f 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -163,12 +163,16 @@ static int write_properties(struct domain *d, struct
kernel_info *kinfo,
*
* * remember xen,dom0-bootargs if we don''t already have
* bootargs (from module #1, above).
- * * remove bootargs, xen,dom0-bootargs and xen,xen-bootargs.
+ * * remove bootargs, xen,dom0-bootargs, xen,xen-bootargs,
+ * linux,initrd-start and linux,initrd-end.
*/
if ( dt_node_path_is_equal(np, "/chosen") )
{
- if ( dt_property_name_is_equal(pp, "xen,xen-bootargs") )
+ if ( dt_property_name_is_equal(pp, "xen,xen-bootargs") ||
+ dt_property_name_is_equal(pp, "linux,initrd-start")
||
+ dt_property_name_is_equal(pp, "linux,initrd-end") )
continue;
+
if ( dt_property_name_is_equal(pp, "xen,dom0-bootargs") )
{
had_dom0_bootargs = 1;
@@ -214,12 +218,22 @@ static int write_properties(struct domain *d, struct
kernel_info *kinfo,
strlen(bootargs) + 1);
if ( res )
return res;
- }
- /*
- * XXX should populate /chosen/linux,initrd-{start,end} here if we
- * have module[2]
- */
+ /*
+ * If the bootloader provides an initrd, we must create a placeholder
+ * for the initrd properties. The values will be replaced later.
+ */
+ if ( early_info.modules.module[MOD_INITRD].size )
+ {
+ res = fdt_property_cell(kinfo->fdt,
"linux,initrd-start", 0);
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(kinfo->fdt,
"linux,initrd-end", 0);
+ if ( res )
+ return res;
+ }
+ }
return 0;
}
@@ -779,6 +793,8 @@ static int prepare_dtb(struct domain *d, struct kernel_info
*kinfo)
int new_size;
int ret;
paddr_t end;
+ paddr_t initrd_len;
+ paddr_t dtb_len;
ASSERT(dt_host && (dt_host->sibling == NULL));
@@ -805,8 +821,10 @@ static int prepare_dtb(struct domain *d, struct kernel_info
*kinfo)
if ( ret < 0 )
goto err;
- /* Actual new size */
- new_size = fdt_totalsize(kinfo->fdt);
+ /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment
*/
+ initrd_len = ALIGN(early_info.modules.module[MOD_INITRD].size, MB(2));
+ dtb_len = ALIGN(fdt_totalsize(kinfo->fdt), MB(2));
+ new_size = initrd_len + dtb_len;
/*
* DTB must be loaded such that it does not conflict with the
@@ -815,14 +833,14 @@ static int prepare_dtb(struct domain *d, struct
kernel_info *kinfo)
* the recommendation in Documentation/arm64/booting.txt is below
* 512MB. Place at 128MB, (or, if we have less RAM, as high as
* possible) in order to satisfy both.
+ * If the bootloader provides an initrd, it will be loaded just
+ * after the DTB.
*/
end = kinfo->mem.bank[0].start + kinfo->mem.bank[0].size;
end = MIN(kinfo->mem.bank[0].start + (128<<20) + new_size, end);
- kinfo->dtb_paddr = end - new_size;
-
- /* Align the address to 2Mb. Linux only requires 4 byte alignment */
- kinfo->dtb_paddr &= ~((2 << 20) - 1);
+ kinfo->initrd_paddr = end - initrd_len;
+ kinfo->dtb_paddr = kinfo->initrd_paddr - dtb_len;
if ( kinfo->dtb_paddr < kinfo->mem.bank[0].start ||
kinfo->mem.bank[0].start + new_size > end )
@@ -855,6 +873,61 @@ static void dtb_load(struct kernel_info *kinfo)
xfree(kinfo->fdt);
}
+static void initrd_load(struct kernel_info *kinfo)
+{
+ paddr_t load_addr = kinfo->initrd_paddr;
+ paddr_t paddr = early_info.modules.module[MOD_INITRD].start;
+ paddr_t len = early_info.modules.module[MOD_INITRD].size;
+ unsigned long offs;
+ int node;
+ int res;
+
+ if ( !len )
+ return;
+
+ printk("Loading dom0 initrd from %"PRIpaddr" to
0x%"PRIpaddr"-0x%"PRIpaddr"\n",
+ paddr, load_addr, load_addr + len);
+
+ /* Fix up linux,initrd-start and linux,initrd-end in /chosen */
+ node = fdt_path_offset(kinfo->fdt, "/chosen");
+ if ( node < 0 )
+ panic("Cannot find the /chosen node");
+
+ res = fdt_setprop_inplace_cell(kinfo->fdt, node,
"linux,initrd-start",
+ load_addr);
+ if ( res )
+ panic("Cannot fix up \"linux,initrd-start\"
property\n");
+
+ res = fdt_setprop_inplace_cell(kinfo->fdt, node,
"linux,initrd-end",
+ load_addr + len);
+ if ( res )
+ panic("Cannot fix up \"linux,initrd-end\"
property\n");
+
+ for ( offs = 0; offs < len; )
+ {
+ int rc;
+ paddr_t s, l, ma;
+ void *dst;
+
+ s = offs & ~PAGE_MASK;
+ l = min(PAGE_SIZE - s, len);
+
+ rc = gvirt_to_maddr(load_addr + offs, &ma);
+ if ( rc )
+ {
+ panic("\nUnable to translate guest address\n");
+ return;
+ }
+
+ dst = map_domain_page(ma>>PAGE_SHIFT);
+
+ copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
+
+ unmap_domain_page(dst);
+ offs += l;
+ }
+}
+
int construct_dom0(struct domain *d)
{
struct kernel_info kinfo = {};
@@ -891,6 +964,8 @@ int construct_dom0(struct domain *d)
p2m_load_VTTBR(d);
kernel_load(&kinfo);
+ /* initrd_load will fix up the fdt, so call it before dtb_load */
+ initrd_load(&kinfo);
dtb_load(&kinfo);
discard_initial_modules();
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 7a91d60..bd59d74 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -71,15 +71,22 @@ static void kernel_zimage_check_overlap(struct kernel_info
*info)
{
paddr_t zimage_start = info->zimage.load_addr;
paddr_t zimage_end = info->zimage.load_addr + info->zimage.len;
- paddr_t dtb_start = info->dtb_paddr;
- paddr_t dtb_end = info->dtb_paddr + fdt_totalsize(info->fdt);
+ paddr_t start = info->dtb_paddr;
+ paddr_t end;
- if ( (dtb_start > zimage_end) || (dtb_end < zimage_start) )
+ end = info->initrd_paddr + early_info.modules.module[MOD_INITRD].size;
+
+ /*
+ * In the dom0 memory, the initrd will be just after the DTB. So we
+ * only need to check if the zImage range will overlap the
+ * DTB-initrd range.
+ */
+ if ( (start > zimage_end) || (end < zimage_start) )
return;
panic(XENLOG_ERR "The kernel(0x%"PRIpaddr"-0x%"PRIpaddr
- ") is overlapping the
DTB(0x%"PRIpaddr"-0x%"PRIpaddr")\n",
- zimage_start, zimage_end, dtb_start, dtb_end);
+ ") is overlapping the
DTB-initrd(0x%"PRIpaddr"-0x%"PRIpaddr")\n",
+ zimage_start, zimage_end, start, end);
}
static void kernel_zimage_load(struct kernel_info *info)
@@ -328,9 +335,6 @@ int kernel_prepare(struct kernel_info *info)
paddr_t start, size;
- if ( early_info.modules.nr_mods > MOD_INITRD )
- panic("Cannot handle dom0 initrd yet\n");
-
if ( early_info.modules.nr_mods < MOD_KERNEL )
{
printk("No boot modules found, trying flash\n");
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index c900e74..debf590 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -21,6 +21,8 @@ struct kernel_info {
paddr_t dtb_paddr;
paddr_t entry;
+ paddr_t initrd_paddr;
+
void *kernel_img;
unsigned kernel_order;
--
1.7.10.4
Julien Grall
2013-Sep-26 12:56 UTC
[PATCH v2 4/4] xen/dts: Support Linux initrd DT bindings
Linux uses the property linux,initrd-start and linux,initrd-end to know where
the initrd lives in memory.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/common/device_tree.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 32cbb1e..8154eaf 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -380,6 +380,29 @@ static void __init process_multiboot_node(const void *fdt,
int node,
early_info.modules.nr_mods = nr;
}
+static void __init process_chosen_node(const void *fdt, int node,
+ const char *name,
+ u32 address_cells, u32 size_cells)
+{
+ struct dt_mb_module *mod = &early_info.modules.module[MOD_INITRD];
+ u32 start, end;
+
+ dt_printk("Checking for initrd in /chosen\n");
+
+ start = device_tree_get_u32(fdt, node, "linux,initrd-start", 0);
+ end = device_tree_get_u32(fdt, node, "linux,initrd-end", 0);
+
+ if ( !start || !end || (start >= end) )
+ return;
+
+ dt_printk("Initrd 0x%x-0x%x\n", start, end);
+
+ mod->start = start;
+ mod->size = end - start;
+
+ early_info.modules.nr_mods = MAX(MOD_INITRD, early_info.modules.nr_mods);
+}
+
static int __init early_scan_node(const void *fdt,
int node, const char *name, int depth,
u32 address_cells, u32 size_cells,
@@ -389,6 +412,8 @@ static int __init early_scan_node(const void *fdt,
process_memory_node(fdt, node, name, address_cells, size_cells);
else if ( device_tree_node_compatible(fdt, node,
"xen,multiboot-module" ) )
process_multiboot_node(fdt, node, name, address_cells, size_cells);
+ else if ( depth == 1 && device_tree_node_matches(fdt, node,
"chosen") )
+ process_chosen_node(fdt, node, name, address_cells, size_cells);
return 0;
}
--
1.7.10.4
On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote:> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > index 2cea1ba..35f3b29 100644 > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -186,6 +186,10 @@ > #define STACK_SIZE (PAGE_SIZE << STACK_ORDER) > > #ifndef __ASSEMBLY__ > +#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))Seems like there ought to be a place under include/xen where this would fit. xen/include/xen/lib.h I reckon.> +#endif > + > +#ifndef __ASSEMBLY__ > extern unsigned long xen_phys_start; > extern unsigned long xenheap_phys_end; > extern unsigned long frametable_virt_end;
On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote:> Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/include/asm-arm/config.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > index 604088e..2cea1ba 100644 > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -119,6 +119,7 @@ > #define BOOT_MISC_VIRT_START _AT(vaddr_t,0x00600000) > > #define HYPERVISOR_VIRT_START XEN_VIRT_START > +#define MB(_mb) (_AC(_mb, UL) << 20)Can you move the GB here too for consistency. In fact it would be worth considering moving this to xen/include/xen/config.h and consolidating the x86 version too. Thanks, Ian.
On 26/09/13 15:30, Ian Campbell wrote:> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: > >> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h >> index 2cea1ba..35f3b29 100644 >> --- a/xen/include/asm-arm/config.h >> +++ b/xen/include/asm-arm/config.h >> @@ -186,6 +186,10 @@ >> #define STACK_SIZE (PAGE_SIZE << STACK_ORDER) >> >> #ifndef __ASSEMBLY__ >> +#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1)) > Seems like there ought to be a place under include/xen where this would > fit. > > xen/include/xen/lib.h I reckon.Except x86 has #define ALIGN _ALIGN which will surely cause problems? ~Andrew
Ian Campbell
2013-Sep-26 14:34 UTC
Re: [PATCH v2 3/4] xen/arm: Add support to load initrd in dom0
On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote:> Signed-off-by: Julien Grall <julien.grall@linaro.org>Acked-by: Ian Campbell <ian.campbell@citrix.com>
On Thu, 2013-09-26 at 15:34 +0100, Andrew Cooper wrote:> On 26/09/13 15:30, Ian Campbell wrote: > > On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: > > > >> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > >> index 2cea1ba..35f3b29 100644 > >> --- a/xen/include/asm-arm/config.h > >> +++ b/xen/include/asm-arm/config.h > >> @@ -186,6 +186,10 @@ > >> #define STACK_SIZE (PAGE_SIZE << STACK_ORDER) > >> > >> #ifndef __ASSEMBLY__ > >> +#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1)) > > Seems like there ought to be a place under include/xen where this would > > fit. > > > > xen/include/xen/lib.h I reckon. > > Except x86 has #define ALIGN _ALIGN which will surely cause problems?ARM has this too (assuming you meant __ALIGN, can''t find the other), but in both cases it is only the #ifdef __ASSEMBLY__, whereas this adds a non-asm version. Could avoid any confusion by naming this version ROUNDUP. xen/common/xmalloc_tlsf.c has ROUNDUP_PAGE already too BTW. Ian.
On 26/09/13 15:38, Ian Campbell wrote:> On Thu, 2013-09-26 at 15:34 +0100, Andrew Cooper wrote: >> On 26/09/13 15:30, Ian Campbell wrote: >>> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: >>> >>>> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h >>>> index 2cea1ba..35f3b29 100644 >>>> --- a/xen/include/asm-arm/config.h >>>> +++ b/xen/include/asm-arm/config.h >>>> @@ -186,6 +186,10 @@ >>>> #define STACK_SIZE (PAGE_SIZE << STACK_ORDER) >>>> >>>> #ifndef __ASSEMBLY__ >>>> +#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1)) >>> Seems like there ought to be a place under include/xen where this would >>> fit. >>> >>> xen/include/xen/lib.h I reckon. >> Except x86 has #define ALIGN _ALIGN which will surely cause problems? > ARM has this too (assuming you meant __ALIGN, can''t find the other), but > in both cases it is only the #ifdef __ASSEMBLY__, whereas this adds a > non-asm version. > > Could avoid any confusion by naming this version ROUNDUP. > > xen/common/xmalloc_tlsf.c has ROUNDUP_PAGE already too BTW. > > Ian. >That seems like a sensible way forward. ~Andrew
On 09/26/2013 03:32 PM, Ian Campbell wrote:> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> --- >> xen/include/asm-arm/config.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h >> index 604088e..2cea1ba 100644 >> --- a/xen/include/asm-arm/config.h >> +++ b/xen/include/asm-arm/config.h >> @@ -119,6 +119,7 @@ >> #define BOOT_MISC_VIRT_START _AT(vaddr_t,0x00600000) >> >> #define HYPERVISOR_VIRT_START XEN_VIRT_START >> +#define MB(_mb) (_AC(_mb, UL) << 20) > > Can you move the GB here too for consistency. > > In fact it would be worth considering moving this to > xen/include/xen/config.h and consolidating the x86 version too.I will do. I''m wondering, do we need to use ULL instead of UL in GB and MB? -- Julien Grall
On 09/26/2013 03:38 PM, Ian Campbell wrote:> On Thu, 2013-09-26 at 15:34 +0100, Andrew Cooper wrote: >> On 26/09/13 15:30, Ian Campbell wrote: >>> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: >>> >>>> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h >>>> index 2cea1ba..35f3b29 100644 >>>> --- a/xen/include/asm-arm/config.h >>>> +++ b/xen/include/asm-arm/config.h >>>> @@ -186,6 +186,10 @@ >>>> #define STACK_SIZE (PAGE_SIZE << STACK_ORDER) >>>> >>>> #ifndef __ASSEMBLY__ >>>> +#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1)) >>> Seems like there ought to be a place under include/xen where this would >>> fit. >>> >>> xen/include/xen/lib.h I reckon. >> >> Except x86 has #define ALIGN _ALIGN which will surely cause problems? > > ARM has this too (assuming you meant __ALIGN, can''t find the other), but > in both cases it is only the #ifdef __ASSEMBLY__, whereas this adds a > non-asm version. > > Could avoid any confusion by naming this version ROUNDUP.I will rename it on the next patch series.> xen/common/xmalloc_tlsf.c has ROUNDUP_PAGE already too BTW. > > Ian. >-- Julien Grall
On Thu, 2013-09-26 at 16:26 +0100, Julien Grall wrote:> On 09/26/2013 03:32 PM, Ian Campbell wrote: > > On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> --- > >> xen/include/asm-arm/config.h | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > >> index 604088e..2cea1ba 100644 > >> --- a/xen/include/asm-arm/config.h > >> +++ b/xen/include/asm-arm/config.h > >> @@ -119,6 +119,7 @@ > >> #define BOOT_MISC_VIRT_START _AT(vaddr_t,0x00600000) > >> > >> #define HYPERVISOR_VIRT_START XEN_VIRT_START > >> +#define MB(_mb) (_AC(_mb, UL) << 20) > > > > Can you move the GB here too for consistency. > > > > In fact it would be worth considering moving this to > > xen/include/xen/config.h and consolidating the x86 version too. > > I will do.Thanks.> I''m wondering, do we need to use ULL instead of UL in GB and MB?Is it used with a paddr_t anywhere? If it''s just vaddr then UL is fine. Ian.
On 09/26/2013 04:30 PM, Ian Campbell wrote:> On Thu, 2013-09-26 at 16:26 +0100, Julien Grall wrote: >> On 09/26/2013 03:32 PM, Ian Campbell wrote: >>> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>>> --- >>>> xen/include/asm-arm/config.h | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h >>>> index 604088e..2cea1ba 100644 >>>> --- a/xen/include/asm-arm/config.h >>>> +++ b/xen/include/asm-arm/config.h >>>> @@ -119,6 +119,7 @@ >>>> #define BOOT_MISC_VIRT_START _AT(vaddr_t,0x00600000) >>>> >>>> #define HYPERVISOR_VIRT_START XEN_VIRT_START >>>> +#define MB(_mb) (_AC(_mb, UL) << 20) >>> >>> Can you move the GB here too for consistency. >>> >>> In fact it would be worth considering moving this to >>> xen/include/xen/config.h and consolidating the x86 version too. >> >> I will do. > > Thanks. > >> I''m wondering, do we need to use ULL instead of UL in GB and MB? > > Is it used with a paddr_t anywhere? If it''s just vaddr then UL is fine.MB will be used with paddr_t. GB not yet, but it could be used to replace 0x1...ULL in a such patch: http://permalink.gmane.org/gmane.comp.emulators.xen.devel/172128 -- Julien Grall
On Thu, 2013-09-26 at 16:32 +0100, Julien Grall wrote:> On 09/26/2013 04:30 PM, Ian Campbell wrote: > > On Thu, 2013-09-26 at 16:26 +0100, Julien Grall wrote: > >> On 09/26/2013 03:32 PM, Ian Campbell wrote: > >>> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote: > >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >>>> --- > >>>> xen/include/asm-arm/config.h | 1 + > >>>> 1 file changed, 1 insertion(+) > >>>> > >>>> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > >>>> index 604088e..2cea1ba 100644 > >>>> --- a/xen/include/asm-arm/config.h > >>>> +++ b/xen/include/asm-arm/config.h > >>>> @@ -119,6 +119,7 @@ > >>>> #define BOOT_MISC_VIRT_START _AT(vaddr_t,0x00600000) > >>>> > >>>> #define HYPERVISOR_VIRT_START XEN_VIRT_START > >>>> +#define MB(_mb) (_AC(_mb, UL) << 20) > >>> > >>> Can you move the GB here too for consistency. > >>> > >>> In fact it would be worth considering moving this to > >>> xen/include/xen/config.h and consolidating the x86 version too. > >> > >> I will do. > > > > Thanks. > > > >> I''m wondering, do we need to use ULL instead of UL in GB and MB? > > > > Is it used with a paddr_t anywhere? If it''s just vaddr then UL is fine. > > MB will be used with paddr_t. GB not yet, but it could be used to > replace 0x1...ULL in a such patch: > http://permalink.gmane.org/gmane.comp.emulators.xen.devel/172128 >OK, maybe give it a go. I don''t think there will be any critical places which would be badly impacted on 32-bit from having to use two registers etc where one would do. My only other concern would be assignments complaining about being truncated and the ball of wool effect fixing them all up -- but it quickly be obvious to you if this is going to be a problem! Ian.