From patchwork Thu Apr 9 14:51:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Grygorii.Strashko@linaro.org" X-Patchwork-Id: 6188391 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 099F49F2E9 for ; Thu, 9 Apr 2015 14:59:08 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DB3F02035D for ; Thu, 9 Apr 2015 14:59:06 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CCD7B2037A for ; Thu, 9 Apr 2015 14:59:05 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YgDsI-0002VN-Bj; Thu, 09 Apr 2015 14:56:10 +0000 Received: from mail-la0-f43.google.com ([209.85.215.43]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YgDny-0006fz-Dc for linux-arm-kernel@lists.infradead.org; Thu, 09 Apr 2015 14:51:43 +0000 Received: by labbd9 with SMTP id bd9so80010962lab.2 for ; Thu, 09 Apr 2015 07:51:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:message-id:date:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=1NPP6CENW9Tvqq7wrpQ6B08Bfhx8yItmX5Ww8oG2qFo=; b=Hya6OdrPiyqsq0fu9+o5PKWubtkcZqY7RsA/B2Xu1tzNS1weqP7RZ1+0konpUp2g5b 3CjspxWd5lVEul3kX/FLq+/w/50lq9Atv4Way+MH5CYHPHftK9BEVYG72S8qnesZ5YcM 7FkQWDKHadsXzX4N1qfevusSP9dRdqr6daWk434j32BlZEyxRlqFRp3kPaybSAlyGq+m jYg8U4OQ9ywn0gjCHg1EDea/sQEyJ9DysOjkgiMPWuSf/oKq/AwbZgsZQnrC4jwI6j1g 6J/38hvEOfmeyuWtshyD5W834lZoIgEcTNvd8XvFNPH1G/xzuw3NJ0MtYhmF7FDQYAAo ukbw== X-Gm-Message-State: ALoCoQmE75itpF0J8T3FrfU0+hipR5+rKTbK+vnwHSCqQO4kr6NKWQa4JKBWBF7zD2i+c51+rr52 X-Received: by 10.112.146.41 with SMTP id sz9mr24187413lbb.77.1428591079340; Thu, 09 Apr 2015 07:51:19 -0700 (PDT) Received: from [172.22.39.17] ([195.238.92.128]) by mx.google.com with ESMTPSA id ds8sm3260458lbb.18.2015.04.09.07.51.18 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Apr 2015 07:51:18 -0700 (PDT) From: "Grygorii.Strashko@linaro.org" X-Google-Original-From: "Grygorii.Strashko@linaro.org" Message-ID: <552691E2.2050000@linaro.org> Date: Thu, 09 Apr 2015 17:51:14 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Russell King - ARM Linux , "Grygorii.Strashko@linaro.org" Subject: Re: [PATCH 2/6] ARM: keystone2: move update of the phys-to-virt constants into generic code References: <20150408094438.GM12732@n2100.arm.linux.org.uk> <552541B1.2000501@linaro.org> <20150408175959.GO12732@n2100.arm.linux.org.uk> In-Reply-To: <20150408175959.GO12732@n2100.arm.linux.org.uk> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150409_075142_864231_8E4B0F6A X-CRM114-Status: GOOD ( 30.24 ) X-Spam-Score: -0.7 (/) Cc: linux-arm-kernel@lists.infradead.org, Santosh Shilimkar X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Russell, On 04/08/2015 09:00 PM, Russell King - ARM Linux wrote: > On Wed, Apr 08, 2015 at 05:56:49PM +0300, Grygorii.Strashko@linaro.org wrote: >> Hi Russell, >> >> On 04/08/2015 12:45 PM, Russell King wrote: >>> Make the init_meminfo function return the offset to be applied to the >>> phys-to-virt translation constants. This allows us to move the update >>> into generic code, along with the requirements for this update. >> >> >> I have a question (or may be proposition) regarding this patch. >> >> Could it be reasonable if .init_meminfo() will return new PHYS offset >> of RAM (new start address), instead of translation's offset? > > I'm not that interested in such a change, for the simple reason that > what we mostly want to know is what the _difference_ between what's > already in the page tables, and what we want to update them to. > > The page table update is a case of merely adding the delta to each > page table entry. > > If we were to want to pass the actual new physical address, we would > have to have the fixup code mask out the old address, and insert the > new address, keeping track of the offset into the kernel's mapping. > We'd also have to do something weirder for the DT mapping too. > > Using an offset makes the page table update rather trivial and easy. > > I actually did start off with passing the new physical address > initially, and I changed to this way because it simplified the > assembly code. > Ok, I understand the point, but still wish to try this proposition (even if I'll be punished)). First, the current commit description confused me a bit and that was the main reason why I commented here. Commit message said: "Make the init_meminfo function return the offset to be applied to the phys-to-virt translation constants" but actual return value can't be applied to "phys-to-virt translation constants" without modification. The offset to be applied to "phys-to-virt translation constants" should be (NEW_PHYS_OFFSET - PAGE_OFFSET) as per commit dc21af9. But now, the .init_meminfo() assumed to return offset between NEW and OLD RAM starting addresses : (NEW_PHYS_OFFSET - OLD_PHYS_OFFSET). And, It looks like this offset actually needed only for lpae_pgtables_remap(). I think, We can get all data needed for physical address switching and pgtables updating code from inside early_paging_init() except the NEW_PHYS_OFFSET - - new starting physical address of the RAM and, therefore, It seems reasonable to get it as return value of .pv_fixup(). Therefore, I did a patch/diff on top of your series to illustrate this (no changes in the assembly code) - K2HK board boots. Regardless of will you accept this change or not - I've tested boot on K2HK board with yours patches (applied on top of k4.0-rc7), so Tested-by: Grygorii Strashko diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index cb3a407..84acaba 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -51,7 +51,7 @@ struct machine_desc { bool (*smp_init)(void); void (*fixup)(struct tag *, char **); void (*dt_fixup)(void); - long long (*pv_fixup)(void); + phys_addr_t (*pv_fixup)(void); void (*reserve)(void);/* reserve mem blocks */ void (*map_io)(void);/* IO mapping function */ void (*init_early)(void); diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c index e288010..f3816b1 100644 --- a/arch/arm/mach-keystone/keystone.c +++ b/arch/arm/mach-keystone/keystone.c @@ -68,7 +68,7 @@ static phys_addr_t keystone_virt_to_idmap(unsigned long x) return (phys_addr_t)(x) - CONFIG_PAGE_OFFSET + KEYSTONE_LOW_PHYS_START; } -static long long __init keystone_pv_fixup(void) +static phys_addr_t __init keystone_pv_fixup(void) { long long offset; phys_addr_t mem_start, mem_end; @@ -88,7 +88,7 @@ static long long __init keystone_pv_fixup(void) return 0; } - offset = KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START; + offset = KEYSTONE_HIGH_PHYS_START; /* Populate the arch idmap hook */ arch_virt_to_idmap = keystone_virt_to_idmap; diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index b2e96bc..1d6c119 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -1401,14 +1401,15 @@ void __init early_paging_init(const struct machine_desc *mdesc) pgtables_remap *lpae_pgtables_remap; unsigned long pa_pgd; unsigned int cr; - long long offset; + phys_addr_t new_phys_offset; + phys_addr_t old_phys_offset = PHYS_OFFSET; void *boot_data; if (!mdesc->pv_fixup) return; - offset = mdesc->pv_fixup(); - if (offset == 0) + new_phys_offset = mdesc->pv_fixup(); + if (new_phys_offset == 0) return; /* @@ -1422,12 +1423,12 @@ void __init early_paging_init(const struct machine_desc *mdesc) boot_data = __va(__atags_pointer); barrier(); - pr_info("Switching physical address space to 0x%08llx\n", - (u64)PHYS_OFFSET + offset); + pr_info("Switching physical address space to %pa\n", + &new_phys_offset); /* Re-set the phys pfn offset, and the pv offset */ - __pv_offset = PHYS_OFFSET + offset - PAGE_OFFSET; - __pv_phys_pfn_offset = PFN_DOWN(PHYS_OFFSET + offset); + __pv_offset = new_phys_offset - PAGE_OFFSET; + __pv_phys_pfn_offset = PFN_DOWN(new_phys_offset); /* Run the patch stub to update the constants */ fixup_pv_table(&__pv_table_begin, @@ -1451,7 +1452,8 @@ void __init early_paging_init(const struct machine_desc *mdesc) * needs to be assembly. It's fairly simple, as we're using the * temporary tables setup by the initial assembly code. */ - lpae_pgtables_remap(offset, pa_pgd, boot_data); + lpae_pgtables_remap((new_phys_offset - old_phys_offset), + pa_pgd, boot_data); /* Re-enable the caches */ set_cr(cr);