[Xen-devel] [PATCH] xen/arm32: setup: Give a xenheap page to the boot allocator

[Xen-devel] [PATCH] xen/arm32: setup: Give a xenheap page to the boot allocator

[Julien Grall]

After commit 6e3e771203 "xen/arm: setup: Relocate the Device-Tree later on
in the boot", the boot allocator will not receive any xenheap page (i.e.
mapped page) on Arm32.

However, the boot allocator implicitely rely on having the first page
already mapped and therefore result to break boot on Arm32.

The easiest way for now is to give a xenheap page to the boot allocator.
We may want to rethink the interface in the future.

Fixes: 6e3e771203 ('xen/arm: setup: Relocate the Device-Tree later on in the boot')
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/setup.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ebbfad94e4..e6ddefb5cf 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -593,6 +593,7 @@ static void __init setup_mm(void)
     unsigned long heap_pages, xenheap_pages, domheap_pages;
     int i;
     const uint32_t ctr = READ_CP32(CTR);
+    mfn_t boot_mfn_start, boot_mfn_end;
 
     if ( !bootinfo.mem.nr_banks )
         panic("No memory bank\n");
@@ -665,6 +666,11 @@ static void __init setup_mm(void)
 
     setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
 
+    /* We need a single mapped page for populating bootmem_region_list. */
+    boot_mfn_start = mfn_add(xenheap_mfn_end, -1);
+    boot_mfn_end = xenheap_mfn_end;
+    init_boot_pages(mfn_to_maddr(boot_mfn_start), mfn_to_maddr(boot_mfn_end));
+
     /* Add non-xenheap memory */
     for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
     {
@@ -710,7 +716,7 @@ static void __init setup_mm(void)
 
     /* Add xenheap memory that was not already added to the boot allocator. */
     init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start),
-                       mfn_to_maddr(xenheap_mfn_end));
+                       mfn_to_maddr(boot_mfn_end));
 }
 #else /* CONFIG_ARM_64 */
 static void __init setup_mm(void)
-- 
2.11.0


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

Re: [Xen-devel] [PATCH] xen/arm32: setup: Give a xenheap page to the boot allocator

[Jan Beulich]

On 17.09.2019 15:21, Julien Grall wrote:
> After commit 6e3e771203 "xen/arm: setup: Relocate the Device-Tree later on
> in the boot", the boot allocator will not receive any xenheap page (i.e.
> mapped page) on Arm32.
> 
> However, the boot allocator implicitely rely on having the first page
> already mapped and therefore result to break boot on Arm32.
> 
> The easiest way for now is to give a xenheap page to the boot allocator.
> We may want to rethink the interface in the future.
> 
> Fixes: 6e3e771203 ('xen/arm: setup: Relocate the Device-Tree later on in the boot')
> Signed-off-by: Julien Grall <julien.grall@arm.com>

FWIW / in case it helps:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

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

Re: [Xen-devel] [PATCH] xen/arm32: setup: Give a xenheap page to the boot allocator

[Stefano Stabellini]

On Tue, 17 Sep 2019, Julien Grall wrote:
> After commit 6e3e771203 "xen/arm: setup: Relocate the Device-Tree later on
> in the boot", the boot allocator will not receive any xenheap page (i.e.
> mapped page) on Arm32.
> 
> However, the boot allocator implicitely rely on having the first page
> already mapped and therefore result to break boot on Arm32.
> 
> The easiest way for now is to give a xenheap page to the boot allocator.
> We may want to rethink the interface in the future.
> 
> Fixes: 6e3e771203 ('xen/arm: setup: Relocate the Device-Tree later on in the boot')
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/setup.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index ebbfad94e4..e6ddefb5cf 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -593,6 +593,7 @@ static void __init setup_mm(void)
>      unsigned long heap_pages, xenheap_pages, domheap_pages;
>      int i;
>      const uint32_t ctr = READ_CP32(CTR);
> +    mfn_t boot_mfn_start, boot_mfn_end;
>  
>      if ( !bootinfo.mem.nr_banks )
>          panic("No memory bank\n");
> @@ -665,6 +666,11 @@ static void __init setup_mm(void)
>  
>      setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
>  
> +    /* We need a single mapped page for populating bootmem_region_list. */
> +    boot_mfn_start = mfn_add(xenheap_mfn_end, -1);
> +    boot_mfn_end = xenheap_mfn_end;
> +    init_boot_pages(mfn_to_maddr(boot_mfn_start), mfn_to_maddr(boot_mfn_end));
> +
>      /* Add non-xenheap memory */
>      for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
>      {
> @@ -710,7 +716,7 @@ static void __init setup_mm(void)
>  
>      /* Add xenheap memory that was not already added to the boot allocator. */
>      init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start),
> -                       mfn_to_maddr(xenheap_mfn_end));
> +                       mfn_to_maddr(boot_mfn_end));

