Re: [Xen-devel] [PATCH 2/2] efi/boot: Don't free ebmalloc area at all

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/02/17 17:41, Daniel Kiper wrote:
> On Tue, Feb 28, 2017 at 04:08:35PM +0000, Andrew Cooper wrote:
>> On 28/02/17 16:03, Jan Beulich wrote:
>>>>>> On 28.02.17 at 16:20, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> Freeing part of the BSS back for general use proves to be problematic.  It 
>>>> is
>>>> not accounted for in xen_in_range(), causing errors when constructing the
>>>> IOMMU tables, resulting in a failure to boot.
>>>>
>>>> Other smaller issues are that tboot treats the entire BSS as hypervisor 
>>>> data,
>>>> creating and checking a MAC of it on S3, and that, by being 1MB in size,
>>>> freeing it guarentees to shatter the hypervisor superpage mappings.
>>>>
>>>> Judging by the content stored in it, 1MB is overkill on size.  Drop it to a
>>>> more-reasonable 32kB and keep the entire buffer around after boot.
>>> Well, that's just because right now there's only a single user. The
>>> reason I refused Daniel making it smaller than its predecessor is
>>> that we can't really give a good estimate of how much data may
>>> need storing there: The memory map can have hundreds of entries
>>> and command lines for modules may also be almost arbitrarily long.
>>>
>>> What I don't recall, Daniel: Why was it that we can't use EFI boot
>>> services allocations here?
>> From the original commit message,
>>
>>     1) We could use native EFI allocation functions (e.g. AllocatePool()
>>        or AllocatePages()) to get memory chunk. However, later (somewhere
>>        in __start_xen()) we must copy its contents to safe place or reserve
>>        it in e820 memory map and map it in Xen virtual address space. This
>>        means that the code referring to Xen command line, loaded modules and
>>        EFI memory map, mostly in __start_xen(), will be further complicated
>>        and diverge from legacy BIOS cases. Additionally, both former things
>>        have to be placed below 4 GiB because their addresses are stored in
>>        multiboot_info_t structure which has 32-bit relevant members.
>>
>>
>> One way or another, if we don't want to shatter superpages, we either
>> must keep the entire allocation, or copy the content out later into a
>> smaller location once other heap facilities are available.
> Hmmm... Why BSS free "shatter superpages" and .init.* sections free does not?

Xen is purposefully laid out like this:

.text, 2M aligned, R/X
.rodata, 2M aligned, R/NX
.init.*, 2M aligned, R/W/X (reclaimed)
.data + .bss, 2M aligned, R/W/NX

(In reality there is a syslinux bug which caused me to revert the 2M
alignment in non-EFI builds, so we are still using 4k alignment, but I
need to find a way to work around this and move back to enforcing the 2M
alignment.)

When .init gets reclaimed, this leaves a (deliberate) hole which is all
2M aligned, which has no impact on the adjacent 2M superpages.

When ebmalloc gets reclaimed, it must shatter one or two superpages
mapping the .data/.bss section.

> In regards to tboot I think that we should just take into account during
> calculation ebmalloc_mem allocated part only. Maybe move of ebmalloc_mem
> region to the end of BSS would help somehow here.

At the moment, all the ranges of Xen VA space are bounded by linker symbols.

It is certainly possible to could account for the ebmalloc object in the
middle of the BSS, but it is turning into a layering violation.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel