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

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

[Ian Campbell]

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?
---
 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
 
 /*
-- 
1.7.10.4

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

[Ian Campbell]

On Thu, 2014-09-11 at 16:21 +0100, Ian Campbell wrote:

$subject should end "...for vexpress"

> 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?
> ---
>  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
>  
>  /*

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

[Andrew Cooper]

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
>  
>  /*

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

[Ian Campbell]

On Thu, 2014-09-11 at 16:54 +0100, Andrew Cooper wrote:
> 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.

For x86 PV (and shadow?) you can establish a mapping of the gnttab from
a virtual address directly to the machine address without needing a p2m
entry, via whichever hypercall it is which does that.

For HAP (and shadow?) you need a p2m mapping to get at the grant table,
so somewhere has to be found in the physical address space where it can
go.

In theory a guest kernel could try and find some io space which it isn't
using but to help them out we give them a hint.

For x86 HVM the IOBAR of the platform device as a handy place to put the
hint, for ARM domU we have a defined region in our address map which we
use and communicate via device tree.

But for ARM dom0 the physical address space layout follows the host,
figuring out a free bit of space is a bit tricky so each platform is
currently required (except some don't and there is a broken default) to
say where is a safe place to use (i.e. a person has to read the
datasheet and figure it out).

Ideally we would in the future walk the device tree and try and find a
hole in the address space, but that isn't implemented yet (for x86 PVHVM
Linux or ARM AFAICT, I heard a rumour that the windows PV drivers were
moving away from the platform device IOBAR somehow).

Ian.

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

[David Vrabel]

On 11/09/14 17:05, Ian Campbell wrote:
> On Thu, 2014-09-11 at 16:54 +0100, Andrew Cooper wrote:
>> 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.
> 
> For x86 PV (and shadow?) you can establish a mapping of the gnttab from
> a virtual address directly to the machine address without needing a p2m
> entry, via whichever hypercall it is which does that.
> 
> For HAP (and shadow?) you need a p2m mapping to get at the grant table,
> so somewhere has to be found in the physical address space where it can
> go.
> 
> In theory a guest kernel could try and find some io space which it isn't
> using but to help them out we give them a hint.
> 
> For x86 HVM the IOBAR of the platform device as a handy place to put the
> hint, for ARM domU we have a defined region in our address map which we
> use and communicate via device tree.
> 
> But for ARM dom0 the physical address space layout follows the host,
> figuring out a free bit of space is a bit tricky so each platform is
> currently required (except some don't and there is a broken default) to
> say where is a safe place to use (i.e. a person has to read the
> datasheet and figure it out).

Linux x86 PVH dom0 and domU uses ballooned pages and they're vmap()'d in
into a contiguous bit of virtual address space.

Perhaps something similar could be considered for ARM in the future?
This would be common code between ARM and x86 PVH.

David

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

[Julien Grall]

Hi Ian,

On 11/09/14 08:21, Ian Campbell wrote:
> 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.

Dropping the default value sounds a good solution.

> Or maybe we should search for an unused hole in the dom0 RAM space?

I'm not sure to understand your suggestion here. Did you mean that as 
DOM0 will never use all the host RAM, we could find a space in the RAM 
to use for grant table?

Regards,

-- 
Julien Grall

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

[Ian Campbell]

On Thu, 2014-09-11 at 13:11 -0700, Julien Grall wrote:
> Hi Ian,
> 
> On 11/09/14 08:21, Ian Campbell wrote:
> > 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.
> 
> Dropping the default value sounds a good solution.
> 
> > Or maybe we should search for an unused hole in the dom0 RAM space?
> 
> I'm not sure to understand your suggestion here. Did you mean that as 
> DOM0 will never use all the host RAM, we could find a space in the RAM 
> to use for grant table?

Something like that, yes.

In fact it occurred to me last night that that since the gnttab needs to
be populate anyway we could allocate it early in the case of 1:1 dom0
and then use that address as a 1:1 value for the gnttab IPA passed to
dom0.

For non-1:1 dom0 we could safely put it right over the dom0 memory
allocation, which as you observe is never going to be all of host ram
(since Xen must be using something...)

In fact, maybe using Xen's own 2M IPA would work everywhere.

Ian.

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

[Ian Campbell]

On Thu, 2014-09-11 at 17:51 +0100, David Vrabel wrote:
> On 11/09/14 17:05, Ian Campbell wrote:
> > On Thu, 2014-09-11 at 16:54 +0100, Andrew Cooper wrote:
> >> 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.
> > 
> > For x86 PV (and shadow?) you can establish a mapping of the gnttab from
> > a virtual address directly to the machine address without needing a p2m
> > entry, via whichever hypercall it is which does that.
> > 
> > For HAP (and shadow?) you need a p2m mapping to get at the grant table,
> > so somewhere has to be found in the physical address space where it can
> > go.
> > 
> > In theory a guest kernel could try and find some io space which it isn't
> > using but to help them out we give them a hint.
> > 
> > For x86 HVM the IOBAR of the platform device as a handy place to put the
> > hint, for ARM domU we have a defined region in our address map which we
> > use and communicate via device tree.
> > 
> > But for ARM dom0 the physical address space layout follows the host,
> > figuring out a free bit of space is a bit tricky so each platform is
> > currently required (except some don't and there is a broken default) to
> > say where is a safe place to use (i.e. a person has to read the
> > datasheet and figure it out).
> 
> Linux x86 PVH dom0 and domU uses ballooned pages and they're vmap()'d in
> into a contiguous bit of virtual address space.
> 
> Perhaps something similar could be considered for ARM in the future?
> This would be common code between ARM and x86 PVH.

That would be good, yes.

Unfortunately the provision of this "hint" region is now part of our
ABI, so we can't just stop providing it, similar to how if PVHVM x86
switched to using ballooned pages you couldn't just remove the platform
devices IO BAR (not that it causes too much pain on x86, so you likely
wouldn't bother anyway).

Although there maybe migration paths which we can consider in both
cases.

Ian.

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

[Julien Grall]

Hi Ian,

On 12/09/14 02:54, Ian Campbell wrote:
> On Thu, 2014-09-11 at 13:11 -0700, Julien Grall wrote:
>> Hi Ian,
>>
>> On 11/09/14 08:21, Ian Campbell wrote:
>>> 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.
>>
>> Dropping the default value sounds a good solution.
>>
>>> Or maybe we should search for an unused hole in the dom0 RAM space?
>>
>> I'm not sure to understand your suggestion here. Did you mean that as
>> DOM0 will never use all the host RAM, we could find a space in the RAM
>> to use for grant table?
>
> Something like that, yes.
>
> In fact it occurred to me last night that that since the gnttab needs to
> be populate anyway we could allocate it early in the case of 1:1 dom0
> and then use that address as a 1:1 value for the gnttab IPA passed to
> dom0.
>
> For non-1:1 dom0 we could safely put it right over the dom0 memory
> allocation, which as you observe is never going to be all of host ram
> (since Xen must be using something...)
>
> In fact, maybe using Xen's own 2M IPA would work everywhere.

The default size of the grant table can be changed via the command line. 
This is something we don't handle actually.

So a "malloc" would be the best solution here.

Regards,

-- 
Julien Grall

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

[Ian Campbell]

On Thu, 2014-09-11 at 16:21 +0100, Ian Campbell wrote:

I think we should apply this workaround for 4.5 and do things properly
for 4.6. Any thoughts?

> 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?
> ---
>  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
>  
>  /*

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

[Stefano Stabellini]

On Tue, 4 Nov 2014, Ian Campbell wrote:
> On Thu, 2014-09-11 at 16:21 +0100, Ian Campbell wrote:
> 
> I think we should apply this workaround for 4.5 and do things properly
> for 4.6. Any thoughts?
 
I agree with you


> > 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?
> > ---
> >  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
> >  
> >  /*
> 
> 

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

[Julien Grall]

On 09/11/2014 04:21 PM, 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

NIT: corresponds

> wired up to anything).
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

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

[Julien Grall]

On 11/04/2014 10:20 AM, Stefano Stabellini wrote:
> On Tue, 4 Nov 2014, Ian Campbell wrote:
>> On Thu, 2014-09-11 at 16:21 +0100, Ian Campbell wrote:
>>
>> I think we should apply this workaround for 4.5 and do things properly
>> for 4.6. Any thoughts?
>  
> I agree with you

+1

Regards,

-- 
Julien Grall

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

[Konrad Rzeszutek Wilk]

On Tue, Nov 04, 2014 at 12:48:15PM +0000, Julien Grall wrote:
> On 11/04/2014 10:20 AM, Stefano Stabellini wrote:
> > On Tue, 4 Nov 2014, Ian Campbell wrote:
> >> On Thu, 2014-09-11 at 16:21 +0100, Ian Campbell wrote:
> >>
> >> I think we should apply this workaround for 4.5 and do things properly
> >> for 4.6. Any thoughts?
> >  
> > I agree with you
> 
> +1


/me unrolls the first-bandaid and puts it on Xen and signs it with:
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Regards,
> 
> -- 
> Julien Grall

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

[Ian Campbell]

On Tue, 2014-11-04 at 12:20 -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 04, 2014 at 12:48:15PM +0000, Julien Grall wrote:
> > On 11/04/2014 10:20 AM, Stefano Stabellini wrote:
> > > On Tue, 4 Nov 2014, Ian Campbell wrote:
> > >> On Thu, 2014-09-11 at 16:21 +0100, Ian Campbell wrote:
> > >>
> > >> I think we should apply this workaround for 4.5 and do things properly
> > >> for 4.6. Any thoughts?
> > >  
> > > I agree with you
> > 
> > +1
> 
> 
> /me unrolls the first-bandaid and puts it on Xen and signs it with:
> Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thanks, applied!

Ian.