I can see what you are trying to do with this patch and it looks like
the right fix at the moment. However, shouldn't this last change:

  mfn_to_maddr(boot_mfn_start)


>  }
>  #else /* CONFIG_ARM_64 */
>  static void __init setup_mm(void)


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

Re: [Xen-devel] [PATCH] xen/arm32: setup: Give a xenheap page to the boot allocator

[Jan Beulich]

On 20.09.2019 00:49, Stefano Stabellini wrote:
> On Tue, 17 Sep 2019, Julien Grall wrote:
>> @@ -665,6 +666,11 @@ static void __init setup_mm(void)
>>  
>>      setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
>>  
>> +    /* We need a single mapped page for populating bootmem_region_list. */
>> +    boot_mfn_start = mfn_add(xenheap_mfn_end, -1);
>> +    boot_mfn_end = xenheap_mfn_end;
>> +    init_boot_pages(mfn_to_maddr(boot_mfn_start), mfn_to_maddr(boot_mfn_end));
>> +
>>      /* Add non-xenheap memory */
>>      for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
>>      {
>> @@ -710,7 +716,7 @@ static void __init setup_mm(void)
>>  
>>      /* Add xenheap memory that was not already added to the boot allocator. */
>>      init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start),
>> -                       mfn_to_maddr(xenheap_mfn_end));
>> +                       mfn_to_maddr(boot_mfn_end));
> 
> I can see what you are trying to do with this patch and it looks like
> the right fix at the moment. However, shouldn't this last change:
> 
>   mfn_to_maddr(boot_mfn_start)

Oh, indeed - when doing the review yesterday I thought I had
carefully compared with how things looked prior to the change
needing fixing up now, yet I didn't spot this (otherwise
obvious) difference to the original code.

Jan

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

Re: [Xen-devel] [PATCH] xen/arm32: setup: Give a xenheap page to the boot allocator

[Julien Grall]

Hi Stefano,

On 20/09/2019 09:48, Jan Beulich wrote:
> On 20.09.2019 00:49, Stefano Stabellini wrote:
>> On Tue, 17 Sep 2019, Julien Grall wrote:
>>> @@ -665,6 +666,11 @@ static void __init setup_mm(void)
>>>   
>>>       setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
>>>   
>>> +    /* We need a single mapped page for populating bootmem_region_list. */
>>> +    boot_mfn_start = mfn_add(xenheap_mfn_end, -1);
>>> +    boot_mfn_end = xenheap_mfn_end;
>>> +    init_boot_pages(mfn_to_maddr(boot_mfn_start), mfn_to_maddr(boot_mfn_end));
>>> +
>>>       /* Add non-xenheap memory */
>>>       for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
>>>       {
>>> @@ -710,7 +716,7 @@ static void __init setup_mm(void)
>>>   
>>>       /* Add xenheap memory that was not already added to the boot allocator. */
>>>       init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start),
>>> -                       mfn_to_maddr(xenheap_mfn_end));
>>> +                       mfn_to_maddr(boot_mfn_end));
>>
>> I can see what you are trying to do with this patch and it looks like
>> the right fix at the moment. However, shouldn't this last change:
>>
>>    mfn_to_maddr(boot_mfn_start)

Doh, yes it should. I will update the patch and resend it.

> 
> Oh, indeed - when doing the review yesterday I thought I had
> carefully compared with how things looked prior to the change
> needing fixing up now, yet I didn't spot this (otherwise
> obvious) difference to the original code.
> 
> Jan
> 

Cheers,

-- 
Julien Grall

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