Blob Blame History Raw
Path: news.gmane.org!not-for-mail
From: Matt Fleming <matt@console-pimps.org>
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 <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>,
	"H. Peter Anvin" <hpa@zytor.com>, Zhang Rui <rui.zhang@intel.com>,
	Huang Ying <huang.ying.caritas@gmail.com>,
	linux-kernel@vger.kernel.org
To: Matthew Garrett <mjg@redhat.com>
Original-X-From: linux-kernel-owner@vger.kernel.org Fri Oct 14 13:36:59 2011
Return-path: <linux-kernel-owner@vger.kernel.org>
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 <linux-kernel-owner@vger.kernel.org>)
	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 <rfc822;glk-linux-kernel-3@m.gmane.org>);
	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
	<rfc822;linux-kernel@vger.kernel.org>);
	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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Xref: news.gmane.org gmane.linux.kernel:1203294
Archived-At: <http://permalink.gmane.org/gmane.linux.kernel/1203294>

From: Matt Fleming <matt.fleming@intel.com>

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: [<c10257b9>] 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:[<c10257b9>] 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:
   [<c104f8ca>] ? page_is_ram+0x1a/0x40
   [<c1025aff>] reserve_memtype+0xdf/0x2f0
   [<c1024dc9>] set_memory_uc+0x49/0xa0
   [<c19334d0>] efi_enter_virtual_mode+0x1c2/0x3aa
   [<c19216d4>] start_kernel+0x291/0x2f2
   [<c19211c7>] ? loglevel+0x1b/0x1b
   [<c19210bf>] 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 <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Huang Ying <huang.ying.caritas@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 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