linux-kernel - Re: [PATCH 2/2 v2] x86/e820: Use much less memory for e820/e820

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2e5ecc7-0b8d-c453-984f-179cee692fa5@redhat.com>
Date:   Sun, 18 Sep 2016 20:11:53 +0200
From:   Denys Vlasenko <dvlasenk@...hat.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Andy Lutomirski <luto@...capital.net>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Brian Gerst <brgerst@...il.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2 v2] x86/e820: Use much less memory for
 e820/e820_saved, save up to 120k

On 09/18/2016 10:31 AM, Ingo Molnar wrote:
> * Denys Vlasenko <dvlasenk@...hat.com> wrote:
>> On 09/15/2016 09:04 AM, Ingo Molnar wrote:
>>> * Denys Vlasenko <dvlasenk@...hat.com> wrote:
>>>
>>>> The maximum size of e820 map array for EFI systems is defined as
>>>> E820_X_MAX (E820MAX + 3 * MAX_NUMNODES).
>>>>
>>>> In x86_64 defconfig, this ends up with E820_X_MAX = 320, e820 and e820_saved
>>>> are 6404 bytes each.
>>>>
>>>> With larger configs, for example Fedora kernels, E820_X_MAX = 3200, e820 and e820_saved
>>>> are 64004 bytes each. Most of this space is wasted. Typical machines have some 20-30
>>>> e820 areas at most.
>>>>
>>>> This patch turns e820 and e820_saved to pointers which initially point to __initdata
>>>> tables, of the same size as before.
>>>>
>>>> At the very end of setup_arch(), when we are done fiddling with these maps,
>>>> allocate smaller alloc_bootmem blocks, copy maps there, and change pointers.
>>>>
>>>> Run-tested.
>>>
>>>> +/*
>>>> + * Initial e820 and e820_saved are largish __initdata arrays.
>>>> + * Copy them to (usually much smaller) dynamically allocated area.
>>>> + * This is done after all tweaks we ever do to them.
>>>> + */
>>>> +__init void e820_reallocate_tables(void)
>>>> +{
>>>> +	struct e820map *n;
>>>> +	int size;
>>>> +
>>>> +	size = offsetof(struct e820map, map) + sizeof(struct e820entry) * e820->nr_map;
>>>> +	n = alloc_bootmem(size);
>>>> +	memcpy(n, e820, size);
>>>> +	e820 = n;
>>>> +
>>>> +	size = offsetof(struct e820map, map) + sizeof(struct e820entry) * e820_saved->nr_map;
>>>> +	n = alloc_bootmem(size);
>>>> +	memcpy(n, e820_saved, size);
>>>> +	e820_saved = n;
>>>> +}
>>>
>>> Ok, this makes me quite nervous, could you please split this into two patches so
>>> that any fails can be nicely bisected to?
>>
>> No problem.
>>
>>> First patch only does the pointerization changes with a trivial placeholder
>>> structure (full size, static allocated), second patch does all the dangerous bits
>>> such as changing it to __initdata, allocating and copying over bits.
>>>
>>> Also, could we please also add some minimal debugging facility to make sure the
>>> memory table does not get extended after it's been reallocated?
>>
>> I have another idea: run e820_reallocate_tables() later, just before
>> we free __init and __initdata. Then e820 tables _can't_ be_ changed -
>> all functions which do that are __init functions.
>>
>> Will test this now, and send a patchset.
>
> Could we also mark it __ro_after_init?

__ro_after_init makes sense only for statically allocated objects.
e820_reallocate_tables() copies e280 maps to kmalloced memory.