Re: [Xen-devel] [PATCH 5/8] xen/vmap: allow vmap() to be called during early boot

From: Julien Grall <julien@xen.org>
To: David Woodhouse <dwmw2@infradead.org>, xen-devel@lists.xenproject.org
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Varad Gautam" <vrd@amazon.de>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Hongyan Xia" <hongyxia@amazon.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Paul Durrant" <pdurrant@amazon.co.uk>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 5/8] xen/vmap: allow vmap() to be called during early boot
Date: Sun, 9 Feb 2020 18:19:32 +0000	[thread overview]
Message-ID: <717d3ef9-c7eb-572d-f891-77fbd6fd1781@xen.org> (raw)
In-Reply-To: <d58a4a99c83875b37f07f6e52d25ec943d35a554.camel@infradead.org>

Hi,

Answring back to this thread as well, so it is easier to track.

On 03/02/2020 16:37, David Woodhouse wrote:
> On Mon, 2020-02-03 at 14:00 +0000, Julien Grall wrote:
>> Hi David,
>>
>> On 01/02/2020 00:33, David Woodhouse wrote:
>>> From: David Woodhouse <dwmw@amazon.co.uk>
>>
>> I am a bit concerned with this change, particularly the consequence this
>> have for the page-tables. There is an assumption that intermediate
>> page-tables allocated via the boot allocator will never be freed.
>>
>> On x86, a call to vunmap() will not free page-tables, but a subsequent
>> call to vmap() may free it depending on the mapping size. So we would
>> call free_domheap_pages() rather than init_heap_pages().
>>
>> I am not entirely sure what is the full consequence, but I think this is
>> a call for investigation and write it down a summary in the commit message.
> 
> This isn't just about page tables, right? It's about *any* allocation
> given out by the boot allocator, being freed with free_heap_pages() ?

That's correct.

> 
> Given the amount of code that has conditionals in both alloc and free
> paths along the lines of…
> 
>    if (system_state > SYS_STATE_boot)
>        use xenheap
>    else
>        use boot allocator
> 
> … I'm not sure I'd really trust the assumption that such a thing never
> happens; that no pages are ever allocated from the boot allocator and
> then freed into the heap.

I believe this assumption holds on Arm because page-tables are never 
freed so far. The only other case is in the ACPI code 
(acpi_os_free_memory()) but I think it would result to a leak instead 
(see is_xmalloc_memory()).

I hadn't realized that this assumption was not holding on x86 :(. 
Actually, from the discussion on the MFN 0 thread, it looks like x86 is 
abusing quite a few interface in page alloc.

So if this is nothing new, this probably yet another good future 
clean-up/hardening as long as the assumption hold outside of x86.

Cheers,

-- 
Julien Grall

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