Re: [PATCH] xen: arm: configure correct dom0_gnttab_start/size

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/09/14 16:21, Ian Campbell wrote:
> Vexpress is currently failing to boot for me with:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:301 __arm_ioremap_pfn_caller+0x118/0x1a4()
> CPU: 0 PID: 1 Comm: swapper Tainted: G        W     3.16.0-arm-native+ #276
> [<c0011e9c>] (unwind_backtrace) from [<c0010758>] (show_stack+0x10/0x14)
> [<c0010758>] (show_stack) from [<c001a3ec>] (warn_slowpath_common+0x5c/0x7c)
> [<c001a3ec>] (warn_slowpath_common) from [<c001a4c8>] (warn_slowpath_null+0x18/0x20)
> [<c001a4c8>] (warn_slowpath_null) from [<c001488c>] (__arm_ioremap_pfn_caller+0x118/0x1a4)
> [<c001488c>] (__arm_ioremap_pfn_caller) from [<c00149a0>] (__arm_ioremap+0x14/0x20)
> [<c00149a0>] (__arm_ioremap) from [<c01d103c>] (gnttab_setup_auto_xlat_frames+0x30/0xdc)
> [<c01d103c>] (gnttab_setup_auto_xlat_frames) from [<c0495324>] (xen_guest_init+0x19c/0x2d4)
> [<c0495324>] (xen_guest_init) from [<c0492c6c>] (do_one_initcall+0xfc/0x1a4)
> [<c0492c6c>] (do_one_initcall) from [<c0492d6c>] (kernel_init_freeable+0x58/0x1b4)
> [<c0492d6c>] (kernel_init_freeable) from [<c039611c>] (kernel_init+0x8/0xe4)
> [<c039611c>] (kernel_init) from [<c000de58>] (ret_from_fork+0x14/0x3c)
> ---[ end trace 3406ff24bd97382f ]---
> xen:grant_table: Failed to ioremap gnttab share frames (addr=0x00000000b0000000)!
>
> which is:
>         /*
>          * Don't allow RAM to be mapped - this causes problems with ARMv6+
>          */
>         if (WARN_ON(pfn_valid(pfn)))
>                 return NULL;
>
> This makes sense since the gnttab defaults to 0xb000000 and my dom0
> is being allocated a 1:1 mapping at 0xa0000000-0xc0000000.
>
> I suspect this broke around the time we stopped forcing dom0 memory to be
> allocated as low as possible which happened to prevent the default dom0_gnttab
> region overlapping RAM.
>
> This patch specifies an explicit dom0_gnttab base which is explicitly unused
> according to the FVP model docs (although it correpsonds to CS5 this isn't
> wired up to anything).
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> This just fixes vexpress, I wonder if a followup patch should either remove the
> default dom0_gnttab (forcing all platforms to specify one explicitly) or make
> the default something less arbitrary than 0xb0000000, e.g. 0x0-0x20000 or
> 0xfffe0000-0x100000000 (very start or very end of RAM). Perhaps with a command
> line option to override for new platform hacking.
>
> Or maybe we should search for an unused hole in the dom0 RAM space?

Why it is hard coded at all? (x86 appears to manage fine.)  Without it
being variably-located, there will always be a risk of collisions like this.

~Andrew

> ---
>  xen/arch/arm/platforms/vexpress.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c
> index 8e6a4ea..ce66935 100644
> --- a/xen/arch/arm/platforms/vexpress.c
> +++ b/xen/arch/arm/platforms/vexpress.c
> @@ -176,6 +176,8 @@ PLATFORM_START(vexpress, "VERSATILE EXPRESS")
>  #endif
>      .reset = vexpress_reset,
>      .blacklist_dev = vexpress_blacklist_dev,
> +    .dom0_gnttab_start = 0x10000000,
> +    .dom0_gnttab_size = 0x20000,
>  PLATFORM_END
>  
>  /*