From patchwork Fri Jan 13 14:05:20 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 135845 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from merlin.infradead.org (merlin.infradead.org [IPv6:2001:4978:20e::2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 60A20B6EFF for ; Sat, 14 Jan 2012 01:08:35 +1100 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1RlhlQ-0006lP-5j; Fri, 13 Jan 2012 14:05:52 +0000 Received: from [2002:4e20:1eda::1] (helo=caramon.arm.linux.org.uk) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1RlhlG-0006k2-T0 for linux-arm-kernel@lists.infradead.org; Fri, 13 Jan 2012 14:05:49 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=AwuLoRIy27sc2ffSqmuNQK85CgFQFoUfKSPpkcVjXLQ=; b=Tjt9NPITdrlkDo3VF4J8oSD8OChiWOWSK10pIXnDnkO/5VSiqRL7GGeZbigjU6GvjOqHdMMNcMwvcFnmbu+D02sShNnq7zJoKMMKwrKRNKC0kddpzpTFC0a7bGKSmm87md7ng7O7c2rVUTHljIN3V1s9zGv/8wldbW0Ndq49yiM=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:54446) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1Rlhkv-0005Zp-Ui; Fri, 13 Jan 2012 14:05:22 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1Rlhku-00024Z-Jt; Fri, 13 Jan 2012 14:05:20 +0000 Date: Fri, 13 Jan 2012 14:05:20 +0000 From: Russell King - ARM Linux To: Tony Lindgren Subject: Re: [PATCH] arm: omap4: Fix omap_barriers_init for generic ioremap changes Message-ID: <20120113140520.GE16726@n2100.arm.linux.org.uk> References: <20120112184256.GA2287@atomide.com> <20120112195951.GB2287@atomide.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20120112195951.GB2287@atomide.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-Spam-Note: CRM114 invocation failed X-Spam-Score: -1.2 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.8 RDNS_NONE Delivered to internal network by a host with no rDNS Cc: linux-omap@vger.kernel.org, Santosh Shilimkar , linux-arm-kernel@lists.infradead.org, Nicolas Pitre X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org On Thu, Jan 12, 2012 at 11:59:51AM -0800, Tony Lindgren wrote: > * Nicolas Pitre [120112 11:15]: > > On Thu, 12 Jan 2012, Nicolas Pitre wrote: > > > > > > Do you really need that return code? > > > > > > It was initially hooked to core_initcall() which requires a return code. > > > Now that you call it directly you could get rid of it. > > > > s/rid of it/rid of the return code/ > > Well memblock_alloc could fail there BTW, that's another thing that's wrong here - it can't fail in such a way that it returns something to you: phys_addr_t __init memblock_alloc_base(phys_addr_t size, phys_addr_t align, phys_addr_t max_addr) { phys_addr_t alloc; alloc = __memblock_alloc_base(size, align, max_addr); if (alloc == 0) panic("ERROR: Failed to allocate 0x%llx bytes below 0x%llx.\n", (unsigned long long) size, (unsigned long long) max_addr); return alloc; } phys_addr_t __init memblock_alloc(phys_addr_t size, phys_addr_t align) { return memblock_alloc_base(size, align, MEMBLOCK_ALLOC_ACCESSIBLE); } So all these tests for zero addresses returned from memblock_alloc() in the OMAP code need to be killed off. Actually, I'll do it myself - especially as I'm considering adding 'arm_memblock_steal()' which will barf if it's misused. This makes the bug in the OMAP4 code glaringly obvious, to the point that OMAP4 will not boot until it's fixed. 8<=== From: Russell King ARM: Add arm_memblock_steal() to allocate memory away from the kernel Several platforms are now using the memblock_alloc+memblock_free+ memblock_remove trick to obtain memory which won't be mapped in the kernel's page tables. Most platforms do this (correctly) in the ->reserve callback. However, OMAP has started to call these functions outside of this callback, and this is extremely unsafe - memory will not be unmapped, and could well be given out after memblock is no longer responsible for its management. So, provide arm_memblock_steal() to perform this function, and ensure that it panic()s if it is used inappropriately. Convert everyone over, including OMAP. As a result, OMAP will panic on boot with this change. OMAP needs to be fixed, or 137d105d50 (ARM: OMAP4: Fix errata i688 with MPU interconnect barriers.) reverted until such time it can be fixed correctly. Signed-off-by: Russell King --- arch/arm/include/asm/memblock.h | 2 ++ arch/arm/mach-imx/mach-mx31_3ds.c | 5 ++--- arch/arm/mach-imx/mach-mx31moboard.c | 5 ++--- arch/arm/mach-imx/mach-pcm037.c | 5 ++--- arch/arm/mach-omap2/omap-secure.c | 13 ++----------- arch/arm/mach-omap2/omap4-common.c | 10 +++------- arch/arm/mm/init.c | 17 +++++++++++++++++ arch/arm/plat-omap/devices.c | 5 ++--- 8 files changed, 32 insertions(+), 30 deletions(-) diff --git a/arch/arm/include/asm/memblock.h b/arch/arm/include/asm/memblock.h index b8da2e4..00ca5f9 100644 --- a/arch/arm/include/asm/memblock.h +++ b/arch/arm/include/asm/memblock.h @@ -6,4 +6,6 @@ struct machine_desc; extern void arm_memblock_init(struct meminfo *, struct machine_desc *); +phys_addr_t arm_memblock_steal(phys_addr_t size, phys_addr_t align); + #endif diff --git a/arch/arm/mach-imx/mach-mx31_3ds.c b/arch/arm/mach-imx/mach-mx31_3ds.c index 89c3325..4d1aab1 100644 --- a/arch/arm/mach-imx/mach-mx31_3ds.c +++ b/arch/arm/mach-imx/mach-mx31_3ds.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -754,10 +755,8 @@ static struct sys_timer mx31_3ds_timer = { static void __init mx31_3ds_reserve(void) { /* reserve MX31_3DS_CAMERA_BUF_SIZE bytes for mx3-camera */ - mx3_camera_base = memblock_alloc(MX31_3DS_CAMERA_BUF_SIZE, + mx3_camera_base = arm_memblock_steal(MX31_3DS_CAMERA_BUF_SIZE, MX31_3DS_CAMERA_BUF_SIZE); - memblock_free(mx3_camera_base, MX31_3DS_CAMERA_BUF_SIZE); - memblock_remove(mx3_camera_base, MX31_3DS_CAMERA_BUF_SIZE); } MACHINE_START(MX31_3DS, "Freescale MX31PDK (3DS)") diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c index b95981d..f225262 100644 --- a/arch/arm/mach-imx/mach-mx31moboard.c +++ b/arch/arm/mach-imx/mach-mx31moboard.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -584,10 +585,8 @@ struct sys_timer mx31moboard_timer = { static void __init mx31moboard_reserve(void) { /* reserve 4 MiB for mx3-camera */ - mx3_camera_base = memblock_alloc(MX3_CAMERA_BUF_SIZE, + mx3_camera_base = arm_memblock_steal(MX3_CAMERA_BUF_SIZE, MX3_CAMERA_BUF_SIZE); - memblock_free(mx3_camera_base, MX3_CAMERA_BUF_SIZE); - memblock_remove(mx3_camera_base, MX3_CAMERA_BUF_SIZE); } MACHINE_START(MX31MOBOARD, "EPFL Mobots mx31moboard") diff --git a/arch/arm/mach-imx/mach-pcm037.c b/arch/arm/mach-imx/mach-pcm037.c index d7e1516..e48854b 100644 --- a/arch/arm/mach-imx/mach-pcm037.c +++ b/arch/arm/mach-imx/mach-pcm037.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -680,10 +681,8 @@ struct sys_timer pcm037_timer = { static void __init pcm037_reserve(void) { /* reserve 4 MiB for mx3-camera */ - mx3_camera_base = memblock_alloc(MX3_CAMERA_BUF_SIZE, + mx3_camera_base = arm_memblock_steal(MX3_CAMERA_BUF_SIZE, MX3_CAMERA_BUF_SIZE); - memblock_free(mx3_camera_base, MX3_CAMERA_BUF_SIZE); - memblock_remove(mx3_camera_base, MX3_CAMERA_BUF_SIZE); } MACHINE_START(PCM037, "Phytec Phycore pcm037") diff --git a/arch/arm/mach-omap2/omap-secure.c b/arch/arm/mach-omap2/omap-secure.c index 69f3c72..d8f8ef4 100644 --- a/arch/arm/mach-omap2/omap-secure.c +++ b/arch/arm/mach-omap2/omap-secure.c @@ -16,6 +16,7 @@ #include #include +#include #include @@ -57,20 +58,10 @@ u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs, u32 arg1, u32 arg2, /* Allocate the memory to save secure ram */ int __init omap_secure_ram_reserve_memblock(void) { - phys_addr_t paddr; u32 size = OMAP_SECURE_RAM_STORAGE; size = ALIGN(size, SZ_1M); - paddr = memblock_alloc(size, SZ_1M); - if (!paddr) { - pr_err("%s: failed to reserve %x bytes\n", - __func__, size); - return -ENOMEM; - } - memblock_free(paddr, size); - memblock_remove(paddr, size); - - omap_secure_memblock_base = paddr; + omap_secure_memblock_base = arm_memblock_steal(size, SZ_1M); return 0; } diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c index bc16c81..40a8fbc 100644 --- a/arch/arm/mach-omap2/omap4-common.c +++ b/arch/arm/mach-omap2/omap4-common.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -61,13 +62,8 @@ static int __init omap_barriers_init(void) return -ENODEV; size = ALIGN(PAGE_SIZE, SZ_1M); - paddr = memblock_alloc(size, SZ_1M); - if (!paddr) { - pr_err("%s: failed to reserve 4 Kbytes\n", __func__); - return -ENOMEM; - } - memblock_free(paddr, size); - memblock_remove(paddr, size); + paddr = arm_memblock_steal(size, SZ_1M); + dram_io_desc[0].virtual = OMAP4_DRAM_BARRIER_VA; dram_io_desc[0].pfn = __phys_to_pfn(paddr); dram_io_desc[0].length = size; diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index e34ea8a..6ec1226 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -307,6 +308,22 @@ static void arm_memory_present(void) } #endif +static bool arm_memblock_steal_permitted = true; + +phys_addr_t arm_memblock_steal(phys_addr_t size, phys_addr_t align) +{ + phys_addr_t phys; + + if (!arm_memblock_steal_permitted) + panic("arm_memblock_steal used in an unsafe manner\n"); + + phys = memblock_alloc(size, align); + memblock_free(phys, size); + memblock_remove(phys, size); + + return phys; +} + void __init arm_memblock_init(struct meminfo *mi, struct machine_desc *mdesc) { int i; @@ -349,6 +366,7 @@ void __init arm_memblock_init(struct meminfo *mi, struct machine_desc *mdesc) if (mdesc->reserve) mdesc->reserve(); + arm_memblock_steal_permitted = false; memblock_allow_resize(); memblock_dump_all(); } diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c index 1971932..60278f4 100644 --- a/arch/arm/plat-omap/devices.c +++ b/arch/arm/plat-omap/devices.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -164,14 +165,12 @@ void __init omap_dsp_reserve_sdram_memblock(void) if (!size) return; - paddr = memblock_alloc(size, SZ_1M); + paddr = arm_memblock_steal(size, SZ_1M); if (!paddr) { pr_err("%s: failed to reserve %x bytes\n", __func__, size); return; } - memblock_free(paddr, size); - memblock_remove(paddr, size); omap_dsp_phys_mempool_base = paddr; }