arm64: restrict __pa translation to linear virtual addresses

The variable 'high_memory' ends up being referenced in the call to dma_contiguous_reserve(). So move the assignment before that call.

Signed-off-by Ard Biesheuvel <[email protected]>

Quoted 4 lines · Ard Biesheuvel

The variable 'high_memory' ends up being referenced in the call to dma_contiguous_reserve(). So move the assignment before that call.

Signed-off-by Ard Biesheuvel <[email protected]>

Does this need cc stable? It looks like we've had this issue for a while and it ends up being used in cma_declare_contiguous(). So we either get a high enough value to not affect this function or we don't trigger this code path very often.

Quoted 4 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +0 -0

@@ -679,7 +679,7 @@ void __init early_fixmap_init(void)
 

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 685

Quoted 7 lines

arch/arm64/mm/mmu.c +1 -1

      pgd = pgd_offset_k(addr);
      if (CONFIG_PGTABLE_LEVELS > 3 &&
-         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
              /*
               * We only end up here if the kernel mapping and the fixmap
               * share the top level pgd entry, which should only happen on

Do I miss any patches? The for-next/core branch has a pgd_none(*pgd) check here, so this patch does not apply.

Quoted 5 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +0 -0

@@ -679,7 +679,7 @@ void __init early_fixmap_init(void)
 

Quoted 11 lines

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 685

Quoted 7 lines

arch/arm64/mm/mmu.c +1 -1

      pgd = pgd_offset_k(addr);
      if (CONFIG_PGTABLE_LEVELS > 3 &&
-         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
              /*
               * We only end up here if the kernel mapping and the fixmap
               * share the top level pgd entry, which should only happen on

Do I miss any patches? The for-next/core branch has a pgd_none(*pgd) check here, so this patch does not apply.

This is actually based on the kaslr branch, not for-next/core

I'm happy to rebase and resend.

I have added some patches for kasan, kvm and smp as well.

Quoted 20 lines · Ard Biesheuvel

Quoted 5 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +0 -0

@@ -679,7 +679,7 @@ void __init early_fixmap_init(void)
 

Quoted 11 lines

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 685

Quoted 7 lines

arch/arm64/mm/mmu.c +1 -1

      pgd = pgd_offset_k(addr);
      if (CONFIG_PGTABLE_LEVELS > 3 &&
-         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
              /*
               * We only end up here if the kernel mapping and the fixmap
               * share the top level pgd entry, which should only happen on

Do I miss any patches? The for-next/core branch has a pgd_none(*pgd) check here, so this patch does not apply.

This is actually based on the kaslr branch, not for-next/core

Quoted 1 line

I'm happy to rebase and resend.

That's fine, no need to resend. I plan to move those as well onto for-next/core but wanted some more testing first on the initial part.

Quoted 1 line

I have added some patches for kasan, kvm and smp as well.

Patches related to the __pa clean-up? Or something else?

Quoted 23 lines · Catalin Marinas

Quoted 21 lines · Ard Biesheuvel

Quoted 5 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +0 -0

@@ -679,7 +679,7 @@ void __init early_fixmap_init(void)
 

Quoted 11 lines

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 685

Quoted 7 lines

arch/arm64/mm/mmu.c +1 -1

      pgd = pgd_offset_k(addr);
      if (CONFIG_PGTABLE_LEVELS > 3 &&
-         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
              /*
               * We only end up here if the kernel mapping and the fixmap
               * share the top level pgd entry, which should only happen on

Do I miss any patches? The for-next/core branch has a pgd_none(*pgd) check here, so this patch does not apply.

This is actually based on the kaslr branch, not for-next/core

I'm happy to rebase and resend.

Quoted 6 lines

That's fine, no need to resend. I plan to move those as well onto for-next/core but wanted some more testing first on the initial part.

Quoted 1 line

I have added some patches for kasan, kvm and smp as well.

Patches related to the __pa clean-up? Or something else?

No, related to the __pa restriction

Quoted 27 lines · Ard Biesheuvel

Quoted 26 lines · Catalin Marinas

Quoted 18 lines · Ard Biesheuvel

Quoted 5 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +0 -0

@@ -679,7 +679,7 @@ void __init early_fixmap_init(void)
 

Quoted 11 lines

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 685

Quoted 7 lines

arch/arm64/mm/mmu.c +1 -1

      pgd = pgd_offset_k(addr);
      if (CONFIG_PGTABLE_LEVELS > 3 &&
-         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
              /*
               * We only end up here if the kernel mapping and the fixmap
               * share the top level pgd entry, which should only happen on

Do I miss any patches? The for-next/core branch has a pgd_none(*pgd) check here, so this patch does not apply.

Quoted 3 lines

This is actually based on the kaslr branch, not for-next/core

I'm happy to rebase and resend.

That's fine, no need to resend. I plan to move those as well onto for-next/core but wanted some more testing first on the initial part.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

I have added some patches for kasan, kvm and smp as well.

Patches related to the __pa clean-up? Or something else?

No, related to the __pa restriction

OK. I'll wait for a while before merging this series to get more reviews and testing. I picked the first patch though (high_memory fix).

Quoted 32 lines · Catalin Marinas

Quoted 31 lines · Ard Biesheuvel

Quoted 29 lines · Catalin Marinas

Quoted 18 lines · Ard Biesheuvel

Quoted 5 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +0 -0

@@ -679,7 +679,7 @@ void __init early_fixmap_init(void)
 

Quoted 11 lines

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 685

Quoted 7 lines

arch/arm64/mm/mmu.c +1 -1

      pgd = pgd_offset_k(addr);
      if (CONFIG_PGTABLE_LEVELS > 3 &&
-         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
              /*
               * We only end up here if the kernel mapping and the fixmap
               * share the top level pgd entry, which should only happen on

Do I miss any patches? The for-next/core branch has a pgd_none(*pgd) check here, so this patch does not apply.

Quoted 3 lines

This is actually based on the kaslr branch, not for-next/core

I'm happy to rebase and resend.

That's fine, no need to resend. I plan to move those as well onto for-next/core but wanted some more testing first on the initial part.

Quoted 1 line

I have added some patches for kasan, kvm and smp as well.

Patches related to the __pa clean-up? Or something else?

Quoted 4 lines

Quoted 1 line

No, related to the __pa restriction

OK. I'll wait for a while before merging this series to get more reviews and testing. I picked the first patch though (high_memory fix).

OK, I'll hold off for now. I have enough stuff in flight as it is. However, I suppose that you will want to address the performance concern at some point

If anyone wants to do any benchmarking:
git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-pa-linear-mapping

Now that we have replaced all occurrences of __pa() translations involving virtual addresses that are covered by the kernel text, we can redefine __virt_to_phys and __pa() etc to only take virtual address that are covered by the linear mapping. This means we can remove the comparison with PAGE_OFFSET in the definition of __virt_to_phys().

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +0 -1

index 56d6739430f3..3b5dc5b243ac 100644
@@ -86,11 +86,14 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */
-#define __virt_to_phys(x) ({						\

Quoted 25 lines · Ard Biesheuvel

Now that we have replaced all occurrences of __pa() translations involving virtual addresses that are covered by the kernel text, we can redefine __virt_to_phys and __pa() etc to only take virtual address that are covered by the linear mapping. This means we can remove the comparison with PAGE_OFFSET in the definition of __virt_to_phys().

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +4 -1

index 56d6739430f3..3b5dc5b243ac 100644
@@ -86,11 +86,14 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */
-#define __virt_to_phys(x) ({                                         \
+#ifndef CONFIG_DEBUG_VM
+#define __virt_to_phys(x)    (((phys_addr_t)(x) & ~PAGE_OFFSET) + PHYS_OFFSET)
+#else
+#define __virt_to_phys(x)    ({                                      \

arch/arm64/include/asm/memory.h line 95

Quoted 6 lines

arch/arm64/include/asm/memory.h +2 -3

      phys_addr_t __x = (phys_addr_t)(x);                             \
-     __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET :   \
-                              (__x - kimage_voffset); })
-
+     BUG_ON(__x < PAGE_OFFSET);                                      \
+     (((phys_addr_t)__x & ~PAGE_OFFSET) + PHYS_OFFSET); })

