Hi Geert!
On 09/10/2018 13:37, Geert Uytterhoeven wrote:
> On Fri, Oct 5, 2018 at 3:55 PM James Morse <james.morse@xxxxxxx> 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
