[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Juergen,

On 9/17/19 2:12 PM, Juergen Gross wrote:
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.

I have just sent a patch to fix the breakage:

https://patchwork.kernel.org/patch/11148883/

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.