Path: news.gmane.org!not-for-mail From: Matt Fleming Newsgroups: gmane.linux.kernel Subject: [PATCH v2] x86, efi: Calling __pa() with an ioremap'd address is invalid Date: Fri, 14 Oct 2011 12:36:45 +0100 Lines: 160 Approved: news@gmane.org Message-ID: <1318592205-11193-1-git-send-email-matt@console-pimps.org> NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1318592224 30879 80.91.229.12 (14 Oct 2011 11:37:04 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 14 Oct 2011 11:37:04 +0000 (UTC) Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Zhang Rui , Huang Ying , linux-kernel@vger.kernel.org To: Matthew Garrett Original-X-From: linux-kernel-owner@vger.kernel.org Fri Oct 14 13:36:59 2011 Return-path: Envelope-to: glk-linux-kernel-3@lo.gmane.org Original-Received: from vger.kernel.org ([209.132.180.67]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1REg4Q-0001UQ-SA for glk-linux-kernel-3@lo.gmane.org; Fri, 14 Oct 2011 13:36:59 +0200 Original-Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755420Ab1JNLgv (ORCPT ); Fri, 14 Oct 2011 07:36:51 -0400 Original-Received: from arkanian.console-pimps.org ([212.110.184.194]:46859 "EHLO arkanian.console-pimps.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751315Ab1JNLgu (ORCPT ); Fri, 14 Oct 2011 07:36:50 -0400 Original-Received: by arkanian.console-pimps.org (Postfix, from userid 1002) id 443C1C0009; Fri, 14 Oct 2011 12:36:49 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on arkanian.vm.bytemark.co.uk X-Spam-Level: X-Spam-Status: No, score=-5.3 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 Original-Received: from localhost (02ddb86b.bb.sky.com [2.221.184.107]) by arkanian.console-pimps.org (Postfix) with ESMTPSA id F0D40C0008; Fri, 14 Oct 2011 12:36:47 +0100 (BST) X-Mailer: git-send-email 1.7.4.4 Original-Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Xref: news.gmane.org gmane.linux.kernel:1203294 Archived-At: From: Matt Fleming If we encounter an efi_memory_desc_t without EFI_MEMORY_WB set in ->attribute we currently call set_memory_uc(), which in turn calls __pa() on a potentially ioremap'd address. On CONFIG_X86_32 this is invalid, resulting in the following oops, BUG: unable to handle kernel paging request at f7f22280 IP: [] reserve_ram_pages_type+0x89/0x210 *pdpt = 0000000001978001 *pde = 0000000001ffb067 *pte = 0000000000000000 Oops: 0000 [#1] PREEMPT SMP Modules linked in: Pid: 0, comm: swapper Not tainted 3.0.0-acpi-efi-0805 #3 EIP: 0060:[] EFLAGS: 00010202 CPU: 0 EIP is at reserve_ram_pages_type+0x89/0x210 EAX: 0070e280 EBX: 38714000 ECX: f7814000 EDX: 00000000 ESI: 00000000 EDI: 38715000 EBP: c189fef0 ESP: c189fea8 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 Process swapper (pid: 0, ti=c189e000 task=c18bbe60 task.ti=c189e000) Stack: 80000200 ff108000 00000000 c189ff00 00038714 00000000 00000000 c189fed0 c104f8ca 00038714 00000000 00038715 00000000 00000000 00038715 00000000 00000010 38715000 c189ff48 c1025aff 38715000 00000000 00000010 00000000 Call Trace: [] ? page_is_ram+0x1a/0x40 [] reserve_memtype+0xdf/0x2f0 [] set_memory_uc+0x49/0xa0 [] efi_enter_virtual_mode+0x1c2/0x3aa [] start_kernel+0x291/0x2f2 [] ? loglevel+0x1b/0x1b [] i386_start_kernel+0xbf/0xc8 So, if we're ioremap'ing an address range let's setup the mapping with the correct caching attribute instead of modifying it after the fact. Also, take this opportunity to unify the 32/64-bit efi_ioremap() implementations because they can both be implemented with ioremap_{cache,nocache}. When asked about the original reason behind using init_memory_mapping() for the 64-bit version Huang Ying said, "The intention of init_memory_mapping() usage is to make EFI virtual address unchanged after kexec. But in fact, init_memory_mapping() can not handle some memory range, so ioremap_xxx() is introduced as a fix. Now we decide to use ioremap_xxx() anyway and use some other scheme for kexec support, so init_memory_mapping() here is unnecessary. IMHO, init_memory_mapping() is not as good as ioremap_xxx() here." And because efi_ioremap() now consists of 4 lines, let's just inline it directly into the one callsite in efi_enter_virtual_mode(). Cc: Thomas Gleixner Cc: Ingo Molnar Cc: H. Peter Anvin Cc: Matthew Garrett Cc: Zhang Rui Cc: Huang Ying Cc: stable@kernel.org Signed-off-by: Matt Fleming --- arch/x86/include/asm/efi.h | 5 ----- arch/x86/platform/efi/efi.c | 24 ++++++++++++++---------- arch/x86/platform/efi/efi_64.c | 17 ----------------- 3 files changed, 14 insertions(+), 32 deletions(-) diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 7093e4a..b8d8bfc 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -33,8 +33,6 @@ extern unsigned long asmlinkage efi_call_phys(void *, ...); #define efi_call_virt6(f, a1, a2, a3, a4, a5, a6) \ efi_call_virt(f, a1, a2, a3, a4, a5, a6) -#define efi_ioremap(addr, size, type) ioremap_cache(addr, size) - #else /* !CONFIG_X86_32 */ extern u64 efi_call0(void *fp); @@ -84,9 +82,6 @@ extern u64 efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3, efi_call6((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4), (u64)(a5), (u64)(a6)) -extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size, - u32 type); - #endif /* CONFIG_X86_32 */ extern int add_efi_memmap; diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 3ae4128..6ea011c 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -670,10 +670,21 @@ void __init efi_enter_virtual_mode(void) end_pfn = PFN_UP(end); if (end_pfn <= max_low_pfn_mapped || (end_pfn > (1UL << (32 - PAGE_SHIFT)) - && end_pfn <= max_pfn_mapped)) + && end_pfn <= max_pfn_mapped)) { va = __va(md->phys_addr); - else - va = efi_ioremap(md->phys_addr, size, md->type); + + if (!(md->attribute & EFI_MEMORY_WB)) { + addr = (u64) (unsigned long)va; + npages = md->num_pages; + memrange_efi_to_native(&addr, &npages); + set_memory_uc(addr, npages); + } + } else { + if (!(md->attribute & EFI_MEMORY_WB)) + va = ioremap_nocache(md->phys_addr, size); + else + va = ioremap_cache(md->phys_addr, size); + } md->virt_addr = (u64) (unsigned long) va; @@ -683,13 +694,6 @@ void __init efi_enter_virtual_mode(void) continue; } - if (!(md->attribute & EFI_MEMORY_WB)) { - addr = md->virt_addr; - npages = md->num_pages; - memrange_efi_to_native(&addr, &npages); - set_memory_uc(addr, npages); - } - systab = (u64) (unsigned long) efi_phys.systab; if (md->phys_addr <= systab && systab < end) { systab += md->virt_addr - md->phys_addr; diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c index ac3aa54..312250c 100644 --- a/arch/x86/platform/efi/efi_64.c +++ b/arch/x86/platform/efi/efi_64.c @@ -80,20 +80,3 @@ void __init efi_call_phys_epilog(void) local_irq_restore(efi_flags); early_code_mapping_set_exec(0); } - -void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size, - u32 type) -{ - unsigned long last_map_pfn; - - if (type == EFI_MEMORY_MAPPED_IO) - return ioremap(phys_addr, size); - - last_map_pfn = init_memory_mapping(phys_addr, phys_addr + size); - if ((last_map_pfn << PAGE_SHIFT) < phys_addr + size) { - unsigned long top = last_map_pfn << PAGE_SHIFT; - efi_ioremap(top, size - (top - phys_addr), type); - } - - return (void __iomem *)__va(phys_addr); -} -- 1.7.4.4