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

[Julien Grall <julien.grall@xxxxxxx>]

(+ Juergen)

Hi,

On 9/16/19 9:51 AM, Jan Beulich wrote:
> 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?
I forgot to test it before sending the patch on the ML :(.

> > 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 know realize we had similar comment in the arm32 code:

"Need a single mapped page for populating bootmem_region_list"

> 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.
It is probably too late for Xen 4.13 to move to a static list for 
bootmem_region_list (Juergen?), so I am thinking to fix up the arm32
code instead.

I can add the change for bootmem_region_list in my todo list for Xen 4.14.

Cheers,

--
Julien Grall

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