What's the #include-hell like if you try to use VM_BUG_ON instead?

Quoted 26 lines · Will Deacon

Quoted 25 lines · Ard Biesheuvel

Now that we have replaced all occurrences of __pa() translations involving virtual addresses that are covered by the kernel text, we can redefine __virt_to_phys and __pa() etc to only take virtual address that are covered by the linear mapping. This means we can remove the comparison with PAGE_OFFSET in the definition of __virt_to_phys().

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +4 -1

index 56d6739430f3..3b5dc5b243ac 100644
@@ -86,11 +86,14 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */
-#define __virt_to_phys(x) ({                                         \
+#ifndef CONFIG_DEBUG_VM
+#define __virt_to_phys(x)    (((phys_addr_t)(x) & ~PAGE_OFFSET) + PHYS_OFFSET)
+#else
+#define __virt_to_phys(x)    ({                                      \

Quoted 9 lines

arch/arm64/include/asm/memory.h line 95

Quoted 6 lines

arch/arm64/include/asm/memory.h +2 -3

      phys_addr_t __x = (phys_addr_t)(x);                             \
-     __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET :   \
-                              (__x - kimage_voffset); })
-
+     BUG_ON(__x < PAGE_OFFSET);                                      \
+     (((phys_addr_t)__x & ~PAGE_OFFSET) + PHYS_OFFSET); })

