[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] efi/boot: Don't free ebmalloc area at all
On Tue, Feb 28, 2017 at 07:09:14PM +0000, Andrew Cooper wrote: > On 28/02/17 19:01, Daniel Kiper wrote: > > On Tue, Feb 28, 2017 at 05:58:26PM +0000, Andrew Cooper wrote: > >> 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 > > AIUI, the purpose is to have properly mapped (in terms of R/W/X/NX) > > sections. Right? > > When I did the original work, it was both to get proper pagetable > permissions, and to get all of the compiled bits of Xen living in 2M > superpages (which allows the entire hypervisor to operate in 5 TLB entries!) Thanks for explanation. > >> (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. > > Looking at this I do not have a better idea for fix off the top of my head. > > So, I have a feeling that we should drop free_ebmalloc_unused_mem() and > > leave > > comment around ebmalloc() why we do not reclaim unused ebmalloc_mem region. > > Should I post a patch or whether you would like to do it? > > The patch at the root of this thread is basically that. Yep, I know. > As a stopgap solution to unblock staging, I think I should drop the > adjustment of MB(1) to KB(32) and submit the patch. Sounds like a plan. > This satisfies Jan's request to not make the current buffer smaller than > it currently is, and will give us more time to discuss alternative > solutions, at the cost of wasting 1MB of RAM. (Not exactly breaking the > bank these days, but definitely something which should be fixed before > 4.9 ships.) Granted! Daniel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |