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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.02.17 at 17:08, <andrew.cooper3@xxxxxxxxxx> 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.

Reading this again, I have to admit that I don't understand why any
copying or reserving would need to be done. We'd need to do runtime
allocations, sure, but I would have thought this goes without saying.

> 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.
> 
> If we are copying data out, we might as well use EFI heap facilities
> rather than rolling our own.

Well, copying data later won't work, as there are pointers stored to
the original allocation.

Jan


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