From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 54BFEC282DD for ; Wed, 8 Jan 2020 17:17:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 228962067D for ; Wed, 8 Jan 2020 17:17:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 228962067D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=deltatee.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B8FCF8E0007; Wed, 8 Jan 2020 12:17:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B3F5C8E0001; Wed, 8 Jan 2020 12:17:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A7D368E0007; Wed, 8 Jan 2020 12:17:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0219.hostedemail.com [216.40.44.219]) by kanga.kvack.org (Postfix) with ESMTP id 94EB78E0001 for ; Wed, 8 Jan 2020 12:17:33 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id 46F21181AEF00 for ; Wed, 8 Jan 2020 17:17:33 +0000 (UTC) X-FDA: 76355123586.24.limit38_7c157ee2bc33d X-HE-Tag: limit38_7c157ee2bc33d X-Filterd-Recvd-Size: 8420 Received: from ale.deltatee.com (ale.deltatee.com [207.54.116.67]) by imf50.hostedemail.com (Postfix) with ESMTP for ; Wed, 8 Jan 2020 17:17:32 +0000 (UTC) Received: from s0106ac1f6bb1ecac.cg.shawcable.net ([70.73.163.230] helo=[192.168.11.155]) by ale.deltatee.com with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1ipExL-0004JB-Ec; Wed, 08 Jan 2020 10:17:20 -0700 To: David Hildenbrand , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-ia64@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-mm@kvack.org, Dan Williams , Michal Hocko , Andrew Morton Cc: Christoph Hellwig , Catalin Marinas , Will Deacon , Benjamin Herrenschmidt , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , Andy Lutomirski , Peter Zijlstra , Eric Badger , Michal Hocko References: <20200107205959.7575-1-logang@deltatee.com> <20200107205959.7575-8-logang@deltatee.com> From: Logan Gunthorpe Message-ID: Date: Wed, 8 Jan 2020 10:17:10 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 70.73.163.230 X-SA-Exim-Rcpt-To: mhocko@suse.com, ebadger@gigaio.com, peterz@infradead.org, luto@kernel.org, dave.hansen@linux.intel.com, bp@alien8.de, mingo@redhat.com, tglx@linutronix.de, benh@kernel.crashing.org, will@kernel.org, catalin.marinas@arm.com, hch@lst.de, akpm@linux-foundation.org, mhocko@kernel.org, dan.j.williams@intel.com, linux-mm@kvack.org, platform-driver-x86@vger.kernel.org, linux-sh@vger.kernel.org, linux-s390@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-ia64@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, david@redhat.com X-SA-Exim-Mail-From: logang@deltatee.com Subject: Re: [PATCH v2 7/8] mm/memory_hotplug: Add pgprot_t to mhp_modifiers X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on ale.deltatee.com) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2020-01-08 5:39 a.m., David Hildenbrand wrote: > On 07.01.20 21:59, Logan Gunthorpe wrote: >> devm_memremap_pages() is currently used by the PCI P2PDMA code to create >> struct page mappings for IO memory. At present, these mappings are created >> with PAGE_KERNEL which implies setting the PAT bits to be WB. However, on >> x86, an mtrr register will typically override this and force the cache >> type to be UC-. In the case firmware doesn't set this register it is >> effectively WB and will typically result in a machine check exception >> when it's accessed. >> >> Other arches are not currently likely to function correctly seeing they >> don't have any MTRR registers to fall back on. >> >> To solve this, add an argument to arch_add_memory() to explicitly >> set the pgprot value to a specific value. > > You're adding a parameter indirectly by adding it to the structure. > Maybe "provide a way to specify the pgprot value explicitly to > arch_add_memory()" > >> >> Of the arches that support MEMORY_HOTPLUG: x86_64, s390 and arm64 is a > > s/is/need/ > >> simple change to pass the pgprot_t down to their respective functions >> which set up the page tables. For x86_32, set the page tables explicitly > > "page table protection" ? > >> using _set_memory_prot() (seeing they are already mapped). For sh, reject >> anything but PAGE_KERNEL settings -- this should be fine, for now, seeing >> sh doesn't support ZONE_DEVICE anyway. >> >> Cc: Dan Williams >> Cc: David Hildenbrand >> Cc: Michal Hocko >> Signed-off-by: Logan Gunthorpe >> --- >> arch/arm64/mm/mmu.c | 3 ++- >> arch/ia64/mm/init.c | 4 ++++ >> arch/powerpc/mm/mem.c | 3 ++- >> arch/s390/mm/init.c | 2 +- >> arch/sh/mm/init.c | 3 +++ >> arch/x86/mm/init_32.c | 5 +++++ >> arch/x86/mm/init_64.c | 2 +- >> include/linux/memory_hotplug.h | 2 ++ >> mm/memory_hotplug.c | 2 +- >> mm/memremap.c | 6 +++--- >> 10 files changed, 24 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index 3320406579c3..9b214b0d268f 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -1058,7 +1058,8 @@ int arch_add_memory(int nid, u64 start, u64 size, >> flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS; >> >> __create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start), >> - size, PAGE_KERNEL, __pgd_pgtable_alloc, flags); >> + size, modifiers->pgprot, __pgd_pgtable_alloc, >> + flags); >> >> memblock_clear_nomap(start, size); >> >> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c >> index daf438e08b96..5fd6ae4929c9 100644 >> --- a/arch/ia64/mm/init.c >> +++ b/arch/ia64/mm/init.c >> @@ -677,6 +677,10 @@ int arch_add_memory(int nid, u64 start, u64 size, >> int ret; >> >> ret = __add_pages(nid, start_pfn, nr_pages, modifiers); >> + if (modifiers->pgprot != PAGE_KERNEL) >> + return -EINVAL; > > ... maybe better "if (WARN_ON_ONCE(...))" > [...] > >> --- a/include/linux/memory_hotplug.h >> +++ b/include/linux/memory_hotplug.h >> @@ -56,9 +56,11 @@ enum { >> /* >> * Restrictions for the memory hotplug: >> * altmap: alternative allocator for memmap array >> + * pgprot: page protection flags to apply to newly added page tables >> */ >> struct mhp_modifiers { >> struct vmem_altmap *altmap; >> + pgprot_t pgprot; >> }; >> >> /* >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >> index 1bb3f92e087d..0888f821af06 100644 >> --- a/mm/memory_hotplug.c >> +++ b/mm/memory_hotplug.c >> @@ -1027,7 +1027,7 @@ static int online_memory_block(struct memory_block *mem, void *arg) >> */ >> int __ref add_memory_resource(int nid, struct resource *res) >> { >> - struct mhp_modifiers modifiers = {}; >> + struct mhp_modifiers modifiers = {.pgprot = PAGE_KERNEL}; > > I think we usually use spaces like > > = { .pgprot = PAGE_KERNEL }; > > t480s: ~/git/linux virtio-mem-v1 $ git grep "= {\." | wc -l > 978 > t480s: ~/git/linux virtio-mem-v1 $ git grep "= { " | wc -l > 35447 > >> u64 start, size; >> bool new_node = false; >> int ret; >> diff --git a/mm/memremap.c b/mm/memremap.c >> index e30be8ba706b..45ab4ef0643d 100644 >> --- a/mm/memremap.c >> +++ b/mm/memremap.c >> @@ -163,8 +163,8 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid) >> * We do not want any optional features only our own memmap >> */ >> .altmap = pgmap_altmap(pgmap), >> + .pgprot = PAGE_KERNEL, >> }; >> - pgprot_t pgprot = PAGE_KERNEL; >> int error, is_ram; >> bool need_devmap_managed = true; >> >> @@ -252,8 +252,8 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid) >> if (nid < 0) >> nid = numa_mem_id(); >> >> - error = track_pfn_remap(NULL, &pgprot, PHYS_PFN(res->start), 0, >> - resource_size(res)); >> + error = track_pfn_remap(NULL, &modifiers.pgprot, PHYS_PFN(res->start), >> + 0, resource_size(res)); >> if (error) >> goto err_pfn_remap; >> >> > > The !arch code looks good to me (besides I would prefer "params" instead > of "modifiers"). > > Acked-by: David Hildenbrand Thanks, I'll make the changes above for v3. Logan