Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754193AbcKYM3C (ORCPT ); Fri, 25 Nov 2016 07:29:02 -0500 Received: from mail-io0-f175.google.com ([209.85.223.175]:34398 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752955AbcKYM2y (ORCPT ); Fri, 25 Nov 2016 07:28:54 -0500 MIME-Version: 1.0 In-Reply-To: <20161125112914.GI2213@rric.localdomain> References: <20161124134238.GI10776@rric.localdomain> <20161124135151.GJ10776@rric.localdomain> <20161124141149.GE2213@rric.localdomain> <20161124150918.GF2213@rric.localdomain> <20161124192659.GH2213@rric.localdomain> <20161125112914.GI2213@rric.localdomain> From: Ard Biesheuvel Date: Fri, 25 Nov 2016 12:28:50 +0000 Message-ID: Subject: Re: [PATCH] arm64: mm: Fix memmap to be initialized for the entire section To: Robert Richter Cc: Will Deacon , Robert Richter , Mark Rutland , Catalin Marinas , David Daney , Hanjun Guo , "linux-arm-kernel@lists.infradead.org" , "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9228 Lines: 120 On 25 November 2016 at 11:29, Robert Richter wrote: > On 24.11.16 19:42:47, Ard Biesheuvel wrote: >> On 24 November 2016 at 19:26, Robert Richter wrote: > >> > I revisited the code and it is working well already since: >> > >> > e7cd190385d1 arm64: mark reserved memblock regions explicitly in iomem >> > >> > Now, try_ram_remap() is only called if the region to be mapped is >> > entirely in IORESOURCE_SYSTEM_RAM. This is only true for normal mem >> > ranges and not NOMAP mem. region_intersects() then returns >> > REGION_INTERSECTS and calls try_ram_remap(). For the NOMAP memory case >> > REGION_DISJOINT would be returned and thus arch_memremap_wb() being >> > called directly. Before the e7cd190385d1 change try_ram_remap() was >> > called also for nomap regions. >> > >> > So we can leave memremap() as it is and just apply this patch >> > unmodified. What do you think? >> >> I agree. The pfn_valid() check in try_ram_remap() is still appropriate >> simply because the PageHighmem check requires a valid struct page. But >> if we don't enter that code path anymore for NOMAP regions, I think >> we're ok. >> >> > Please ack. >> > >> >> I still don't fully understand how it is guaranteed that *all* memory >> (i.e., all regions for which memblock_is_memory() returns true) is >> covered by a struct page, but marked as reserved. Are we relying on >> the fact that NOMAP memory is also memblock_reserve()'d? > > See free_low_memory_core_early(): > > ---- > for_each_free_mem_range(i, NUMA_NO_NODE, MEMBLOCK_NONE, &start, &end, > NULL) > count += __free_memory_core(start, end); > ---- > > Only mem with the MEMBLOCK_NONE flag is added. And NOMAP pages are > also *not* marked reserved. So nothing at all from NOMAP mem is > reported to mm, it is not present (see below for a mem config, note > flags: 0x4 mem regions). > OK, thanks for clearing that up. But that still does not explain how we can be certain that NOMAP regions are guaranteed to be covered by a struct page, does it? Because that is ultimately what pfn_valid() means, that it is safe to, e.g., look at the page flags. > [ 0.000000] efi: Processing EFI memory map: > [ 0.000000] efi: 0x000001400000-0x00000147ffff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000001480000-0x0000024bffff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x0000024c0000-0x0000211fffff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000021200000-0x00002121ffff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000021220000-0x0000fffebfff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x0000fffec000-0x0000ffff5fff [ACPI Reclaim Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x0000ffff6000-0x0000ffff6fff [ACPI Memory NVS | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x0000ffff7000-0x0000ffffffff [ACPI Reclaim Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000100000000-0x000ff7ffffff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000ff8000000-0x000ff801ffff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000ff8020000-0x000fffa9efff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x000fffa9f000-0x000fffffffff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010000400000-0x010f816aefff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f816af000-0x010f816b1fff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f816b2000-0x010f826f1fff [Loader Code | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f826f2000-0x010f82701fff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f82702000-0x010f82787fff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f82788000-0x010f9276bfff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f9276c000-0x010f9276cfff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f9276d000-0x010f935a8fff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f935a9000-0x010f93880fff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010f93881000-0x010ff7880fff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ff7881000-0x010ff7886fff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ff7887000-0x010ff78a3fff [Loader Code | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ff78a4000-0x010ff9e8dfff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ff9e8e000-0x010ff9f16fff [Runtime Data |RUN| | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ff9f17000-0x010ffaeb5fff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffaeb6000-0x010ffafc8fff [Runtime Data |RUN| | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffafc9000-0x010ffafccfff [Runtime Code |RUN| | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffafcd000-0x010ffaff4fff [Runtime Data |RUN| | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffaff5000-0x010ffb008fff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffb009000-0x010fffe28fff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010fffe29000-0x010fffe3ffff [Conventional Memory| | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010fffe40000-0x010fffe53fff [Loader Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010fffe54000-0x010ffffb8fff [Boot Code | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffffb9000-0x010ffffccfff [Runtime Code |RUN| | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffffcd000-0x010fffffefff [Runtime Data |RUN| | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x010ffffff000-0x010fffffffff [Boot Data | | | | | | | | |WB|WT|WC|UC] > [ 0.000000] efi: 0x804000001000-0x804000001fff [Memory Mapped I/O |RUN| | | | | | | | | | |UC] > [ 0.000000] efi: 0x87e0d0001000-0x87e0d0001fff [Memory Mapped I/O |RUN| | | | | | | | | | |UC] > > [ 0.000000] MEMBLOCK configuration: > [ 0.000000] memory size = 0x1ffe800000 reserved size = 0x39146a21 > [ 0.000000] memory.cnt = 0x9 > [ 0.000000] memory[0x0] [0x00000001400000-0x000000fffdffff], 0xfebe0000 bytes on node 0 flags: 0x0 > [ 0.000000] memory[0x1] [0x000000fffe0000-0x000000ffffffff], 0x20000 bytes on node 0 flags: 0x4 > [ 0.000000] memory[0x2] [0x00000100000000-0x00000fffffffff], 0xf00000000 bytes on node 0 flags: 0x0 > [ 0.000000] memory[0x3] [0x00010000400000-0x00010ff9e7ffff], 0xff9a80000 bytes on node 1 flags: 0x0 > [ 0.000000] memory[0x4] [0x00010ff9e80000-0x00010ff9f1ffff], 0xa0000 bytes on node 1 flags: 0x4 > [ 0.000000] memory[0x5] [0x00010ff9f20000-0x00010ffaeaffff], 0xf90000 bytes on node 1 flags: 0x0 > [ 0.000000] memory[0x6] [0x00010ffaeb0000-0x00010ffaffffff], 0x150000 bytes on node 1 flags: 0x4 > [ 0.000000] memory[0x7] [0x00010ffb000000-0x00010ffffaffff], 0x4fb0000 bytes on node 1 flags: 0x0 > [ 0.000000] memory[0x8] [0x00010ffffb0000-0x00010fffffffff], 0x50000 bytes on node 1 flags: 0x4 > [ 0.000000] reserved.cnt = 0xd > [ 0.000000] reserved[0x0] [0x00000001480000-0x0000000249ffff], 0x1020000 bytes flags: 0x0 > [ 0.000000] reserved[0x1] [0x00000021200000-0x00000021210536], 0x10537 bytes flags: 0x0 > [ 0.000000] reserved[0x2] [0x000000c0000000-0x000000dfffffff], 0x20000000 bytes flags: 0x0 > [ 0.000000] reserved[0x3] [0x00000ffbfb8000-0x00000ffffdffff], 0x4028000 bytes flags: 0x0 > [ 0.000000] reserved[0x4] [0x00000ffffecb00-0x00000fffffffff], 0x13500 bytes flags: 0x0 > [ 0.000000] reserved[0x5] [0x00010f81780000-0x00010f8178ffff], 0x10000 bytes flags: 0x0 > [ 0.000000] reserved[0x6] [0x00010f82870000-0x00010f9286ffff], 0x10000000 bytes flags: 0x0 > [ 0.000000] reserved[0x7] [0x00010ffbce0000-0x00010fffceffff], 0x4010000 bytes flags: 0x0 > [ 0.000000] reserved[0x8] [0x00010fffee6d80-0x00010ffff2fffb], 0x4927c bytes flags: 0x0 > [ 0.000000] reserved[0x9] [0x00010ffff30000-0x00010ffffa000f], 0x70010 bytes flags: 0x0 > [ 0.000000] reserved[0xa] [0x00010ffffae280-0x00010ffffaff7f], 0x1d00 bytes flags: 0x0 > [ 0.000000] reserved[0xb] [0x00010ffffaffa0-0x00010ffffaffce], 0x2f bytes flags: 0x0 > [ 0.000000] reserved[0xc] [0x00010ffffaffd0-0x00010ffffafffe], 0x2f bytes flags: 0x0