From: Russell King - ARM Linux <linux@arm.linux.org.uk> To: Catalin Marinas <catalin.marinas@arm.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 07/20] ARM: LPAE: Page table maintenance for the 3-level format Date: Mon, 22 Nov 2010 12:58:13 +0000 [thread overview] Message-ID: <20101122125813.GC31227@n2100.arm.linux.org.uk> (raw) In-Reply-To: <1289584840-18097-8-git-send-email-catalin.marinas@arm.com> On Fri, Nov 12, 2010 at 06:00:27PM +0000, Catalin Marinas wrote: > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index 97a5de3..41236f0 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -124,7 +124,12 @@ extern pgprot_t pgprot_kernel; > extern struct page *empty_zero_page; > #define ZERO_PAGE(vaddr) (empty_zero_page) > > +#ifdef CONFIG_ARM_LPAE > +#define pte_pfn(pte) ((pte_val(pte) & PTE_PFN_MASK) >> PAGE_SHIFT) > +#else > #define pte_pfn(pte) (pte_val(pte) >> PAGE_SHIFT) > +#endif Just make LPAE and non-LPAE both provide PTE_PFN_MASK - for non-LPAE this can be defined as ~0UL to optimize it away. However, PTE_PFN_MASK is the wrong name for this - you're not masking out the PFN, but the physical address. It only becomes a PFN when you shift. This is important because... > +static inline pte_t *pmd_page_vaddr(pmd_t pmd) > +{ > + return __va(pmd_val(pmd) & PTE_PFN_MASK); ... here it becomes much more confusing - it suggests that "pmd_val(pmd) & PTE_PFN_MASK" gives you a PFN, which you then pass to a function which takes a physical address. Also, pmd_page_vaddr() in my patches ends up as: static inline pte_t *pmd_page_vaddr(pmd_t pmd) { + return __va(pmd_val(pmd) & PAGE_MASK); } which is almost the same. I'd suggest that this becomes for both: static inline pte_t *pmd_page_vaddr(pmd_t pmd) { return __va(pmd_val(pmd) & PTE_PFN_MASK & PAGE_MASK); } but with PTE_PFN_MASK more appropriately named. > +} > + > +#else /* !CONFIG_ARM_LPAE */ > + > #define pmd_bad(pmd) (pmd_val(pmd) & 2) > > #define copy_pmd(pmdpd,pmdps) \ > @@ -252,7 +285,13 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd) > return __va(ptr); > } > > +#endif /* CONFIG_ARM_LPAE */ > + > +#ifdef CONFIG_ARM_LPAE > +#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PTE_PFN_MASK)) > +#else > #define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd))) > +#endif Ditto. > diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h > index 8fdae9b..f00ae99 100644 > --- a/arch/arm/include/asm/proc-fns.h > +++ b/arch/arm/include/asm/proc-fns.h > @@ -263,6 +263,18 @@ > > #define cpu_switch_mm(pgd,mm) cpu_do_switch_mm(virt_to_phys(pgd),mm) > > +#ifdef CONFIG_ARM_LPAE > +#define cpu_get_pgd() \ > + ({ \ > + unsigned long pg, pg2; \ > + __asm__("mrrc p15, 0, %0, %1, c2" \ > + : "=r" (pg), "=r" (pg2) \ > + : \ > + : "cc"); \ > + pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1); \ > + (pgd_t *)phys_to_virt(pg); \ > + }) > +#else > #define cpu_get_pgd() \ > ({ \ > unsigned long pg; \ > @@ -271,6 +283,7 @@ > pg &= ~0x3fff; \ I think this wants updating to use similar math to the one above. > @@ -81,7 +90,8 @@ void free_pgd_slow(struct mm_struct *mm, pgd_t *pgd) > if (!pgd) > return; > > - /* pgd is always present and good */ > + if (pgd_none(*pgd)) > + goto free; This actually wants to become something more like: + pgd = pgd_base + pgd_index(0); + if (pgd_none_or_clear_bad(pgd)) + goto no_pgd; + pmd = pmd_offset(pgd, 0); + if (pmd_none_or_clear_bad(pmd)) + goto no_pmd; pte = pmd_pgtable(*pmd); pmd_clear(pmd); pte_free(mm, pte); +no_pmd: + pgd_clear(pgd); pmd_free(mm, pmd); +no_pgd: free_pgd(pgd_base);
WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 07/20] ARM: LPAE: Page table maintenance for the 3-level format Date: Mon, 22 Nov 2010 12:58:13 +0000 [thread overview] Message-ID: <20101122125813.GC31227@n2100.arm.linux.org.uk> (raw) In-Reply-To: <1289584840-18097-8-git-send-email-catalin.marinas@arm.com> On Fri, Nov 12, 2010 at 06:00:27PM +0000, Catalin Marinas wrote: > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index 97a5de3..41236f0 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -124,7 +124,12 @@ extern pgprot_t pgprot_kernel; > extern struct page *empty_zero_page; > #define ZERO_PAGE(vaddr) (empty_zero_page) > > +#ifdef CONFIG_ARM_LPAE > +#define pte_pfn(pte) ((pte_val(pte) & PTE_PFN_MASK) >> PAGE_SHIFT) > +#else > #define pte_pfn(pte) (pte_val(pte) >> PAGE_SHIFT) > +#endif Just make LPAE and non-LPAE both provide PTE_PFN_MASK - for non-LPAE this can be defined as ~0UL to optimize it away. However, PTE_PFN_MASK is the wrong name for this - you're not masking out the PFN, but the physical address. It only becomes a PFN when you shift. This is important because... > +static inline pte_t *pmd_page_vaddr(pmd_t pmd) > +{ > + return __va(pmd_val(pmd) & PTE_PFN_MASK); ... here it becomes much more confusing - it suggests that "pmd_val(pmd) & PTE_PFN_MASK" gives you a PFN, which you then pass to a function which takes a physical address. Also, pmd_page_vaddr() in my patches ends up as: static inline pte_t *pmd_page_vaddr(pmd_t pmd) { + return __va(pmd_val(pmd) & PAGE_MASK); } which is almost the same. I'd suggest that this becomes for both: static inline pte_t *pmd_page_vaddr(pmd_t pmd) { return __va(pmd_val(pmd) & PTE_PFN_MASK & PAGE_MASK); } but with PTE_PFN_MASK more appropriately named. > +} > + > +#else /* !CONFIG_ARM_LPAE */ > + > #define pmd_bad(pmd) (pmd_val(pmd) & 2) > > #define copy_pmd(pmdpd,pmdps) \ > @@ -252,7 +285,13 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd) > return __va(ptr); > } > > +#endif /* CONFIG_ARM_LPAE */ > + > +#ifdef CONFIG_ARM_LPAE > +#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PTE_PFN_MASK)) > +#else > #define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd))) > +#endif Ditto. > diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h > index 8fdae9b..f00ae99 100644 > --- a/arch/arm/include/asm/proc-fns.h > +++ b/arch/arm/include/asm/proc-fns.h > @@ -263,6 +263,18 @@ > > #define cpu_switch_mm(pgd,mm) cpu_do_switch_mm(virt_to_phys(pgd),mm) > > +#ifdef CONFIG_ARM_LPAE > +#define cpu_get_pgd() \ > + ({ \ > + unsigned long pg, pg2; \ > + __asm__("mrrc p15, 0, %0, %1, c2" \ > + : "=r" (pg), "=r" (pg2) \ > + : \ > + : "cc"); \ > + pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1); \ > + (pgd_t *)phys_to_virt(pg); \ > + }) > +#else > #define cpu_get_pgd() \ > ({ \ > unsigned long pg; \ > @@ -271,6 +283,7 @@ > pg &= ~0x3fff; \ I think this wants updating to use similar math to the one above. > @@ -81,7 +90,8 @@ void free_pgd_slow(struct mm_struct *mm, pgd_t *pgd) > if (!pgd) > return; > > - /* pgd is always present and good */ > + if (pgd_none(*pgd)) > + goto free; This actually wants to become something more like: + pgd = pgd_base + pgd_index(0); + if (pgd_none_or_clear_bad(pgd)) + goto no_pgd; + pmd = pmd_offset(pgd, 0); + if (pmd_none_or_clear_bad(pmd)) + goto no_pmd; pte = pmd_pgtable(*pmd); pmd_clear(pmd); pte_free(mm, pte); +no_pmd: + pgd_clear(pgd); pmd_free(mm, pmd); +no_pgd: free_pgd(pgd_base);
next prev parent reply other threads:[~2010-11-22 12:58 UTC|newest] Thread overview: 154+ messages / expand[flat|nested] mbox.gz Atom feed top 2010-11-12 18:00 [PATCH v2 00/20] ARM: Add support for the Large Physical Address Extensions Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 01/20] ARM: LPAE: Use PMD_(SHIFT|SIZE|MASK) instead of PGDIR_* Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 12:43 ` Russell King - ARM Linux 2010-11-22 12:43 ` Russell King - ARM Linux 2010-11-22 13:00 ` Catalin Marinas 2010-11-22 13:00 ` Catalin Marinas 2010-11-22 13:28 ` Russell King - ARM Linux 2010-11-22 13:28 ` Russell King - ARM Linux 2010-11-12 18:00 ` [PATCH v2 02/20] ARM: LPAE: Factor out 2-level page table definitions into separate files Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-15 23:31 ` Russell King - ARM Linux 2010-11-15 23:31 ` Russell King - ARM Linux 2010-11-16 9:14 ` Catalin Marinas 2010-11-16 9:14 ` Catalin Marinas 2010-11-16 9:59 ` Russell King - ARM Linux 2010-11-16 9:59 ` Russell King - ARM Linux 2010-11-16 10:02 ` Catalin Marinas 2010-11-16 10:02 ` Catalin Marinas 2010-11-16 10:04 ` Russell King - ARM Linux 2010-11-16 10:04 ` Russell King - ARM Linux 2010-11-16 10:11 ` Catalin Marinas 2010-11-16 10:11 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 03/20] ARM: LPAE: use u32 instead of unsigned long for 32-bit ptes Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-14 13:19 ` Russell King - ARM Linux 2010-11-14 13:19 ` Russell King - ARM Linux 2010-11-14 14:09 ` Catalin Marinas 2010-11-14 14:09 ` Catalin Marinas 2010-11-14 14:13 ` Catalin Marinas 2010-11-14 14:13 ` Catalin Marinas 2010-11-14 15:14 ` Russell King - ARM Linux 2010-11-14 15:14 ` Russell King - ARM Linux 2010-11-15 9:39 ` Catalin Marinas 2010-11-15 9:39 ` Catalin Marinas 2010-11-15 9:47 ` Arnd Bergmann 2010-11-15 9:47 ` Arnd Bergmann 2010-11-15 9:51 ` Catalin Marinas 2010-11-15 9:51 ` Catalin Marinas 2010-11-15 22:11 ` Nicolas Pitre 2010-11-15 22:11 ` Nicolas Pitre 2010-11-15 23:35 ` Russell King - ARM Linux 2010-11-15 23:35 ` Russell King - ARM Linux 2010-11-16 9:19 ` Catalin Marinas 2010-11-16 9:19 ` Catalin Marinas 2010-11-15 22:07 ` Nicolas Pitre 2010-11-15 22:07 ` Nicolas Pitre 2010-11-15 17:36 ` Russell King - ARM Linux 2010-11-15 17:36 ` Russell King - ARM Linux 2010-11-15 17:39 ` Catalin Marinas 2010-11-15 17:39 ` Catalin Marinas 2010-11-16 19:34 ` Catalin Marinas 2010-11-16 19:34 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 04/20] ARM: LPAE: Do not assume Linux PTEs are always at PTRS_PER_PTE offset Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-15 17:42 ` Russell King - ARM Linux 2010-11-15 17:42 ` Russell King - ARM Linux 2010-11-15 21:46 ` Catalin Marinas 2010-11-15 21:46 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 05/20] ARM: LPAE: Introduce L_PTE_NOEXEC and L_PTE_NOWRITE Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-15 18:30 ` Russell King - ARM Linux 2010-11-15 18:30 ` Russell King - ARM Linux 2010-11-16 10:07 ` Catalin Marinas 2010-11-16 10:07 ` Catalin Marinas 2010-11-16 15:18 ` Catalin Marinas 2010-11-16 15:18 ` Catalin Marinas 2010-11-16 15:32 ` Catalin Marinas 2010-11-16 15:32 ` Catalin Marinas 2010-11-16 18:19 ` Russell King - ARM Linux 2010-11-16 18:19 ` Russell King - ARM Linux 2010-11-17 17:02 ` Catalin Marinas 2010-11-17 17:02 ` Catalin Marinas 2010-11-17 17:16 ` Russell King - ARM Linux 2010-11-17 17:16 ` Russell King - ARM Linux 2010-11-17 17:22 ` Catalin Marinas 2010-11-17 17:22 ` Catalin Marinas 2010-11-17 17:24 ` Russell King - ARM Linux 2010-11-17 17:24 ` Russell King - ARM Linux 2010-11-17 17:30 ` Catalin Marinas 2010-11-17 17:30 ` Catalin Marinas 2010-11-17 17:32 ` Russell King - ARM Linux 2010-11-17 17:32 ` Russell King - ARM Linux 2010-11-17 17:34 ` Catalin Marinas 2010-11-17 17:34 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 06/20] ARM: LPAE: Introduce the 3-level page table format definitions Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-15 18:34 ` Russell King - ARM Linux 2010-11-15 18:34 ` Russell King - ARM Linux 2010-11-16 9:57 ` Catalin Marinas 2010-11-16 9:57 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 07/20] ARM: LPAE: Page table maintenance for the 3-level format Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 12:58 ` Russell King - ARM Linux [this message] 2010-11-22 12:58 ` Russell King - ARM Linux 2010-11-12 18:00 ` [PATCH v2 08/20] ARM: LPAE: MMU setup for the 3-level page table format Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-14 10:13 ` Catalin Marinas 2010-11-14 10:13 ` Catalin Marinas 2010-11-22 13:10 ` Russell King - ARM Linux 2010-11-22 13:10 ` Russell King - ARM Linux 2010-11-23 11:38 ` Catalin Marinas 2010-11-23 11:38 ` Catalin Marinas 2010-11-23 17:33 ` Russell King - ARM Linux 2010-11-23 17:33 ` Russell King - ARM Linux 2010-11-23 17:35 ` Catalin Marinas 2010-11-23 17:35 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 09/20] ARM: LPAE: Change setup_mm_for_reboot() to work with LPAE Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 13:11 ` Russell King - ARM Linux 2010-11-22 13:11 ` Russell King - ARM Linux 2010-11-12 18:00 ` [PATCH v2 10/20] ARM: LPAE: Remove the FIRST_USER_PGD_NR and USER_PTRS_PER_PGD definitions Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 13:11 ` Russell King - ARM Linux 2010-11-22 13:11 ` Russell King - ARM Linux 2010-11-12 18:00 ` [PATCH v2 11/20] ARM: LPAE: Add fault handling support Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 13:15 ` Russell King - ARM Linux 2010-11-22 13:15 ` Russell King - ARM Linux 2010-11-22 13:19 ` Catalin Marinas 2010-11-22 13:19 ` Catalin Marinas 2010-11-22 13:32 ` Russell King - ARM Linux 2010-11-22 13:32 ` Russell King - ARM Linux 2010-11-22 13:38 ` Catalin Marinas 2010-11-22 13:38 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 12/20] ARM: LPAE: Add context switching support Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 13/20] ARM: LPAE: Add SMP support for the 3-level page table format Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 13:37 ` Russell King - ARM Linux 2010-11-22 13:37 ` Russell King - ARM Linux 2010-11-12 18:00 ` [PATCH v2 14/20] ARM: LPAE: use phys_addr_t instead of unsigned long for physical addresses Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 15/20] ARM: LPAE: Use generic dma_addr_t type definition Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 16/20] ARM: LPAE: mark memory banks with start > ULONG_MAX as highmem Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 17/20] ARM: LPAE: use phys_addr_t for physical start address in early_mem Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 18/20] ARM: LPAE: add support for ATAG_MEM64 Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 19/20] ARM: LPAE: define printk format for physical addresses and page table entries Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-22 13:43 ` Russell King - ARM Linux 2010-11-22 13:43 ` Russell King - ARM Linux 2010-11-22 13:49 ` Catalin Marinas 2010-11-22 13:49 ` Catalin Marinas 2010-11-12 18:00 ` [PATCH v2 20/20] ARM: LPAE: Add the Kconfig entries Catalin Marinas 2010-11-12 18:00 ` Catalin Marinas 2010-11-13 12:38 ` Sergei Shtylyov 2010-11-13 12:38 ` Sergei Shtylyov 2010-11-14 10:11 ` Catalin Marinas 2010-11-14 10:11 ` Catalin Marinas
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20101122125813.GC31227@n2100.arm.linux.org.uk \ --to=linux@arm.linux.org.uk \ --cc=catalin.marinas@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.