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

[Juergen Gross <jgross@xxxxxxxx>]

On 17.09.19 14:31, Julien Grall wrote:
> (+ 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.
Depends on the patch size and the risk to take it. But in general I'd
prefer a simple solution, especially in order to get a push rather
sooner than later.


Juergen

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