From: Steve Capper <steve.capper@linaro.org>
To: Jungseok Lee <jays.lee@samsung.com>
Cc: linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, Catalin.Marinas@arm.com,
Marc Zyngier <Marc.Zyngier@arm.com>,
Christoffer Dall <christoffer.dall@linaro.org>,
kgene.kim@samsung.com, Arnd Bergmann <arnd@arndb.de>,
linux-kernel@vger.kernel.org, ilho215.lee@samsung.com,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
sungjinn.chung@samsung.com
Subject: Re: [PATCH 7/8] arm64: mm: Implement 4 levels of translation tables
Date: Mon, 14 Apr 2014 10:14:25 +0100 [thread overview]
Message-ID: <20140414091424.GA14204@linaro.org> (raw)
In-Reply-To: <000501cf57b4$e57e8e80$b07bab80$@samsung.com>
On Mon, Apr 13, 2014 at 04:41:07PM +0900, Jungseok Lee wrote:
> This patch implements 4 levels of translation tables since 3 levels
> of page tables with 4KB pages cannot support 40-bit physical address
> space described in [1] due to the following issue.
>
> It is a restriction that kernel logical memory map with 4KB + 3 levels
> (0xffffffc000000000-0xffffffffffffffff) cannot cover RAM region from
> 544GB to 1024GB in [1]. Specifically, ARM64 kernel fails to create
> mapping for this region in map_mem function since __phys_to_virt for
> this region reaches to address overflow.
>
> If SoC design follows the document, [1], over 32GB RAM would be placed
> from 544GB. Even 64GB system is supposed to use the region from 544GB
> to 576GB for only 32GB RAM. Naturally, it would reach to enable 4 levels
> of page tables to avoid hacking __virt_to_phys and __phys_to_virt.
>
> However, it is recommended 4 levels of page table should be only enabled
> if memory map is too sparse or there is about 512GB RAM.
Hi,
So I thought I'd apply this series and have a play, this patch doesn't apply
cleanly for me, please see below why...
[ ... ]
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 0fd5650..0b0b16a 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -46,8 +46,8 @@
> #error KERNEL_RAM_VADDR must start at 0xXXX80000
> #endif
>
> -#define SWAPPER_DIR_SIZE (3 * PAGE_SIZE)
> -#define IDMAP_DIR_SIZE (2 * PAGE_SIZE)
> +#define SWAPPER_DIR_SIZE (4 * PAGE_SIZE)
> +#define IDMAP_DIR_SIZE (3 * PAGE_SIZE)
>
> .globl swapper_pg_dir
> .equ swapper_pg_dir, KERNEL_RAM_VADDR - SWAPPER_DIR_SIZE
> @@ -384,6 +384,20 @@ ENDPROC(__calc_phys_offset)
> .endm
>
> /*
> + * Macro to populate the PUD for the corresponding block entry in the next
> + * level (tbl) for the given virtual address.
> + *
> + * Preserves: pud, tbl, virt
> + * Corrupts: tmp1, tmp2
> + */
> + .macro create_pud_entry, pud, tbl, virt, tmp1, tmp2
> + lsr \tmp1, \virt, #PUD_SHIFT
> + and \tmp1, \tmp1, #PTRS_PER_PUD - 1 // PUD index
> + orr \tmp2, \tbl, #3 // PUD entry table type
> + str \tmp2, [\pud, \tmp1, lsl #3]
> + .endm
> +
> +/*
> * Macro to populate block entries in the page table for the start..end
> * virtual range (inclusive).
> *
> @@ -445,10 +459,18 @@ __create_page_tables:
> ldr x3, =KERNEL_START
> add x3, x3, x28 // __pa(KERNEL_START)
I don't think we have C++ style comments in the kernel. Also, I can't see
any references to =KERNEL_START in arch/arm64/kernel/head.S (from 3.14 down).
> create_pgd_entry x25, x0, x3, x5, x6
> +#ifdef CONFIG_ARM64_4_LEVELS
> + add x1, x0, #PAGE_SIZE
> + create_pud_entry x0, x1, x3, x5, x6
> +#endif
> ldr x6, =KERNEL_END
> mov x5, x3 // __pa(KERNEL_START)
> add x6, x6, x28 // __pa(KERNEL_END)
> +#ifndef CONFIG_ARM64_4_LEVELS
> create_block_map x0, x7, x3, x5, x6
> +#else
> + create_block_map x1, x7, x3, x5, x6
> +#endif
>
> /*
> * Map the kernel image (starting with PHYS_OFFSET).
> @@ -456,9 +478,17 @@ __create_page_tables:
> add x0, x26, #PAGE_SIZE // section table address
> mov x5, #PAGE_OFFSET
> create_pgd_entry x26, x0, x5, x3, x6
> +#ifdef CONFIG_ARM64_4_LEVELS
> + add x1, x0, #PAGE_SIZE
> + create_pud_entry x0, x1, x3, x5, x6
> +#endif
> ldr x6, =KERNEL_END
> mov x3, x24 // phys offset
> +#ifndef CONFIG_ARM64_4_LEVELS
> create_block_map x0, x7, x3, x5, x6
> +#else
> + create_block_map x1, x7, x3, x5, x6
> +#endif
>
> /*
> * Map the FDT blob (maximum 2MB; must be within 512MB of
> @@ -474,14 +504,25 @@ __create_page_tables:
> add x5, x5, x6 // __va(FDT blob)
> add x6, x5, #1 << 21 // 2MB for the FDT blob
> sub x6, x6, #1 // inclusive range
> +#ifndef CONFIG_ARM64_4_LEVELS
> create_block_map x0, x7, x3, x5, x6
> +#else
> + create_block_map x1, x7, x3, x5, x6
> +#endif
> 1:
> /*
> * Create the pgd entry for the fixed mappings.
> */
> ldr x5, =FIXADDR_TOP // Fixed mapping virtual address
> +#ifndef CONFIG_ARM64_4_LEVELS
> add x0, x26, #2 * PAGE_SIZE // section table address
> create_pgd_entry x26, x0, x5, x6, x7
> +#else
> + add x0, x26, #PAGE_SIZE
> + create_pgd_entry x26, x0, x5, x6, x7
> + add x1, x0, #2 * PAGE_SIZE
> + create_pud_entry x0, x1, x5, x6, x7
> +#endif
>
> /*
> * Since the page tables have been populated with non-cacheable
What tree is this series based on?
Thanks,
--
Steve
WARNING: multiple messages have this Message-ID (diff)
From: steve.capper@linaro.org (Steve Capper)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 7/8] arm64: mm: Implement 4 levels of translation tables
Date: Mon, 14 Apr 2014 10:14:25 +0100 [thread overview]
Message-ID: <20140414091424.GA14204@linaro.org> (raw)
In-Reply-To: <000501cf57b4$e57e8e80$b07bab80$@samsung.com>
On Mon, Apr 13, 2014 at 04:41:07PM +0900, Jungseok Lee wrote:
> This patch implements 4 levels of translation tables since 3 levels
> of page tables with 4KB pages cannot support 40-bit physical address
> space described in [1] due to the following issue.
>
> It is a restriction that kernel logical memory map with 4KB + 3 levels
> (0xffffffc000000000-0xffffffffffffffff) cannot cover RAM region from
> 544GB to 1024GB in [1]. Specifically, ARM64 kernel fails to create
> mapping for this region in map_mem function since __phys_to_virt for
> this region reaches to address overflow.
>
> If SoC design follows the document, [1], over 32GB RAM would be placed
> from 544GB. Even 64GB system is supposed to use the region from 544GB
> to 576GB for only 32GB RAM. Naturally, it would reach to enable 4 levels
> of page tables to avoid hacking __virt_to_phys and __phys_to_virt.
>
> However, it is recommended 4 levels of page table should be only enabled
> if memory map is too sparse or there is about 512GB RAM.
Hi,
So I thought I'd apply this series and have a play, this patch doesn't apply
cleanly for me, please see below why...
[ ... ]
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 0fd5650..0b0b16a 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -46,8 +46,8 @@
> #error KERNEL_RAM_VADDR must start at 0xXXX80000
> #endif
>
> -#define SWAPPER_DIR_SIZE (3 * PAGE_SIZE)
> -#define IDMAP_DIR_SIZE (2 * PAGE_SIZE)
> +#define SWAPPER_DIR_SIZE (4 * PAGE_SIZE)
> +#define IDMAP_DIR_SIZE (3 * PAGE_SIZE)
>
> .globl swapper_pg_dir
> .equ swapper_pg_dir, KERNEL_RAM_VADDR - SWAPPER_DIR_SIZE
> @@ -384,6 +384,20 @@ ENDPROC(__calc_phys_offset)
> .endm
>
> /*
> + * Macro to populate the PUD for the corresponding block entry in the next
> + * level (tbl) for the given virtual address.
> + *
> + * Preserves: pud, tbl, virt
> + * Corrupts: tmp1, tmp2
> + */
> + .macro create_pud_entry, pud, tbl, virt, tmp1, tmp2
> + lsr \tmp1, \virt, #PUD_SHIFT
> + and \tmp1, \tmp1, #PTRS_PER_PUD - 1 // PUD index
> + orr \tmp2, \tbl, #3 // PUD entry table type
> + str \tmp2, [\pud, \tmp1, lsl #3]
> + .endm
> +
> +/*
> * Macro to populate block entries in the page table for the start..end
> * virtual range (inclusive).
> *
> @@ -445,10 +459,18 @@ __create_page_tables:
> ldr x3, =KERNEL_START
> add x3, x3, x28 // __pa(KERNEL_START)
I don't think we have C++ style comments in the kernel. Also, I can't see
any references to =KERNEL_START in arch/arm64/kernel/head.S (from 3.14 down).
> create_pgd_entry x25, x0, x3, x5, x6
> +#ifdef CONFIG_ARM64_4_LEVELS
> + add x1, x0, #PAGE_SIZE
> + create_pud_entry x0, x1, x3, x5, x6
> +#endif
> ldr x6, =KERNEL_END
> mov x5, x3 // __pa(KERNEL_START)
> add x6, x6, x28 // __pa(KERNEL_END)
> +#ifndef CONFIG_ARM64_4_LEVELS
> create_block_map x0, x7, x3, x5, x6
> +#else
> + create_block_map x1, x7, x3, x5, x6
> +#endif
>
> /*
> * Map the kernel image (starting with PHYS_OFFSET).
> @@ -456,9 +478,17 @@ __create_page_tables:
> add x0, x26, #PAGE_SIZE // section table address
> mov x5, #PAGE_OFFSET
> create_pgd_entry x26, x0, x5, x3, x6
> +#ifdef CONFIG_ARM64_4_LEVELS
> + add x1, x0, #PAGE_SIZE
> + create_pud_entry x0, x1, x3, x5, x6
> +#endif
> ldr x6, =KERNEL_END
> mov x3, x24 // phys offset
> +#ifndef CONFIG_ARM64_4_LEVELS
> create_block_map x0, x7, x3, x5, x6
> +#else
> + create_block_map x1, x7, x3, x5, x6
> +#endif
>
> /*
> * Map the FDT blob (maximum 2MB; must be within 512MB of
> @@ -474,14 +504,25 @@ __create_page_tables:
> add x5, x5, x6 // __va(FDT blob)
> add x6, x5, #1 << 21 // 2MB for the FDT blob
> sub x6, x6, #1 // inclusive range
> +#ifndef CONFIG_ARM64_4_LEVELS
> create_block_map x0, x7, x3, x5, x6
> +#else
> + create_block_map x1, x7, x3, x5, x6
> +#endif
> 1:
> /*
> * Create the pgd entry for the fixed mappings.
> */
> ldr x5, =FIXADDR_TOP // Fixed mapping virtual address
> +#ifndef CONFIG_ARM64_4_LEVELS
> add x0, x26, #2 * PAGE_SIZE // section table address
> create_pgd_entry x26, x0, x5, x6, x7
> +#else
> + add x0, x26, #PAGE_SIZE
> + create_pgd_entry x26, x0, x5, x6, x7
> + add x1, x0, #2 * PAGE_SIZE
> + create_pud_entry x0, x1, x5, x6, x7
> +#endif
>
> /*
> * Since the page tables have been populated with non-cacheable
What tree is this series based on?
Thanks,
--
Steve
next prev parent reply other threads:[~2014-04-14 9:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-14 7:41 [PATCH 7/8] arm64: mm: Implement 4 levels of translation tables Jungseok Lee
2014-04-14 7:41 ` Jungseok Lee
2014-04-14 7:41 ` Jungseok Lee
2014-04-14 9:14 ` Steve Capper [this message]
2014-04-14 9:14 ` Steve Capper
2014-04-14 9:24 ` Jungseok Lee
2014-04-14 9:24 ` Jungseok Lee
2014-04-14 9:33 ` Steve Capper
2014-04-14 9:33 ` Steve Capper
2014-04-14 9:42 ` Jungseok Lee
2014-04-14 9:42 ` Jungseok Lee
2014-04-14 15:13 ` Steve Capper
2014-04-14 15:13 ` Steve Capper
2014-04-14 15:33 ` Steve Capper
2014-04-14 15:33 ` Steve Capper
2014-04-15 0:30 ` Jungseok Lee
2014-04-15 0:30 ` Jungseok Lee
2014-04-15 1:37 ` Jungseok Lee
2014-04-15 1:37 ` Jungseok Lee
2014-04-15 7:28 ` Steve Capper
2014-04-15 7:28 ` Steve Capper
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=20140414091424.GA14204@linaro.org \
--to=steve.capper@linaro.org \
--cc=Catalin.Marinas@arm.com \
--cc=Marc.Zyngier@arm.com \
--cc=arnd@arndb.de \
--cc=christoffer.dall@linaro.org \
--cc=ilho215.lee@samsung.com \
--cc=jays.lee@samsung.com \
--cc=kgene.kim@samsung.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=sungjinn.chung@samsung.com \
/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: link
Be 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.