Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965642AbcKXN6f (ORCPT ); Thu, 24 Nov 2016 08:58:35 -0500 Received: from mail-io0-f181.google.com ([209.85.223.181]:34806 "EHLO mail-io0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965600AbcKXN6c (ORCPT ); Thu, 24 Nov 2016 08:58:32 -0500 MIME-Version: 1.0 In-Reply-To: <20161124135151.GJ10776@rric.localdomain> References: <20161028091905.GM22012@rric.localdomain> <20161107210514.GP20591@arm.com> <20161109195132.GZ22012@rric.localdomain> <20161117142528.GJ22855@arm.com> <20161117151805.GJ2151@rric.localdomain> <20161123211538.GH10776@rric.localdomain> <20161124134238.GI10776@rric.localdomain> <20161124135151.GJ10776@rric.localdomain> From: Ard Biesheuvel Date: Thu, 24 Nov 2016 13:58:30 +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: 2582 Lines: 60 On 24 November 2016 at 13:51, Robert Richter wrote: > On 24.11.16 13:44:31, Ard Biesheuvel wrote: >> On 24 November 2016 at 13:42, Robert Richter wrote: >> > On 23.11.16 21:25:06, Ard Biesheuvel wrote: >> >> Why? MEMREMAP_WB is used often, among other things for mapping >> >> firmware tables, which are marked as NOMAP, so in these cases, the >> >> linear address is not mapped. >> > >> > If fw tables are mapped wb, that is wrong and needs a separate fix. >> > >> >> Why is that wrong? > > The whole issue with mapping acpi tables is not marking them cachable, > what wb does. What 'issue'? > Otherwise we could just use linear mapping for those mem > ranges. > Regions containing firmware tables are owned by the firmware, and it is the firmware that tells us which memory attributes we are allowed to use. If those attributes include WB, it is perfectly legal to use a cacheable mapping. That does *not* mean they should be covered by the linear mapping. The linear mapping is read-write-non-exec, for instance, and we may prefer to use a read-only mapping and/or executable mapping. >> >> > If you think pfn_valid() is wrong here, I am happy to send a patch >> >> > that fixes this by using page_is_ram(). In any case, the worst case >> >> > that may happen is to behave the same as v4.4, we might fix then the >> >> > wrong use of pfn_valid() where it is not correctly used to check for >> >> > ram. >> >> > >> >> >> >> page_is_ram() uses string comparisons to look for regions called >> >> 'System RAM'. Is that something we can tolerate for each pfn_valid() >> >> calll? >> >> >> >> Perhaps the solution is to reimplement page_is_ram() for arm64 using >> >> memblock_is_memory() instead, But that still means we need to modify >> >> the generic memremap() code first to switch to it before changing the >> >> arm64 implementation of pfn_valid >> > >> > No, that's not the solution. pfn_valid() should just check if there is >> > a valid struct page, as other archs do. And if there is a mis-use of >> > pfn_valid() to check for ram, only that calls should be fixed to use >> > page_is_ram(), however this is implemented, or something appropriate. >> > But I don't see any problematic code, and if so, I will fix that. >> > >> >> memremap() uses pfn_valid() to decide whether some address is covered >> by the linear mapping. If we correct pfn_valid() to adhere to your >> definition, we will need to fix memremap() first in any case. > > As said, will fix that if needed. But I think the caller is wrong > then. >