* [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
@ 2018-10-05 13:49 James Morse
2018-10-05 14:53 ` Mark Rutland
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: James Morse @ 2018-10-05 13:49 UTC (permalink / raw)
To: linux-arm-kernel
__is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
as these symbols are internal to asm-generic and aren't defined in the
way kconfig expects. This makes them always evaluate to false.
Switch to #ifdef.
Signed-off-by: James Morse <james.morse@arm.com>
CC: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/include/asm/pgtable.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index b58f764babf8..50b1ef8584c0 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -445,10 +445,12 @@ static inline bool in_swapper_pgdir(void *addr)
static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
{
- if (__is_defined(__PAGETABLE_PMD_FOLDED) && in_swapper_pgdir(pmdp)) {
+#ifdef __PAGETABLE_PMD_FOLDED
+ if (in_swapper_pgdir(pmdp)) {
set_swapper_pgd((pgd_t *)pmdp, __pgd(pmd_val(pmd)));
return;
}
+#endif /* __PAGETABLE_PMD_FOLDED */
WRITE_ONCE(*pmdp, pmd);
@@ -503,10 +505,12 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)
static inline void set_pud(pud_t *pudp, pud_t pud)
{
- if (__is_defined(__PAGETABLE_PUD_FOLDED) && in_swapper_pgdir(pudp)) {
+#ifdef __PAGETABLE_PUD_FOLDED
+ if (in_swapper_pgdir(pudp)) {
set_swapper_pgd((pgd_t *)pudp, __pgd(pud_val(pud)));
return;
}
+#endif /* __PAGETABLE_PUD_FOLDED */
WRITE_ONCE(*pudp, pud);
--
2.19.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
2018-10-05 13:49 [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines James Morse
@ 2018-10-05 14:53 ` Mark Rutland
2018-10-05 16:20 ` Catalin Marinas
2018-10-09 12:37 ` Geert Uytterhoeven
2 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2018-10-05 14:53 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 05, 2018 at 02:49:16PM +0100, James Morse wrote:
> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
> as these symbols are internal to asm-generic and aren't defined in the
> way kconfig expects. This makes them always evaluate to false.
> Switch to #ifdef.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Mark Rutland <mark.rutland@arm.com>
Sorry for the bogus cleanup.
FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
>
> arch/arm64/include/asm/pgtable.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index b58f764babf8..50b1ef8584c0 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -445,10 +445,12 @@ static inline bool in_swapper_pgdir(void *addr)
>
> static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
> {
> - if (__is_defined(__PAGETABLE_PMD_FOLDED) && in_swapper_pgdir(pmdp)) {
> +#ifdef __PAGETABLE_PMD_FOLDED
> + if (in_swapper_pgdir(pmdp)) {
> set_swapper_pgd((pgd_t *)pmdp, __pgd(pmd_val(pmd)));
> return;
> }
> +#endif /* __PAGETABLE_PMD_FOLDED */
>
> WRITE_ONCE(*pmdp, pmd);
>
> @@ -503,10 +505,12 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)
>
> static inline void set_pud(pud_t *pudp, pud_t pud)
> {
> - if (__is_defined(__PAGETABLE_PUD_FOLDED) && in_swapper_pgdir(pudp)) {
> +#ifdef __PAGETABLE_PUD_FOLDED
> + if (in_swapper_pgdir(pudp)) {
> set_swapper_pgd((pgd_t *)pudp, __pgd(pud_val(pud)));
> return;
> }
> +#endif /* __PAGETABLE_PUD_FOLDED */
>
> WRITE_ONCE(*pudp, pud);
>
> --
> 2.19.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
2018-10-05 13:49 [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines James Morse
2018-10-05 14:53 ` Mark Rutland
@ 2018-10-05 16:20 ` Catalin Marinas
2018-10-09 12:37 ` Geert Uytterhoeven
2 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2018-10-05 16:20 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 05, 2018 at 02:49:16PM +0100, James Morse wrote:
> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
> as these symbols are internal to asm-generic and aren't defined in the
> way kconfig expects. This makes them always evaluate to false.
> Switch to #ifdef.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Mark Rutland <mark.rutland@arm.com>
Applied. Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
2018-10-05 13:49 [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines James Morse
@ 2018-10-09 12:37 ` Geert Uytterhoeven
2018-10-05 16:20 ` Catalin Marinas
2018-10-09 12:37 ` Geert Uytterhoeven
2 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-09 12:37 UTC (permalink / raw)
To: James Morse
Cc: Linux ARM, Mark Rutland, Catalin Marinas, Will Deacon, Linux-Renesas
On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@arm.com> wrote:
> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
> as these symbols are internal to asm-generic and aren't defined in the
> way kconfig expects. This makes them always evaluate to false.
> Switch to #ifdef.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Mark Rutland <mark.rutland@arm.com>
> ---
>
> arch/arm64/include/asm/pgtable.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index b58f764babf8..50b1ef8584c0 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -445,10 +445,12 @@ static inline bool in_swapper_pgdir(void *addr)
>
> static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
> {
> - if (__is_defined(__PAGETABLE_PMD_FOLDED) && in_swapper_pgdir(pmdp)) {
> +#ifdef __PAGETABLE_PMD_FOLDED
> + if (in_swapper_pgdir(pmdp)) {
> set_swapper_pgd((pgd_t *)pmdp, __pgd(pmd_val(pmd)));
> return;
> }
> +#endif /* __PAGETABLE_PMD_FOLDED */
>
> WRITE_ONCE(*pmdp, pmd);
>
> @@ -503,10 +505,12 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)
>
> static inline void set_pud(pud_t *pudp, pud_t pud)
> {
> - if (__is_defined(__PAGETABLE_PUD_FOLDED) && in_swapper_pgdir(pudp)) {
> +#ifdef __PAGETABLE_PUD_FOLDED
> + if (in_swapper_pgdir(pudp)) {
> set_swapper_pgd((pgd_t *)pudp, __pgd(pud_val(pud)));
> return;
> }
> +#endif /* __PAGETABLE_PUD_FOLDED */
>
> WRITE_ONCE(*pudp, pud);
>
This is now e9ed821be48600ea ("arm64: mm: Use #ifdef for the
__PAGETABLE_P?D_FOLDED defines") in arm64/for-next/core.
If CONFIG_DEBUG_VIRTUAL=y, it prints a few times during boot on
several R-Car Gen3 platforms:
virt_to_phys used for non-linear address: (____ptrval____)
(swapper_pg_dir+0x7e0/0x1000)
WARNING: CPU: 0 PID: 0 at arch/arm64/mm/physaddr.c:15 __virt_to_phys+0x28/0x60
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted
4.19.0-rc7-ebisu-06695-ga0f4930e0a9c94c9 #36
Hardware name: Renesas Ebisu board based on r8a77990 (DT)
pstate: 60400085 (nZCv daIf +PAN -UAO)
pc : __virt_to_phys+0x28/0x60
lr : __virt_to_phys+0x28/0x60
sp : ffffff8009003d00
x29: ffffff8009003d00 x28: 0000000048e00018
x27: ffffffbf00000000 x26: 0060000000000711
x25: 0060000000000f11 x24: ffffffbf00dfffff
x23: ffffff8009107628 x22: ffffffbf00e00000
x21: 000000007b1cf003 x20: ffffff8008d3c7e0
x19: ffffff8008d3c7e0 x18: 000000000000000a
x17: 0000000000000000 x16: 0000000061cb4b2b
x15: 0000000000000000 x14: 645f67705f726570
x13: 706177732820295f x12: 5f5f5f6c61767274
x11: 705f5f5f5f28203a x10: 7373657264646120
x9 : ffffff800900e6d0 x8 : 0000000000000000
x7 : ffffff800814bcd4 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000000
x3 : ffffffffffffffff x2 : ffffff8009036f60
x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
__virt_to_phys+0x28/0x60
set_swapper_pgd+0x2c/0xc8
vmemmap_pud_populate+0x48/0x6c
vmemmap_populate+0x74/0x150
sparse_mem_map_populate+0x48/0x5c
sparse_init_nid+0x234/0x2f4
sparse_init+0xf4/0x1b4
bootmem_init+0x80/0x1a4
setup_arch+0x1ec/0x52c
start_kernel+0x78/0x45c
Reverting this commit fixes the symptoms, but I doubt it fixes the real
underlying issue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
@ 2018-10-09 12:37 ` Geert Uytterhoeven
0 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-09 12:37 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@arm.com> wrote:
> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
> as these symbols are internal to asm-generic and aren't defined in the
> way kconfig expects. This makes them always evaluate to false.
> Switch to #ifdef.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Mark Rutland <mark.rutland@arm.com>
> ---
>
> arch/arm64/include/asm/pgtable.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index b58f764babf8..50b1ef8584c0 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -445,10 +445,12 @@ static inline bool in_swapper_pgdir(void *addr)
>
> static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
> {
> - if (__is_defined(__PAGETABLE_PMD_FOLDED) && in_swapper_pgdir(pmdp)) {
> +#ifdef __PAGETABLE_PMD_FOLDED
> + if (in_swapper_pgdir(pmdp)) {
> set_swapper_pgd((pgd_t *)pmdp, __pgd(pmd_val(pmd)));
> return;
> }
> +#endif /* __PAGETABLE_PMD_FOLDED */
>
> WRITE_ONCE(*pmdp, pmd);
>
> @@ -503,10 +505,12 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)
>
> static inline void set_pud(pud_t *pudp, pud_t pud)
> {
> - if (__is_defined(__PAGETABLE_PUD_FOLDED) && in_swapper_pgdir(pudp)) {
> +#ifdef __PAGETABLE_PUD_FOLDED
> + if (in_swapper_pgdir(pudp)) {
> set_swapper_pgd((pgd_t *)pudp, __pgd(pud_val(pud)));
> return;
> }
> +#endif /* __PAGETABLE_PUD_FOLDED */
>
> WRITE_ONCE(*pudp, pud);
>
This is now e9ed821be48600ea ("arm64: mm: Use #ifdef for the
__PAGETABLE_P?D_FOLDED defines") in arm64/for-next/core.
If CONFIG_DEBUG_VIRTUAL=y, it prints a few times during boot on
several R-Car Gen3 platforms:
virt_to_phys used for non-linear address: (____ptrval____)
(swapper_pg_dir+0x7e0/0x1000)
WARNING: CPU: 0 PID: 0 at arch/arm64/mm/physaddr.c:15 __virt_to_phys+0x28/0x60
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted
4.19.0-rc7-ebisu-06695-ga0f4930e0a9c94c9 #36
Hardware name: Renesas Ebisu board based on r8a77990 (DT)
pstate: 60400085 (nZCv daIf +PAN -UAO)
pc : __virt_to_phys+0x28/0x60
lr : __virt_to_phys+0x28/0x60
sp : ffffff8009003d00
x29: ffffff8009003d00 x28: 0000000048e00018
x27: ffffffbf00000000 x26: 0060000000000711
x25: 0060000000000f11 x24: ffffffbf00dfffff
x23: ffffff8009107628 x22: ffffffbf00e00000
x21: 000000007b1cf003 x20: ffffff8008d3c7e0
x19: ffffff8008d3c7e0 x18: 000000000000000a
x17: 0000000000000000 x16: 0000000061cb4b2b
x15: 0000000000000000 x14: 645f67705f726570
x13: 706177732820295f x12: 5f5f5f6c61767274
x11: 705f5f5f5f28203a x10: 7373657264646120
x9 : ffffff800900e6d0 x8 : 0000000000000000
x7 : ffffff800814bcd4 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000000
x3 : ffffffffffffffff x2 : ffffff8009036f60
x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
__virt_to_phys+0x28/0x60
set_swapper_pgd+0x2c/0xc8
vmemmap_pud_populate+0x48/0x6c
vmemmap_populate+0x74/0x150
sparse_mem_map_populate+0x48/0x5c
sparse_init_nid+0x234/0x2f4
sparse_init+0xf4/0x1b4
bootmem_init+0x80/0x1a4
setup_arch+0x1ec/0x52c
start_kernel+0x78/0x45c
Reverting this commit fixes the symptoms, but I doubt it fixes the real
underlying issue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
2018-10-09 12:37 ` Geert Uytterhoeven
@ 2018-10-09 13:25 ` James Morse
-1 siblings, 0 replies; 9+ messages in thread
From: James Morse @ 2018-10-09 13:25 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux ARM, Mark Rutland, Catalin Marinas, Will Deacon, Linux-Renesas
Hi Geert!
On 09/10/2018 13:37, Geert Uytterhoeven wrote:
> On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@arm.com> wrote:
>> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
>> as these symbols are internal to asm-generic and aren't defined in the
>> way kconfig expects. This makes them always evaluate to false.
>> Switch to #ifdef.
> This is now e9ed821be48600ea ("arm64: mm: Use #ifdef for the
> __PAGETABLE_P?D_FOLDED defines") in arm64/for-next/core.
>
> If CONFIG_DEBUG_VIRTUAL=y, it prints a few times during boot on
> several R-Car Gen3 platforms:
Aha, I didn't test this with debug-virtual ... and the new __pa() to setup the
fixmap for swapper is what is causing this:
>From arch/arm64/mm/mmu.c:
| void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
| {
| pgd_t *fixmap_pgdp;
|
| spin_lock(&swapper_pgdir_lock);
| fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
| WRITE_ONCE(*fixmap_pgdp, pgd);
This patch is just exposing the problem on your configuration, its commit
2330b7ca78350efcb ("arm64/mm: use fixmap to modify swapper_pg_dir") that missed
this.
We always know pgdp here will point within swapper_pg_dir, so I think its fair
to switch this to the __pa_symbol() version. This fixes it for me:
--------------------------%<--------------------------
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6f0e2edcc114..6deb836a102a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -74,7 +74,7 @@ void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
pgd_t *fixmap_pgdp;
spin_lock(&swapper_pgdir_lock);
- fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
+ fixmap_pgdp = pgd_set_fixmap(__pa_symbol(pgdp));
WRITE_ONCE(*fixmap_pgdp, pgd);
/*
* We need dsb(ishst) here to ensure the page-table-walker sees
--------------------------%<--------------------------
(which I will post shortly).
Passing not-a-symbol-name to __pa_symbol() doesn't look nice, but lm_alias()
would do this too. Manually calculating the offset in swapper_pg_dir is some
logic that is already wrapped up in pgd_set_fixmap().
Thanks!
James
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
@ 2018-10-09 13:25 ` James Morse
0 siblings, 0 replies; 9+ messages in thread
From: James Morse @ 2018-10-09 13:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Geert!
On 09/10/2018 13:37, Geert Uytterhoeven wrote:
> On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@arm.com> wrote:
>> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
>> as these symbols are internal to asm-generic and aren't defined in the
>> way kconfig expects. This makes them always evaluate to false.
>> Switch to #ifdef.
> This is now e9ed821be48600ea ("arm64: mm: Use #ifdef for the
> __PAGETABLE_P?D_FOLDED defines") in arm64/for-next/core.
>
> If CONFIG_DEBUG_VIRTUAL=y, it prints a few times during boot on
> several R-Car Gen3 platforms:
Aha, I didn't test this with debug-virtual ... and the new __pa() to setup the
fixmap for swapper is what is causing this:
>From arch/arm64/mm/mmu.c:
| void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
| {
| pgd_t *fixmap_pgdp;
|
| spin_lock(&swapper_pgdir_lock);
| fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
| WRITE_ONCE(*fixmap_pgdp, pgd);
This patch is just exposing the problem on your configuration, its commit
2330b7ca78350efcb ("arm64/mm: use fixmap to modify swapper_pg_dir") that missed
this.
We always know pgdp here will point within swapper_pg_dir, so I think its fair
to switch this to the __pa_symbol() version. This fixes it for me:
--------------------------%<--------------------------
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6f0e2edcc114..6deb836a102a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -74,7 +74,7 @@ void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
pgd_t *fixmap_pgdp;
spin_lock(&swapper_pgdir_lock);
- fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
+ fixmap_pgdp = pgd_set_fixmap(__pa_symbol(pgdp));
WRITE_ONCE(*fixmap_pgdp, pgd);
/*
* We need dsb(ishst) here to ensure the page-table-walker sees
--------------------------%<--------------------------
(which I will post shortly).
Passing not-a-symbol-name to __pa_symbol() doesn't look nice, but lm_alias()
would do this too. Manually calculating the offset in swapper_pg_dir is some
logic that is already wrapped up in pgd_set_fixmap().
Thanks!
James
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
2018-10-09 13:25 ` James Morse
@ 2018-10-09 14:19 ` Geert Uytterhoeven
-1 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-09 14:19 UTC (permalink / raw)
To: James Morse
Cc: Linux ARM, Mark Rutland, Catalin Marinas, Will Deacon, Linux-Renesas
Hi James,
On Tue, Oct 9, 2018 at 3:25 PM James Morse <james.morse@arm.com> wrote:
> On 09/10/2018 13:37, Geert Uytterhoeven wrote:
> > On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@arm.com> wrote:
> >> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
> >> as these symbols are internal to asm-generic and aren't defined in the
> >> way kconfig expects. This makes them always evaluate to false.
> >> Switch to #ifdef.
>
> > This is now e9ed821be48600ea ("arm64: mm: Use #ifdef for the
> > __PAGETABLE_P?D_FOLDED defines") in arm64/for-next/core.
> >
> > If CONFIG_DEBUG_VIRTUAL=y, it prints a few times during boot on
> > several R-Car Gen3 platforms:
>
> Aha, I didn't test this with debug-virtual ... and the new __pa() to setup the
> fixmap for swapper is what is causing this:
>
> From arch/arm64/mm/mmu.c:
> | void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
> | {
> | pgd_t *fixmap_pgdp;
> |
> | spin_lock(&swapper_pgdir_lock);
> | fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
> | WRITE_ONCE(*fixmap_pgdp, pgd);
>
> This patch is just exposing the problem on your configuration, its commit
> 2330b7ca78350efcb ("arm64/mm: use fixmap to modify swapper_pg_dir") that missed
> this.
>
> We always know pgdp here will point within swapper_pg_dir, so I think its fair
> to switch this to the __pa_symbol() version. This fixes it for me:
> --------------------------%<--------------------------
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6f0e2edcc114..6deb836a102a 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -74,7 +74,7 @@ void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
> pgd_t *fixmap_pgdp;
>
> spin_lock(&swapper_pgdir_lock);
> - fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
> + fixmap_pgdp = pgd_set_fixmap(__pa_symbol(pgdp));
> WRITE_ONCE(*fixmap_pgdp, pgd);
> /*
> * We need dsb(ishst) here to ensure the page-table-walker sees
> --------------------------%<--------------------------
Thanks!
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines
@ 2018-10-09 14:19 ` Geert Uytterhoeven
0 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-09 14:19 UTC (permalink / raw)
To: linux-arm-kernel
Hi James,
On Tue, Oct 9, 2018 at 3:25 PM James Morse <james.morse@arm.com> wrote:
> On 09/10/2018 13:37, Geert Uytterhoeven wrote:
> > On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@arm.com> wrote:
> >> __is_defined(__PAGETABLE_P?D_FOLDED) doesn't quite work as intended
> >> as these symbols are internal to asm-generic and aren't defined in the
> >> way kconfig expects. This makes them always evaluate to false.
> >> Switch to #ifdef.
>
> > This is now e9ed821be48600ea ("arm64: mm: Use #ifdef for the
> > __PAGETABLE_P?D_FOLDED defines") in arm64/for-next/core.
> >
> > If CONFIG_DEBUG_VIRTUAL=y, it prints a few times during boot on
> > several R-Car Gen3 platforms:
>
> Aha, I didn't test this with debug-virtual ... and the new __pa() to setup the
> fixmap for swapper is what is causing this:
>
> From arch/arm64/mm/mmu.c:
> | void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
> | {
> | pgd_t *fixmap_pgdp;
> |
> | spin_lock(&swapper_pgdir_lock);
> | fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
> | WRITE_ONCE(*fixmap_pgdp, pgd);
>
> This patch is just exposing the problem on your configuration, its commit
> 2330b7ca78350efcb ("arm64/mm: use fixmap to modify swapper_pg_dir") that missed
> this.
>
> We always know pgdp here will point within swapper_pg_dir, so I think its fair
> to switch this to the __pa_symbol() version. This fixes it for me:
> --------------------------%<--------------------------
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6f0e2edcc114..6deb836a102a 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -74,7 +74,7 @@ void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
> pgd_t *fixmap_pgdp;
>
> spin_lock(&swapper_pgdir_lock);
> - fixmap_pgdp = pgd_set_fixmap(__pa(pgdp));
> + fixmap_pgdp = pgd_set_fixmap(__pa_symbol(pgdp));
> WRITE_ONCE(*fixmap_pgdp, pgd);
> /*
> * We need dsb(ishst) here to ensure the page-table-walker sees
> --------------------------%<--------------------------
Thanks!
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-09 21:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 13:49 [PATCH] arm64: mm: Use #ifdef for the __PAGETABLE_P?D_FOLDED defines James Morse
2018-10-05 14:53 ` Mark Rutland
2018-10-05 16:20 ` Catalin Marinas
2018-10-09 12:37 ` Geert Uytterhoeven
2018-10-09 12:37 ` Geert Uytterhoeven
2018-10-09 13:25 ` James Morse
2018-10-09 13:25 ` James Morse
2018-10-09 14:19 ` Geert Uytterhoeven
2018-10-09 14:19 ` Geert Uytterhoeven
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.