Xen devel - Nov 2013 - [PATCH] xen/arm: Don't call init_domheap_page with an empty range

When an initrd is given to xen by U-boot, it will reserve the memory in
the device tree.
In this case, when xen decides to free unused memory, dt_unreserved_regions
will call init_domheap_page with the start and the end of range equals. But
the latter assumes that (start > end), if not Xen will hang because the
number of pages is equals to (unsigned)-1.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 1081b43..5794a03 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -173,7 +173,8 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
         }
     }
 
-    cb(s, e);
+    if ( s != e )
+        cb(s, e);
 }
 
 void __init discard_initial_modules(void)
-- 
1.8.3.1

On Tue, 2013-11-12 at 21:42 +0000, Julien Grall wrote:
> When an initrd is given to xen by U-boot, it will reserve the memory in
> the device tree.
> In this case, when xen decides to free unused memory, dt_unreserved_regions
> will call init_domheap_page with the start and the end of range equals. But
> the latter assumes that (start > end), if not Xen will hang because the
> number of pages is equals to (unsigned)-1.
init_xenheap_pages starts with "if (pe<=ps) return". I think
init_domheap_pages usefully do the same.

If we are to fix this in dt_unreserved_regions it should be with < or
whatever not !=.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/setup.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 1081b43..5794a03 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -173,7 +173,8 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
>          }
>      }
>  
> -    cb(s, e);
> +    if ( s != e )
> +        cb(s, e);
>  }
>  
>  void __init discard_initial_modules(void)

On 11/13/2013 10:45 AM, Ian Campbell wrote:
> On Tue, 2013-11-12 at 21:42 +0000, Julien Grall wrote:
>> When an initrd is given to xen by U-boot, it will reserve the memory in
>> the device tree.
>> In this case, when xen decides to free unused memory,
dt_unreserved_regions
>> will call init_domheap_page with the start and the end of range equals.
But
>> the latter assumes that (start > end), if not Xen will hang because
the
>> number of pages is equals to (unsigned)-1.
>
> init_xenheap_pages starts with "if (pe<=ps) return". I think
> init_domheap_pages usefully do the same.
Actually, init_domheap_pages doesn''t contain this fix. I will rework 
this patch to move the check in this function.

-- 
Julien Grall

On Wed, 2013-11-13 at 12:59 +0000, Julien Grall wrote:
> 
> On 11/13/2013 10:45 AM, Ian Campbell wrote:
> > On Tue, 2013-11-12 at 21:42 +0000, Julien Grall wrote:
> >> When an initrd is given to xen by U-boot, it will reserve the
memory in
> >> the device tree.
> >> In this case, when xen decides to free unused memory,
dt_unreserved_regions
> >> will call init_domheap_page with the start and the end of range
equals. But
> >> the latter assumes that (start > end), if not Xen will hang
because the
> >> number of pages is equals to (unsigned)-1.
> >
> > init_xenheap_pages starts with "if (pe<=ps) return". I
think
> > init_domheap_pages usefully do the same.
oops:                 ^could 
> Actually, init_domheap_pages doesn''t contain this fix.
Yeah, I missed a word out, sorry!
>I will rework  this patch to move the check in this function.
Thanks!

Ian.