xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "xen/arm: do not relocate Xen outside of visible RAM"
@ 2016-10-24 22:29 Sameer Goel
  2016-10-24 23:14 ` Stefano Stabellini
  0 siblings, 1 reply; 4+ messages in thread
From: Sameer Goel @ 2016-10-24 22:29 UTC (permalink / raw)
  To: xen-devel, ian.campbell
  Cc: Sameer Goel, Julien Grall, Stefano Stabellini, shankerd

This reverts commit db92b1ac55cd5e193ae22b0b6f01fb47bc9e5d2f.

There does not seem to be a restriction on non contiguous memory anymore . So,
reverting this change, so that Xen is placed at the end of the useable
system RAM even if the partitions are not contiguous.

Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
---
The load restriction placed in the above reverted patch resulted in a crash
when booting DOM0 on a Qualcomm platform with non contiguous system RAM.

(XEN) DOM0: [ 0.000000] bootmem alloc of 64 bytes failed!
(XEN) DOM0: [ 0.000000] Kernel panic - not syncing: Out of memory
(XEN) DOM0: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.7.0-rc7+ #19
(XEN) DOM0: [ 0.000000] Hardware name: (null) (DT)
(XEN) DOM0: [ 0.000000] Call trace:
(XEN) DOM0: [ 0.000000] [<ffff000008088854>] dump_backtrace+0x0/0x1e4
(XEN) DOM0: [ 0.000000] [<ffff000008088a5c>] show_stack+0x24/0x2c
(XEN) DOM0: [ 0.000000] [<ffff000008452004>] dump_stack+0x8c/0xb0
(XEN) DOM0: [ 0.000000] [<ffff00000819ee78>] panic+0x128/0x268
(XEN) DOM0: [ 0.000000] [<ffff00000902c018>] __alloc_bootmem_low+0x40/0x4c
(XEN) DOM0: [ 0.000000] [<ffff000009012adc>] setup_arch+0x2d8/0x560
(XEN) DOM0: [ 0.000000] [<ffff00000901084c>] start_kernel+0x60/0x3b4
(XEN) DOM0: [ 0.000000] [<ffff0000090101bc>] __primary_switched+0x30/0x74
(XEN) DOM0: [ 0.000000] ---[ end Kernel panic - not syncing: Out of memory

The root cause for the crash was >4GB difference between the arm grant table
(lower address) and the kernel start address. The kernel sees the grant table
region as the start of system RAM.

Since, the grant table is a reuse of the text region of Xen image this issue
would not be seen if Xen is loaded high enough in memory.

 xen/arch/arm/setup.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 38eb888..1678871 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -392,25 +392,17 @@ static paddr_t __init get_xen_paddr(void)
 {
     struct meminfo *mi = &bootinfo.mem;
     paddr_t min_size;
-    paddr_t paddr = 0, last_end;
+    paddr_t paddr = 0;
     int i;
 
     min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
 
-    last_end = mi->bank[0].start;
-
     /* Find the highest bank with enough space. */
     for ( i = 0; i < mi->nr_banks; i++ )
     {
         const struct membank *bank = &mi->bank[i];
         paddr_t s, e;
 
-        /* We can only deal with contiguous memory at the moment */
-        if ( last_end != bank->start )
-            break;
-
-        last_end = bank->start + bank->size;
-
         if ( bank->size >= min_size )
         {
             e = consider_modules(bank->start, bank->start + bank->size,
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Revert "xen/arm: do not relocate Xen outside of visible RAM"
  2016-10-24 22:29 [PATCH] Revert "xen/arm: do not relocate Xen outside of visible RAM" Sameer Goel
@ 2016-10-24 23:14 ` Stefano Stabellini
  2016-10-25 14:51   ` Ruigrok, Richard
  2016-10-25 16:05   ` Goel, Sameer
  0 siblings, 2 replies; 4+ messages in thread
From: Stefano Stabellini @ 2016-10-24 23:14 UTC (permalink / raw)
  To: Sameer Goel
  Cc: xen-devel, ian.campbell, Julien Grall, Stefano Stabellini, shankerd

On Mon, 24 Oct 2016, Sameer Goel wrote:
> This reverts commit db92b1ac55cd5e193ae22b0b6f01fb47bc9e5d2f.
> 
> There does not seem to be a restriction on non contiguous memory anymore . So,
> reverting this change, so that Xen is placed at the end of the useable
> system RAM even if the partitions are not contiguous.
> 
> Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
> ---
> The load restriction placed in the above reverted patch resulted in a crash
> when booting DOM0 on a Qualcomm platform with non contiguous system RAM.

