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