Re: [Xen-devel] dom/xen heap and boot allocator (WAS Re: [xen-unstable-smoke test] 141333: regressions - FAIL)

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.09.2019 19:51, Julien Grall wrote:
> Hi,
> 
> On 9/15/19 3:09 PM, osstest service owner wrote:
>> flight 141333 xen-unstable-smoke real [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/141333/
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>   test-armhf-armhf-xl           7 xen-boot                 fail REGR. vs. 
>> 141253
> 
> Osstest does not provide the stack trace as the crash happen before the 
> console is setup, but I managed to reproduce it locally:
> 
> (XEN) Assertion 'is_xen_heap_mfn(maddr_to_mfn(ma))' failed at 
> /home/julieng/works/xen/xen/include/asm/mm.h:250
> (XEN) ----[ Xen-4.13-unstable  arm32  debug=y   Not tainted ]----
> 
> [...]
> 
> (XEN) Xen call trace:
> (XEN)    [<002992c0>] page_alloc.c#bootmem_region_add+0xf8/0x17c (PC)
> (XEN)    [<002995ac>] init_boot_pages+0x8c/0x1a0 (LR)
> (XEN)    [<002995ac>] init_boot_pages+0x8c/0x1a0
> (XEN)    [<002acc2c>] dt_unreserved_regions+0x268/0x284
> (XEN)    [<002ad8e0>] start_xen+0x5c8/0xe90
> (XEN)    [<00200098>] arm32/head.o#primary_switched+0x4/0x10
> 
> This is happening because of commit 6e3e771203 "xen/arm: setup: Relocate 
> the Device-Tree later on in the boot". Since this patch, none of xenheap 
> memory is given to the boot allocator.

So this change wasn't tested on 32-bit Arm at all then before
committing?

> The boot allocator is bootstrapping itself and re-use a page from the 
> first added region. If this region is not a xenheap region, then it will 
> crash when calling mfn_to_virt() or later on because the virtual address 
> is not mapped in memory.
> 
> The description of the boot allocator in page_alloc.c leads to think 
> that only domheap memory may be given to the boot allocator.
> 
> Furthermore, as the boot allocator may have domheap page, it means that 
> calling mfn_to_virt(mfn_x(alloc_boot_pages(...)) may not work when 
> CONFIG_SEPARATE_XENHEAP=y.
> 
> It feels to me that imposing to give a xenheap page to the boot 
> allocator is quite ugly. As the boot allocator will be used in most of 
> the case, statically allocating bootmem_region_list maybe the best. Any 
> thoughts?

I've gone back to 4.2 code, where 32-bit x86 was still supported. There
we had

#if defined(CONFIG_X86_32)
    xenheap_initial_phys_start = (PFN_UP(__pa(&_end)) + 1) << PAGE_SHIFT;
    /* Must pass a single mapped page for populating bootmem_region_list. */
    init_boot_pages(__pa(&_end), xenheap_initial_phys_start);
    xenheap_phys_end = DIRECTMAP_MBYTES << 20;
#else

i.e. it was clearly intended for the first thing given to the boot
allocator to be a Xen heap page.

Beyond this no x86 code that wasn't x86-64 specific depended on there
only coming direct-mapped memory back out of alloc_boot_pages().

But yes, a static allocation may make sense (and then arguably it may
not even need to be a full page) - commit 0409e29e2b, which is what
had introduced bootmem_region_list, doesn't (as was the common case
back then) have any description at all, i.e. there's no way to know
why it wasn't done this way in the first place.

Jan

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