Hi Sameer,

Thanks for the patch. It might be worth to point to
2d02b05c77fc5e7c76bf6f112db84bbaa44fdcb5 in the commit message, which I
think is the commit that made non-contiguous ram regions work properly.

This is not a fix for a currently supported platform, right? This is for
a new platform?

I am asking because if this is for a new platform, I would prefer to
consider the patch for the next development cycle, Xen 4.9. We have just
tagged 4.8-RC3, it would be best to avoid changes with potentially
harmful side effects at this stage (theoretically this change would need
to validated on every supported platforms, which at this stage is
unrealistic).



> (XEN) DOM0: [ 0.000000] bootmem alloc of 64 bytes failed!
> (XEN) DOM0: [ 0.000000] Kernel panic - not syncing: Out of memory
> (XEN) DOM0: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.7.0-rc7+ #19
> (XEN) DOM0: [ 0.000000] Hardware name: (null) (DT)
> (XEN) DOM0: [ 0.000000] Call trace:
> (XEN) DOM0: [ 0.000000] [<ffff000008088854>] dump_backtrace+0x0/0x1e4
> (XEN) DOM0: [ 0.000000] [<ffff000008088a5c>] show_stack+0x24/0x2c
> (XEN) DOM0: [ 0.000000] [<ffff000008452004>] dump_stack+0x8c/0xb0
> (XEN) DOM0: [ 0.000000] [<ffff00000819ee78>] panic+0x128/0x268
> (XEN) DOM0: [ 0.000000] [<ffff00000902c018>] __alloc_bootmem_low+0x40/0x4c
> (XEN) DOM0: [ 0.000000] [<ffff000009012adc>] setup_arch+0x2d8/0x560
> (XEN) DOM0: [ 0.000000] [<ffff00000901084c>] start_kernel+0x60/0x3b4
> (XEN) DOM0: [ 0.000000] [<ffff0000090101bc>] __primary_switched+0x30/0x74
> (XEN) DOM0: [ 0.000000] ---[ end Kernel panic - not syncing: Out of memory
> 
> The root cause for the crash was >4GB difference between the arm grant table
> (lower address) and the kernel start address. The kernel sees the grant table
> region as the start of system RAM.
> 
> Since, the grant table is a reuse of the text region of Xen image this issue
> would not be seen if Xen is loaded high enough in memory.
> 
>  xen/arch/arm/setup.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 38eb888..1678871 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -392,25 +392,17 @@ static paddr_t __init get_xen_paddr(void)
>  {
>      struct meminfo *mi = &bootinfo.mem;
>      paddr_t min_size;
> -    paddr_t paddr = 0, last_end;
> +    paddr_t paddr = 0;
>      int i;
>  
>      min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
>  
> -    last_end = mi->bank[0].start;
> -
>      /* Find the highest bank with enough space. */
>      for ( i = 0; i < mi->nr_banks; i++ )
>      {
>          const struct membank *bank = &mi->bank[i];
>          paddr_t s, e;
>  
> -        /* We can only deal with contiguous memory at the moment */
> -        if ( last_end != bank->start )
> -            break;
> -
> -        last_end = bank->start + bank->size;
> -
>          if ( bank->size >= min_size )
>          {
>              e = consider_modules(bank->start, bank->start + bank->size,
> -- 
> Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Revert "xen/arm: do not relocate Xen outside of visible RAM"
  2016-10-24 23:14 ` Stefano Stabellini
@ 2016-10-25 14:51   ` Ruigrok, Richard
  2016-10-25 16:05   ` Goel, Sameer
  1 sibling, 0 replies; 4+ messages in thread
From: Ruigrok, Richard @ 2016-10-25 14:51 UTC (permalink / raw)
  To: Stefano Stabellini, Sameer Goel
  Cc: Julien Grall, xen-devel, ian.campbell, shankerd



On 10/24/2016 5:14 PM, Stefano Stabellini wrote:
> On Mon, 24 Oct 2016, Sameer Goel wrote:
>> This reverts commit db92b1ac55cd5e193ae22b0b6f01fb47bc9e5d2f.
>>
>> There does not seem to be a restriction on non contiguous memory anymore . So,
>> reverting this change, so that Xen is placed at the end of the useable
>> system RAM even if the partitions are not contiguous.
>>
>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
>> ---
>> The load restriction placed in the above reverted patch resulted in a crash
>> when booting DOM0 on a Qualcomm platform with non contiguous system RAM.
> Hi Sameer,
>
> Thanks for the patch. It might be worth to point to
> 2d02b05c77fc5e7c76bf6f112db84bbaa44fdcb5 in the commit message, which I
> think is the commit that made non-contiguous ram regions work properly.
>
> This is not a fix for a currently supported platform, right? This is for
> a new platform?
>
> I am asking because if this is for a new platform, I would prefer to
> consider the patch for the next development cycle, Xen 4.9. We have just
> tagged 4.8-RC3, it would be best to avoid changes with potentially
> harmful side effects at this stage (theoretically this change would need
> to validated on every supported platforms, which at this stage is
> unrealistic).
>
Hi Sameer, Stefano,

Yes this is for a new Qualcomm platform that is not fully supported 
yet.  I think it's fine to push this to Xen 4.9.

>
>> (XEN) DOM0: [ 0.000000] bootmem alloc of 64 bytes failed!
>> (XEN) DOM0: [ 0.000000] Kernel panic - not syncing: Out of memory
>> (XEN) DOM0: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.7.0-rc7+ #19
>> (XEN) DOM0: [ 0.000000] Hardware name: (null) (DT)
>> (XEN) DOM0: [ 0.000000] Call trace:
>> (XEN) DOM0: [ 0.000000] [<ffff000008088854>] dump_backtrace+0x0/0x1e4
>> (XEN) DOM0: [ 0.000000] [<ffff000008088a5c>] show_stack+0x24/0x2c
>> (XEN) DOM0: [ 0.000000] [<ffff000008452004>] dump_stack+0x8c/0xb0
>> (XEN) DOM0: [ 0.000000] [<ffff00000819ee78>] panic+0x128/0x268
>> (XEN) DOM0: [ 0.000000] [<ffff00000902c018>] __alloc_bootmem_low+0x40/0x4c
>> (XEN) DOM0: [ 0.000000] [<ffff000009012adc>] setup_arch+0x2d8/0x560
>> (XEN) DOM0: [ 0.000000] [<ffff00000901084c>] start_kernel+0x60/0x3b4
>> (XEN) DOM0: [ 0.000000] [<ffff0000090101bc>] __primary_switched+0x30/0x74
>> (XEN) DOM0: [ 0.000000] ---[ end Kernel panic - not syncing: Out of memory
>>
>> The root cause for the crash was >4GB difference between the arm grant table
>> (lower address) and the kernel start address. The kernel sees the grant table
>> region as the start of system RAM.
>>
>> Since, the grant table is a reuse of the text region of Xen image this issue
>> would not be seen if Xen is loaded high enough in memory.
>>
>>   xen/arch/arm/setup.c | 10 +---------
>>   1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 38eb888..1678871 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -392,25 +392,17 @@ static paddr_t __init get_xen_paddr(void)
>>   {
>>       struct meminfo *mi = &bootinfo.mem;
>>       paddr_t min_size;
>> -    paddr_t paddr = 0, last_end;
>> +    paddr_t paddr = 0;
>>       int i;
>>   
>>       min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
>>   
>> -    last_end = mi->bank[0].start;
>> -
>>       /* Find the highest bank with enough space. */
>>       for ( i = 0; i < mi->nr_banks; i++ )
>>       {
>>           const struct membank *bank = &mi->bank[i];
>>           paddr_t s, e;
>>   
>> -        /* We can only deal with contiguous memory at the moment */
>> -        if ( last_end != bank->start )
>> -            break;
>> -
>> -        last_end = bank->start + bank->size;
>> -
>>           if ( bank->size >= min_size )
>>           {
>>               e = consider_modules(bank->start, bank->start + bank->size,
>> -- 
>> Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
>>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Revert "xen/arm: do not relocate Xen outside of visible RAM"
  2016-10-24 23:14 ` Stefano Stabellini
  2016-10-25 14:51   ` Ruigrok, Richard
@ 2016-10-25 16:05   ` Goel, Sameer
  1 sibling, 0 replies; 4+ messages in thread
From: Goel, Sameer @ 2016-10-25 16:05 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Julien Grall, xen-devel, Ian Campbell, shankerd

Makes sense. I will update the commit message. Yes, it is ok to push 
this out to Xen 4.9. Appreciate the feedback.

On 10/24/2016 5:14 PM, Stefano Stabellini wrote:
> On Mon, 24 Oct 2016, Sameer Goel wrote:
>> This reverts commit db92b1ac55cd5e193ae22b0b6f01fb47bc9e5d2f.
>>
>> There does not seem to be a restriction on non contiguous memory anymore . So,
>> reverting this change, so that Xen is placed at the end of the useable
>> system RAM even if the partitions are not contiguous.
>>
>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
>> ---
>> The load restriction placed in the above reverted patch resulted in a crash
>> when booting DOM0 on a Qualcomm platform with non contiguous system RAM.
>
> Hi Sameer,
>
> Thanks for the patch. It might be worth to point to
> 2d02b05c77fc5e7c76bf6f112db84bbaa44fdcb5 in the commit message, which I
> think is the commit that made non-contiguous ram regions work properly.
>
> This is not a fix for a currently supported platform, right? This is for
> a new platform?
>
> I am asking because if this is for a new platform, I would prefer to
> consider the patch for the next development cycle, Xen 4.9. We have just
> tagged 4.8-RC3, it would be best to avoid changes with potentially
> harmful side effects at this stage (theoretically this change would need
> to validated on every supported platforms, which at this stage is
> unrealistic).
>
>
>
>> (XEN) DOM0: [ 0.000000] bootmem alloc of 64 bytes failed!
>> (XEN) DOM0: [ 0.000000] Kernel panic - not syncing: Out of memory
>> (XEN) DOM0: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.7.0-rc7+ #19
>> (XEN) DOM0: [ 0.000000] Hardware name: (null) (DT)
>> (XEN) DOM0: [ 0.000000] Call trace:
>> (XEN) DOM0: [ 0.000000] [<ffff000008088854>] dump_backtrace+0x0/0x1e4
>> (XEN) DOM0: [ 0.000000] [<ffff000008088a5c>] show_stack+0x24/0x2c
>> (XEN) DOM0: [ 0.000000] [<ffff000008452004>] dump_stack+0x8c/0xb0
>> (XEN) DOM0: [ 0.000000] [<ffff00000819ee78>] panic+0x128/0x268
>> (XEN) DOM0: [ 0.000000] [<ffff00000902c018>] __alloc_bootmem_low+0x40/0x4c
>> (XEN) DOM0: [ 0.000000] [<ffff000009012adc>] setup_arch+0x2d8/0x560
>> (XEN) DOM0: [ 0.000000] [<ffff00000901084c>] start_kernel+0x60/0x3b4
>> (XEN) DOM0: [ 0.000000] [<ffff0000090101bc>] __primary_switched+0x30/0x74
>> (XEN) DOM0: [ 0.000000] ---[ end Kernel panic - not syncing: Out of memory
>>
>> The root cause for the crash was >4GB difference between the arm grant table
>> (lower address) and the kernel start address. The kernel sees the grant table
>> region as the start of system RAM.
>>
>> Since, the grant table is a reuse of the text region of Xen image this issue
>> would not be seen if Xen is loaded high enough in memory.
>>
>>  xen/arch/arm/setup.c | 10 +---------
>>  1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 38eb888..1678871 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -392,25 +392,17 @@ static paddr_t __init get_xen_paddr(void)
>>  {
>>      struct meminfo *mi = &bootinfo.mem;
>>      paddr_t min_size;
>> -    paddr_t paddr = 0, last_end;
>> +    paddr_t paddr = 0;
>>      int i;
>>
>>      min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
>>
>> -    last_end = mi->bank[0].start;
>> -
>>      /* Find the highest bank with enough space. */
>>      for ( i = 0; i < mi->nr_banks; i++ )
>>      {
>>          const struct membank *bank = &mi->bank[i];
>>          paddr_t s, e;
>>
>> -        /* We can only deal with contiguous memory at the moment */
>> -        if ( last_end != bank->start )
>> -            break;
>> -
>> -        last_end = bank->start + bank->size;
>> -
>>          if ( bank->size >= min_size )
>>          {
>>              e = consider_modules(bank->start, bank->start + bank->size,
>> --
>> Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
>>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-10-25 16:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-24 22:29 [PATCH] Revert "xen/arm: do not relocate Xen outside of visible RAM" Sameer Goel
2016-10-24 23:14 ` Stefano Stabellini
2016-10-25 14:51   ` Ruigrok, Richard
2016-10-25 16:05   ` Goel, Sameer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).