What's the #include-hell like if you try to use VM_BUG_ON instead?

The #include hell would not change I think, since

include/linux/mmdebug.h:18:#define VM_BUG_ON(cond) BUG_ON(cond)

but it would certainly make this code look a lot cleaner.

Quoted 26 lines · Will Deacon

Quoted 25 lines · Ard Biesheuvel

Now that we have replaced all occurrences of __pa() translations involving virtual addresses that are covered by the kernel text, we can redefine __virt_to_phys and __pa() etc to only take virtual address that are covered by the linear mapping. This means we can remove the comparison with PAGE_OFFSET in the definition of __virt_to_phys().

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +4 -1

index 56d6739430f3..3b5dc5b243ac 100644
@@ -86,11 +86,14 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */
-#define __virt_to_phys(x) ({                                         \
+#ifndef CONFIG_DEBUG_VM
+#define __virt_to_phys(x)    (((phys_addr_t)(x) & ~PAGE_OFFSET) + PHYS_OFFSET)
+#else
+#define __virt_to_phys(x)    ({                                      \

Quoted 9 lines

arch/arm64/include/asm/memory.h line 95

Quoted 6 lines

arch/arm64/include/asm/memory.h +2 -3

      phys_addr_t __x = (phys_addr_t)(x);                             \
-     __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET :   \
-                              (__x - kimage_voffset); })
-
+     BUG_ON(__x < PAGE_OFFSET);                                      \
+     (((phys_addr_t)__x & ~PAGE_OFFSET) + PHYS_OFFSET); })

What's the #include-hell like if you try to use VM_BUG_ON instead?

The #include hell would not change I think, since

include/linux/mmdebug.h:18:#define VM_BUG_ON(cond) BUG_ON(cond)

but it would certainly make this code look a lot cleaner.

As pointed out by Will, performing non-trivial arithmetic in the implementation of __pa() may affect performance. So instead of allowing __pa() to deal with addresses that are either covered by the kernel mappin in the vmalloc area, or by the linear region, let's restrict ourselves to the latter.

This involves replacing some __pa() translations with a specific translation for kernel symbols (__pa_symbol), but other than that it is fairly straightforward (except KASAN is being a pain again, but for the purpose of this discussion that should not matter too much)

Patch #1 is a fix for a bug that I spotted while working on this.

Patch #2 introduces the new translations __kimg_to_phys and __pa_symbol. They do exactly the same but the former matches the existing __phys_to_kimg, and the latter already exists on x86 as well for translating 'symbols visible to C code'

Patch #3 - #9 replace various instances of __pa() translations involving virtual addresses pointing into the kernel image.

Patch #10 finally removes the comparison against PAGE_OFFSET from the implementation of __virt_to_phys(), so that it can only be used for addresses that are known to be in the linear region.

git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-pa-linear-mapping

Ard Biesheuvel (10):
arm64: mm: move assignment of 'high_memory' before its first use
arm64: introduce __kimg_to_phys() and __pa_symbol()
arm64: mm: avoid __pa translations in cpu_replace_ttbr1
arm64: mm: avoid __pa translations on empty_zero_page
arm64: mm: avoid __pa translations in early_fixmap_init
arm64: mm: use __pa_symbol() not __pa() for section boundary symbols
arm64: mm: avoid __pa translations for idmap_pg_dir and swapper_pg_dir
arm64: vdso: avoid __pa translations
arm64: insn: avoid __pa translations
arm64: mm: restrict __pa() translations to linear virtual addresses

 arch/arm64/include/asm/memory.h      | 15 ++++++++----
 arch/arm64/include/asm/mmu_context.h | 10 ++++----
 arch/arm64/include/asm/pgalloc.h     |  2 --
 arch/arm64/include/asm/pgtable.h     |  2 +-
 arch/arm64/kernel/insn.c             |  2 +-
 arch/arm64/kernel/setup.c            |  8 +++----
 arch/arm64/kernel/vdso.c             |  5 ++--
 arch/arm64/mm/init.c                 | 13 +++++++----
 arch/arm64/mm/mmu.c                  | 24 ++++++++++----------
 9 files changed, 43 insertions(+), 38 deletions(-)

--
2.5.0